Re: [PATCH] doc: set actual revdate for manpages

From: Felipe Contreras <felipe.contreras@gmail.com>
Date: Apr 14, 2023, 18:26 UTC
Message-ID: <64399ac5685a4_5f77329495@chronos.notmuch>
In-Reply-To: <xmqqildys97o.fsf@gitster.g>

Junio C Hamano wrote:

Show 28 quoted lines

> Felipe Contreras <felipe.contreras@gmail.com> 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.

Show 23 quoted lines

> 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