On Sat, Dec 28, 2024 at 10:01 AM Simon Josefsson <[email protected]> wrote:
> Jim Meyering <[email protected]> writes:
>
> >> if test "x$v" = xUNKNOWN \
> >> && test -f ${tarball_version_file}-git \
> >> && head -1 ${tarball_version_file}-git \
> >> | grep -v '^$Format' > /dev/null 2>&1; then
> >> v=$(head -1 ${tarball_version_file}-git)
> >> fi
> >
> > That code uses "grep -v" where the intent must have been to use just "grep".
> > I've fixed that and cleaned up via this just-pushed change:
>
> Hi Jim. Thanks for review! No the intent is -v. The export-subst
> logic is backwards. The content of the file can only be used if it does
> NOT contain the '^$Format' keyword. If the file contained that keyword,
Sorry. I should have known.
How about this tiny adjustment, then?
- fmt=$(awk 'NR==1 && /^\$Format/ {print}' \
+ fmt=$(awk 'NR==1 && ! /^\$Format/ {print}' \
> it was not substituted by the git attribute export-subst, and the file
> content cannot be used as a version number since it is just the string
> '$Format:%(describe)$' which is not a version number. It is git-archive
> that replaces that keyword with a suitable version number.
>
> Meanwhile, I noticed a small problem with my patch: when the version
> number is taken from the .tarball-version-git file, it is not put
> through the same post-processing as a version number taken from the git
> command. This doesn't really matter, but it helps to have things
> consistent.
Yes, I like that part.
> How about this patch? Not pushed since it is untested.