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

[Felipe Contreras <felipe.contreras@gmail.com>]

Jeff King wrote:
> On Thu, Apr 13, 2023 at 01:47:22AM -0600, Felipe Contreras wrote:
> 
> > manpages expect the date of the last revision, if that is not found
> > DocBook Stylesheets go through a series of hacks to generate one with
> > the format `%d/%d/%Y` which is not ideal.
> > 
> > In addition to this format not being standard, different tools generate
> > dates with different formats.
> > 
> > There's no need for any confusion if we specify the revision date, so
> > let's do so.
> 
> That seems like a good goal, and should reduce our asciidoc/asciidoctor
> diff considerably.
> 
> > This patch requires [1] to actually work, and has a simple conflict with
> > [2], so it's written on top of both.
> > 
> > [1] https://lore.kernel.org/git/20230323221523.52472-1-felipe.contreras@gmail.com/
> > [2] https://lore.kernel.org/git/20230408001829.11031-1-felipe.contreras@gmail.com/
> 
> I wasted a bit of time trying this out, so let me elaborate on "actually
> work" for the benefit of other reviewers. Without the patch in [1]
> (which is 8806120de6c on fc/remove-header-workarounds-for-asciidoc),
> this patch works as advertised with asciidoctor, but has no effect with
> asciidoc.  The reason is that asciidoc puts the <date> tags in the
> header, and the custom header removed by 8806120de6c suppresses
> asciidoc's default header entirely (so a workaround would be to include
> the <date> tags in our custom header, but I don't see any reason not to
> just build on top of 8806120de6c, as you did here).

Yes, I also "wasted" a bit of time comparing the custom header introduced in
7ef195ba3e (Documentation: Add version information to man pages, 2007-03-25) to
what is in asciidoc and the difference is simple:

--- a/Documentation/asciidoc.conf
+++ b/Documentation/asciidoc.conf
@@ -56,6 +56,9 @@ ifdef::backend-docbook[]
 [header]
 template::[header-declarations]
 <refentry>
+<refentryinfo>
+template::[docinfo]
+</refentryinfo>
 <refmeta>
 <refentrytitle>{mantitle}</refentrytitle>
 <manvolnum>{manvolnum}</manvolnum>

This is of course not needed if we get rid of that custom header, but I sent
the patch anyway for reference in a semi-unrelated patch series [1].

But to me the rationale is simple: the information isn't there because we are
overriding the asciidoc header and not putting it there.

> > 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.

The perfect doesn't have to be the enemy of the good, and that can be done
later.

In the meantime something is better than nothing.

For the record, the GIT-VERSION-GEN script can be simplified a lot, I had a
patch from 2013 around, cleaned it up, and sent it a new series, so it should
be easier to implement this on top of that [2].

> diff --git a/Documentation/Makefile b/Documentation/Makefile
> index 2ccc3a9bc9..307634a94f 100644
> --- a/Documentation/Makefile
> +++ b/Documentation/Makefile
> @@ -144,8 +144,6 @@ man5dir = $(mandir)/man5
>  man7dir = $(mandir)/man7
>  # DESTDIR =
>  
> -GIT_DATE := $(shell git show --quiet --pretty='%as')
> -
>  ASCIIDOC = asciidoc
>  ASCIIDOC_EXTRA =
>  ASCIIDOC_HTML = xhtml11
> diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
> index 9a1111af9b..14903bd261 100755
> --- a/GIT-VERSION-GEN
> +++ b/GIT-VERSION-GEN
> @@ -10,7 +10,8 @@ LF='
>  # then try git-describe, then default.
>  if test -f version
>  then
> -	VN=$(cat version) || VN="$DEF_VER"
> +	VN=$(cut -d" " -f1 version) || VN="$DEF_VER"
> +	DN=$(cut -d" " -f2 version) || DN=""

Although this works, I have an issue with doing multiple unnecessary forks.

This does the same, no?

  read VN DN <version

Cheers.

[1] https://lore.kernel.org/git/20230413115745.116063-3-felipe.contreras@gmail.com/
[2] https://lore.kernel.org/git/20230414121841.373980-1-felipe.contreras@gmail.com/

-- 
Felipe Contreras