Junio C Hamano wrote:
> Felipe Contreras <felipe.contreras@xxxxxxxxx> writes:
>
> >> > diff --git a/Documentation/Makefile b/Documentation/Makefile
> >> > index 3133ea3182..b629176d7d 100644
> >> > --- a/Documentation/Makefile
> >> > +++ b/Documentation/Makefile
> >> > @@ -144,13 +144,16 @@ man5dir = $(mandir)/man5
> >> > man7dir = $(mandir)/man7
> >> > # DESTDIR =
> >> >
> >> > +GIT_DATE := $(shell git show --quiet --pretty='%as')
> >>
> >> What will/should this do in a distribution tarball, where we won't have
> >> a Git repository at all? I think we'll just end up with a blank date in
> >> the xml file, though it looks like docbook turns that into today's date
> >> in the result.
> >>
> >> That's not _too_ bad, but feels a bit inconsistent (and it uses the
> >> format you're trying to get rid of!).
> >>
> >> It would be nicer to populate the date variable in that case, like we do
> >> for GIT_VERSION. I think that could look something like this:
> >
> > Indeed, that would be better, but that totally can be done in a separate patch,
> > or a separate series even.
>
> Seeing Peff's change, it sounds so small a thing that it feels a bit
> unnatural not to do that in the same series, at least to me.
It should be a small thing, but the GIT-VERSION-GEN script has not been
updated since 2008 and has a lot of issues (from my point of view).
I think it should be cleaned up first (I already sent a patch series for
that), *then* it would be trivial to add another field.
But that would add another (unncessary) dependency to this series.
> Having said that, I think that "we make progress one step at a time"
> is perfectly acceptable and may even be preferred, as long as the
> formatted manpages from the tarball would not change between with
> and without this patch. That way, we make the output from a
> repository better while keeping the output from a tarball extract
> intact, and make the latter match the former in a later effort.
>
> So, I "wasted" (not really---this was a fruitful validation that is
> a part of a review process) some time to play with this on top of
> 'seen' to see how well the tarball extract fares. We get an error
> message from "git show" complaining about "not a git repository" but
> that is to be expected ("sh GIT-VERSION-GEN" does not share the
> problem, though).
>
> At least with the versions of toolchain picked by default in my
> environment, I seem to be getting identical "04/14/2023" in a
> directory extracted out of a tarball taken from 'seen' (with and
> without this patch) in the formatted manpages, because we do not
> have any record in the input either way.
>
> Formatted output from a repository working tree changes from
> "04/14/2023" to "2023-04-13". The value change may be intended, but
> I am not sure if the format change was intended or even welcome.
I live in the vast majority of countries where MM/DD/YYYY makes
absolutely no sense, so I think it's a plus if any man pages have a sane
date.
I'm pretty sure I'm not alone on this, but some might disagree.
> If we want to correct the date format, it can totally be done in a
> separate patch, or a separate series even, with some justification in
> the proposed log message, I think.
Sure, but in order to do that we would need a way to convert the sane
standard git dates to USA-centric dates, and I'm not so sure the
tidyness of having both dates wrong is worth it.
Would you consider something like this more welcome?
GIT_DATE := $(shell git show --quiet --pretty='%as' | sed -e 's|^\(.\+\)-\(.\+\)-\(.\+\)$|\2/\3/\1|'
--
Felipe Contreras
