Ticket #47967

gitrev is wrong in freeciv-server -v

Eröffnet am: 2023-04-30 06:58 Letztes Update: 2024-02-09 15:33

Auswertung:
Verantwortlicher:
(Keine)
Typ:
Status:
Offen
Komponente:
Meilenstein:
Priorität:
5 - Mittel
Schweregrad:
5 - Mittel
Lösung:
Keine
Datei:
4

Details

S3_1

The value return buy freeciv-server -v is an old reference (in example 164 commit behind origin/S3_1 pulled today)

$ LANG=en ./server/freeciv-server -v
Encodings: Data=UTF-8, Local=ISO-8859-1, Internal=UTF-8
Freeciv version 3.1.0-beta1+ (beta version) (modified 75200a454) 

$ git log origin/S3_1...75200a454 | grep commit | wc -l
164

It seems that the values of bootstrap/generate_gitrev.sh -> ./common/fc_gitrev_gen.h are not taken into account and we have a fallback from somewhere.

Ticket-Verlauf (3/24 Historien)

2023-04-30 06:58 Aktualisiert von: alain_bkr
  • New Ticket "gitrev is wrong in freeciv-server -v" created
2023-04-30 07:07 Aktualisiert von: cazfi
Kommentar

Can you check:
- If there are multiple fc_gitrev_gen.h files (e.g. one in source directory, one in build directory)
- Timestamp of common/version.o (i.e. has it been rebuilt after you did 'git pull')
- Timestamp of fc_gitrev_gen.h (i.e. has version.o been rebuilt *after* that header has been regenerated)

2023-04-30 16:52 Aktualisiert von: None
Kommentar

I build in a separate directory

 git rootdir : /Big/Games/freeciv_S3_1/
 builddir    : /Big/Games/freeciv_S3_1/build_clang-15
 
make distclean
../autogen.sh --enable-gitrev --enable-debug=yes ....

So as you expected there are have 2 fc_gitrev_gen.h files ( one in source directory ../ , one in build directory ./ )

OK i got it : I remove the one in source directory , probably a sequel of a previous build in root dir, and now i have what i want.

The generated file ./common/fc_gitrev_gen.h is not erased by make distclean, i guess this explains my trouble

2023-04-30 23:45 Aktualisiert von: None
Kommentar

make maintainer-clean does erase the generated fc_gitrev_gen.h

btw, i have improved bootstrap/generate_gitrev.sh , to find the most recent origin/S3_1 commit , and head commit , and count the number of local commit (here 6), and add "+ modified" if there are uncommited local changes

$ ./fcser -v
Encodings: Data=UTF-8, Local=UTF-8, Internal=UTF-8
Freeciv version 3.1.0-beta1+ (version bêta) (origin/S3_1 373049274 HEAD 7ef4e956f (+6) + modified ) 
2023-05-01 01:57 Aktualisiert von: cazfi
Kommentar

Reply To (Anonymous)

add "+ modified" if there are uncommited local changes

- There's a regression that "+ modified" does not get translated (in general: is there some reason how you split the information between REV1 and REV2?)
- Need to update comments in fc_gitrev_gen.h.tmpl to match (and I notice that translatability of "modified " was something already missing from there)
- I have not yet checked if such a long string makes sense for all freeciv_name_version() and fc_git_revision() callers, but I suspect some adjustments are needed for latter at least

2023-05-01 02:12 Aktualisiert von: alain_bkr
Kommentar
 $ ./fcser -v
 Encodings: Data=UTF-8, Local=UTF-8, Internal=UTF-8
 Freeciv version 3.1.0-beta1+ (version bêta) (origin/S3_1 373049274 HEAD 7ef4e956f (+6) + modified ) 

It works like a charm on my machine. :-)

(Edited, 2023-05-01 02:13 Aktualisiert von: alain_bkr)
2023-05-01 03:52 Aktualisiert von: alain_bkr
Kommentar

Reply To cazfi

Reply To (Anonymous)

add "+ modified" if there are uncommited local changes

- There's a regression that "+ modified" does not get translated

OK i 'll remove the + and use translate

(in general: is there some reason how you split the information between REV1 and REV2?)

yes,

  • it was split REV1/REV2
  • The REV2 gives the same hash as before the patch.

- Need to update comments in fc_gitrev_gen.h.tmpl to match (and I notice that translatability of "modified " was something already missing from there)

done, i attach the corrresponding patch

- I have not yet checked if such a long string makes sense for all freeciv_name_version() and fc_git_revision() callers, but I suspect some adjustments are needed for latter at least

It just works for the displayed version in GUI or command line, these are strings anyways, just longer ones.

2023-05-01 04:09 Aktualisiert von: cazfi
Kommentar

Reply To alain_bkr

- I have not yet checked if such a long string makes sense for all freeciv_name_version() and fc_git_revision() callers, but I suspect some adjustments are needed for latter at least

It just works for the displayed version in GUI or command line, these are strings anyways, just longer ones.

Yeah, looking for the callers, the only one not used "just for display" is the lua API one. If someone has custom script parsing the version number assuming specific format, they have taken the risk themselves.

Existing string has filled entire horizontal space on sdl2-client on lower resolution displays, but likely that's something we can resolve separately.

2023-05-01 23:24 Aktualisiert von: alain_bkr
Kommentar

1/ i don't know how to translate the word "modified" inside the string, and i would advocate it is not that important, because it occurs only when one has some uncommitted changes, so i guess that person would understand :-)

2/ maybe add common/fc_gitrev_gen.h to make clean list ? (i don't know how)

(Edited, 2023-05-01 23:27 Aktualisiert von: alain_bkr)
2023-05-02 01:31 Aktualisiert von: cazfi
Kommentar

Reply To alain_bkr

1/ i don't know how to translate the word "modified" inside the string, and i would advocate it is not that important, because it occurs only when one has some uncommitted changes, so i guess that person would understand :-)

Existing code translates REV1:

  fc_snprintf(buf, sizeof(buf), "%s%s",
              translate ? _(FC_GITREV1) : FC_GITREV1, FC_GITREV2);

The string is collected from translations/Strings.txt

Not that it would be important, but it shouldn't be that hard either.

2023-05-02 09:01 Aktualisiert von: cazfi
  • Meilenstein Update from (Keine) to 3.0.8 (closed)
  • Komponente Update from General to Bootstrap
  • Typ Update from Fehler to Patches
  • Priorität Update from 7 to 5 - Mittel
Kommentar

Reply To alain_bkr

2/ maybe add common/fc_gitrev_gen.h to make clean list ? (i don't know how)

Thought this was the bug this ticket originally was about, there's now so much more discussion about the improvement patch, that I did ticket splitting so that #47976 is about the bug.

2023-05-02 17:18 Aktualisiert von: None
Kommentar

Reply To cazfi

Reply To alain_bkr

1/ i don't know how to translate the word "modified" inside the string, and i would advocate it is not that important, because it occurs only when one has some uncommitted changes, so i guess that person would understand :-)

Existing code translates REV1:
{{{ fc_snprintf(buf, sizeof(buf), "%s%s", translate ? _(FC_GITREV1) : FC_GITREV1, FC_GITREV2); }}} The string is collected from translations/Strings.txt Not that it would be important, but it shouldn't be that hard either.

I don't know how to implement this, and i think it is unimportant, and a waste of time and energy (as i explained, people seeing this are developers with local uncommitted changes, so they will understand "modified").

For me my 2 patches are ready for production :-).

If you believe it should be implemented, please show me how to translate a substring.

2023-05-25 14:41 Aktualisiert von: cazfi
  • Verantwortlicher Update from (Keine) to cazfi
2023-05-27 02:59 Aktualisiert von: cazfi
Kommentar

One should use 'git diff' to check that the changes they are about to 'git add' are what they expect (no debug code forgotten in, etc.) At least in any environment where I work, 'git diff' also shows trailing spaces as colored blocks (screenshot attached, showing couple of trailing spaces in these patches)

2023-05-27 03:26 Aktualisiert von: cazfi
Kommentar

Working on this, but it's taking a bit head scratching to figure out what you mean to happen, e.g., with local branches for which there's no similarly named branch on a remote named 'origin'

2023-05-27 04:16 Aktualisiert von: cazfi
Kommentar

Fixed some more minor issues. Not a commit candidate.

---

- Removed trailing spaces
- Removed "common/" in a comment talking about "common/fc_gitrev_gen.h" - that's not the path e.g. in meson based builds
- "optionnally" -> "optionally"
- Removed tabs
- Separated third part, GITREV3, and made handling of "modified" via that one
- While touching this script, also removing deprecated x-prefixing ( if test "x$HEAD" != "x" ; then )
- A couple of indentation adjustments

2023-05-27 04:21 Aktualisiert von: cazfi
  • Verantwortlicher Update from cazfi to (Keine)
Kommentar

Reply To cazfi

Working on this, but it's taking a bit head scratching to figure out what you mean to happen, e.g., with local branches for which there's no similarly named branch on a remote named 'origin'

What I currently get in my work branch:

During build:

  fc_gitrev_gen.h
fatal: ambiguous argument 'origin/main-work2': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'
fatal: bad revision '^'
  CC       version.lo

Running, with no changes relative to latest commit on my tree:

$ ./fcser --version
Encodings: Data=UTF-8, Local=UTF-8, Internal=UTF-8
Freeciv version 3.2.90.3-dev (origin/main-work2  HEAD b9a62d6907 (+) )

Running, with changes relative to latest commit on my tree:

$ ./fcser --version
Encodings: Data=UTF-8, Local=UTF-8, Internal=UTF-8
Freeciv version 3.2.90.3-dev (origin/main-work2  HEAD b9a62d6907 (+)  (modified))

---

I'd prefer to give this back to alain_bkr at this point.

2023-06-23 15:51 Aktualisiert von: cazfi
2023-11-10 14:52 Aktualisiert von: cazfi
2024-02-09 15:33 Aktualisiert von: cazfi

Dateianhangliste

Bearbeiten

Please login to add comment to this ticket » Anmelden