#381 support for git in svnversion.h

[exrom]

Hi VICE-team,

as shortly discussed on 27th July in irc, here is my patch that adds support for git revision info generated by gensvnversion.sh.
Basic idea of the patch is to make the generation of svnversion.h compatible with git, while not breaking svn. This is a first step towards the goal noted in https://vice-emu.pokefinder.org/wiki/Move_to_git

In detail:

• gensvnversion.sh will now check whether the workspace is svn or git tracked and generate the revision number and the string accordingly into svnversion.h.
• svn info is no longer party retrieved by configure.ac, now all done inside gensvnversion.sh (1st parameter removed).
• SVN info now retrieved by "svn info --show-item revision" (rather than "svninfo") as this directly prints the number - no need for grep any more
• Remove adding the 'r' at several printfs in the code. This is now done in the sh too as it is needed only in svn case.
• the header file svnversion.h is now generated always. In case there is no version control, rev is zero and string is empty. There is now no need any more for conditional compilation with "#ifdef USE_SVN_REVISION". Cleaned this up many times in the code.
• The SVN_REVISION_OVERRIDE thing is now done also inside the sh.

I tested the patch with r45255.

svn workspace:
make output:
./buildtools/gensvnversion.sh: workspace is versioned by svn
./buildtools/gensvnversion.sh: using version control revision number 45255 string svn rev r45255
./buildtools/gensvnversion.sh: generating svnversion.h
svnversion.h:
#define VICE_SVN_REV_NUMBER 45255
#define VICE_SVN_REV_STRING "svn rev r45255"
x64sc ui about: 3.8 svn rev r45255

git workspace:
make output:
./buildtools/gensvnversion.sh: workspace is versioned by git
./buildtools/gensvnversion.sh: using version control revision number 1034854 string git rev cffe3fc2b9e
./buildtools/gensvnversion.sh: generating svnversion.h
svnversion.h:
#define VICE_SVN_REV_NUMBER 1034854
#define VICE_SVN_REV_STRING "git rev cffe3fc2b9e"
x64sc ui about: 3.8 git rev cffe3fc2b9e

"export SVN_REVISION_OVERRIDE=64738" before calling configure, make
make output:
./buildtools/gensvnversion.sh: SVN_REVISION_OVERRIDE is 64738
./buildtools/gensvnversion.sh: using version control revision number 64738 string 64738
./buildtools/gensvnversion.sh: generating svnversion.h
svnversion.h:
#define VICE_SVN_REV_NUMBER 64738
#define VICE_SVN_REV_STRING "64738"
x64sc ui about: 3.8 64738

few questions
The revision string is now longer than before. Seems no issue in the GTK uiabout window, but elsewhere?
I added a way to get an increasing number for git revisions. But this is uncommon in git due to git's decentral design and there are several corner cases where such a number is problematic. We should discuss.
I just have just set SVN_REVISION_OVERRIDE in the shell scope before performing the build. Did not try as an argument to configure. Not sure whether this covers all use cases.
Rename header files and macros to smth more generic (omit 'svn')?
A few scripts (e.g. make_bindist_win32.sh) use their own way to determine the svn revision. Centralize this too?

Awaiting and looking forward to your feedback.
Thank you!

[gpz]

Quickly skimmed over it - this patch seems to remove some things that we really don't wont to remove. It also shows that incremental build numbers are an integral part of what we want :) Eg:
- we absolutely want the build number in the snapshot (its there for a reason :))
- the VERSION macro should contain (only) the VICE version, and not the build number. This one is reserved for when we really only want the version. (And that is also why USE_SVN_REVISION exists)

Generally to me it seems this patch does too much, and doesn't solve the main issue (incremental build numbers). (Perhaps also the "modernizing" of how svn revision is determined could be a separate patch)

So this is the thing i'd try to solve first: somehow produce incremental numbers for builds. of course due to how git works we'll have to fake this a bit. My proposal would be something like this:

Generally, we want to be able to use this number the same way as we use svn revisions (this implies a first patch would only need to change how VICE_SVN_REV_NUMBER is defined). For this to work...

• the revision must be a (single) number
• the revision must be unique for each commit
• the revision of "newer" commits must be higher than the revision of "older" commits (note that this does NOT mean that it has to increase by one for every commit)

Now, to achieve this with git, i'd combine the git hash (which solves the "unique" part) with the date of the commit (retrieved from git). And then combine it into 64bit perhaps, like::

The time could be encoded into 32bits:

• year (0-127, 7 bits) (*)
• month (0-11, 4 bits
• day (0-30, 5bits)
• hour (0-23, 5bits)
• minute (0-59, 6bits)
• second (0-59, 6bits) (*)

YYYYYYYM|MMMDDDDD|HHHHHMMM|MMMS|SSSS(S) (*)

(*) perhaps use 6 bits for the year only, OR shift down seconds by 1 bit and use 5 bits for seconds only

.. and then the other 32bit could be (the first 32bit of) the GIT hash

When the revision is generated like this, it would then need a few code adjustments to use 64bit when it deals with the revision (perhaps while at it: typedef a vice_revision_t for this :))

[exrom]

Well, would be nice if you first could take the time and have a closer look to the patch. I did neither remove the build number nor did i reuse the version for the build number.

I removed USE_SVN_REVISION in the code, yes. But this doesn't mean, there is no svn revsion any more. Wherever i found #ifdef USE_SVN_REVISION, it is there to handle the case when VICE_SVN_REV_STRING does not exist, right?
My patch now ensures, the header exists always. Then also VICE_SVN_REV_STRING is defined always - may it contain a valid rev or just nothing. So the patch is just a clean-up measure which tries to make the code better readable. Or did i overlook something?

But ok, my patch contains multiple things at once. You are right, can be separated.
My idea was to first address the issue of not spreading version control commands over many files and concentrate them in one file. But if you see the increasing number issue as most important, let's talk about this first.

You suggest to calculate the build number from git's commit time stamp info and you say
>the revision of "newer" commits must be higher than the revision of "older" commits
Git does not guarantee this.
Say, there is commit A and on top of it there is commit B. This means, A is the parent, B is the child, or in other words, B was brought to the history later and is dependent on A. Or as picture: A <-- B
Now, B can have an older time stamp than A if B is an older commit somebody cherry-picked and rebased on top of A.
Git is designed to work like this and it would be no good idea to somehow enforce increasing commit times. What also is a problem, commit time is generated decentrally. You have to rely on the correct setting of people's RTCs.

Another idea is to not take the time stamp info from the commit, but to just count the commits up to the latest one.
This is what i tried with my added line "git rev-list HEAD --count) + 1000000".
One the one hand this would be strictly increasing but on the other hand the build number would not be unique any more.
Say, A is the latest commit on main on github with revision n. Now B comes on top and would get n+1. But B can be different for different people due to git's decentralized nature.
While the "official" next commit is B1, somebody else could add B2 on top. B1 and B2 would get the same build rev while they are totally different.
As also this approach does not work, i added a comment "WARNING:" to my patch.

In the end, any attempt to calculate such a number from the git history does not work reliably and thus, fails its purpose.

I suggest to not rely on commit meta info and also not on commit order, but on information from the build pipeline.
Whenever a pull request is merged, a new commit will appear on the main branch on the official repo. Then, a build is triggered, the ci pipeline will run and have a unique and increasing number generated for this particular pipeline run. I assume, vice wants to be on github. They call this actions. Seems, this works already (which is great btw.): https://github.com/VICE-Team/svn-mirror/actions/runs/10231027641
This could help understanding what's possible: https://stackoverflow.com/questions/54310050/how-to-version-build-artifacts-using-github-actions
Every inofficial/private build would of course not have this build number. But this immediately identifies it as "untrusted" because any change could have been done.
It's not a bug, it's a feature. Wouldn't this be a great benefit to vice, if you could say: Build no. missing. Please take an official build before filing bug reports?