Patrick Steinhardt <ps@xxxxxx> writes:
> While at it, change the format of the version file by removing the
> spaces around the equals sign.
This may cause third-parties that act on the contents of the output
for their own purpose (read: may break distro scripts), but that is
part of packaging third-party software for a distro, so it may not
be considered a regression by them ;-)
> +if test "$#" -ne 3
> +then
> + echo "USAGE: $0 <SOURCE_DIR> <INPUT> <OUTPUT>" >&2
> + exit 1
> +fi
> +
> +SOURCE_DIR="$1"
> +INPUT="$2"
> +OUTPUT="$3"
> +
> +if ! test -f "$INPUT"
> +then
> + echo "Input is not a file: $INPUT" >&2
> + exit 1
> +fi
Just a taste thing, which does not warrant a reroll on its own, but
when redirecting to the standard error stream, writing >&2 early on
the command line is easier to signal that we are reading an error
message, e.g.
echo >&2 "This is an error message."
> +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.
> # 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...
> + VN=$(git -C "$SOURCE_DIR" describe --match "v[0-9]*" HEAD 2>/dev/null) &&
> case "$VN" in
> *$LF*) (exit 1) ;;
> v[0-9]*)
> - git update-index -q --refresh
> - test -z "$(git diff-index --name-only HEAD --)" ||
> + git -C "$SOURCE_DIR" update-index -q --refresh
> + test -z "$(git -C "$SOURCE_DIR" diff-index --name-only HEAD --)" ||
> VN="$VN-dirty" ;;
VN matching this case arm is a good sign that we have "git"
available, so not squelching "Command git not found" here is
acceptable, with or without this patch.
> esac
> then
> @@ -26,15 +44,31 @@ else
> VN="$DEF_VER"
> fi
>
> -VN=$(expr "$VN" : v*'\(.*\)')
> +GIT_BUILT_FROM_COMMIT=$(git -C "$SOURCE_DIR" rev-parse -q --verify HEAD 2>/dev/null)
The 2>/dev/null here is required and is good. Even if the
source-dir is git controlled, if we do not have an installed Git
available, we set GIT_BUILT_FROM_COMMIT to an empty string, as we
cannot tell which commit we are building from (yet).
> +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. 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".
> +GIT_VERSION=$(expr "$VN" : v*'\(.*\)')
I think VN is now set by the if/else cascade above to a string that
begins with 'v', as lnog as the maintainer makes sure that their
tags begin with 'v' (or a distro person prepares the 'version' file
to honor the convention), so this should be a safething to do.
> +if test -z "$GIT_USER_AGENT"
> +then
> + GIT_USER_AGENT=git/$GIT_VERSION
Not required by the language, but Documentation/CodingGuidelines
encourages you to dq the RHS of this assignment.
> +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.
> 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)?
Thanks.
