On Wed, Nov 20, 2024 at 12:47:40PM +0900, Junio C Hamano wrote:
> Patrick Steinhardt <ps@xxxxxx> writes:
> > +GIT_CEILING_DIRECTORIES="$SOURCE_DIR/.."
> > +export GIT_CEILING_DIRECTORIES
>
> Interesting. Presumably this is to prevent a foreign project having
> a tarball extract of us in its subdirectory, which would be a good
> protection.
Yup.
> > # First see if there is a version file (included in release tarballs),
> > # then try git-describe, then default.
> > -if test -f version
> > +if test -f "$SOURCE_DIR"/version
> > then
> > - VN=$(cat version) || VN="$DEF_VER"
> > -elif { test -d "${GIT_DIR:-.git}" || test -f .git; } &&
> > - VN=$(git describe --match "v[0-9]*" HEAD 2>/dev/null) &&
> > + VN=$(cat "$SOURCE_DIR"/version) || VN="$DEF_VER"
> > +elif { test -d "$SOURCE_DIR/.git" || test -d "${GIT_DIR:-.git}" || test -f "$SOURCE_DIR"/.git; } &&
>
> The line has grown a bit too long...
Fair. I wonder how this is supposed to be formatted in the first
place... I'll get creative.
> > +GIT_DATE=$(git -C "$SOURCE_DIR" show --quiet --format='%as')
>
> This needs 2>/dev/null to squelch the case where we have no
> installed git.
Will fix.
> I suspect "%cs" is more in line with the spirit of GIT_DATE if I
> understand its purpose, i.e. "this is the time this version was
> recorded in the Git history, with the intention to give it the public"
> and better than "%as".
While I agree with you, I'll leave this one as-is for now because this
is preexisting logic. But I'll rearrange a bit so that required
placeholders are only wired up once they are actually used so that it
can be seen where they come from and that the rewrite is faithful to the
original logic.
> > +fi
> > +
> > +read GIT_MAJOR_VERSION GIT_MINOR_VERSION GIT_MICRO_VERSION GIT_PATCH_LEVEL trailing <<EOF
> > +$(echo "$GIT_VERSION" 0 0 0 0 | tr '.a-zA-Z-' ' ')
> > +EOF
>
> And because GIT_VERSION generation is safe above, this probably is
> safe, too. In the ancient days, we had tags like "v0.99.9g" which
> may not match the above convention, but with the understanding that
> we establish an official convention going forward (i.e. we allow up
> to four numbers, the rest is discarded so do not use more than
> four), it is OK.
>
> Who wants these broken-out versions and are they fine with
> up-to-four limitation? Just being curious.
The only user is "git.rc", which is where the limitation originates
from.
> > diff --git a/contrib/buildsystems/git-version.in b/contrib/buildsystems/git-version.in
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..9750505ae77685ebb31a38468caaf13501b6739d
> > --- /dev/null
> > +++ b/contrib/buildsystems/git-version.in
> > @@ -0,0 +1 @@
> > +@GIT_MAJOR_VERSION@.@GIT_MINOR_VERSION@.@GIT_MICRO_VERSION@
>
> And this one seems to discard the fourth number, so those who
> prepares VN to contain the fourth digit to differenciate a new
> version with previous ones would be disappointed. Similarly,
> because this requires the third number, we cannot change the
> versioning scheme at 3.0 boundary to say "3.1 is the next version
> after 3.0".
>
> As this is merely setting the rule for our future, perhaps we want
> to be consistently allow and require N dot-separated numbers
> everywhere (e.g., we allow and require 3 numbers, not 2, not 4, but
> exactly 3 numbers)?
Yeah, being consistent would be nice indeed. But for now I'd prefer to
keep this as-is because we'd otherwise change the version schema used by
CMake builds. In theory we can use 4 numbers here, too, where the fourth
number would correspond to the `PROJECT_VERSION_TWEAK` CMake variable.
Patrick
