[PATCH] asciidoctor-extensions: provide `<refmiscinfo/>`

[PATCH] asciidoctor-extensions: provide `<refmiscinfo/>`

[Martin Ågren]

When we build with AsciiDoc, asciidoc.conf ensures that each xml-file we
generate contains some meta-information which `xmlto` can act on, based
on the following template:

  <refmeta>
  <refentrytitle>{mantitle}</refentrytitle>
  <manvolnum>{manvolnum}</manvolnum>
  <refmiscinfo class="source">Git</refmiscinfo>
  <refmiscinfo class="version">{git_version}</refmiscinfo>
  <refmiscinfo class="manual">Git Manual</refmiscinfo>
  </refmeta>

When we build with Asciidoctor, it does not honor this configuration file
and we end up with only this (for a hypothetical git-foo.xml):

  <refmeta>
  <refentrytitle>git-foo</refentrytitle>
  <manvolnum>1</manvolnum>
  </refmeta>

That is, we miss out on the `<refmiscinfo/>` tags. As a result, the
header of each man page doesn't say "Git Manual", but "git-foo(1)"
instead. Worse, the footers don't give the Git version number and
instead provide the fairly ugly "[FIXME: source]".

That Asciidoctor ignores asciidoc.conf is nothing new. This is why we
implement the `linkgit:` macro in asciidoc.conf *and* in
asciidoctor-extensions.rb. Follow suit and provide these tags in
asciidoctor-extensions.rb, using a "postprocessor" extension.

We may consider a few alternatives:

  * Provide the `mansource` attribute to Asciidoctor. This attribute
    looks promising until one realizes that it can only be given inside
    the source file (the .txt file in our case), *not* on the command
    line using `-a mansource=foobar`. I toyed with the idea of injecting
    this attribute while feeding Asciidoctor the input on stdin, but it
    didn't feel like it was worth the complexity in the Makefile.

  * Similar to that last idea, we could inject these lines into the
    xml-files from the Makefile, e.g., using `sed`. This reduces
    repetition, but seems fairly brittle. It feels wrong to impose
    another step and another risk on the Asciidoc-processing only to
    benefit the Asciidoctor-one.

  * Considering the above abandoned ideas, it seems better to put any
    complexity inside asciidoctor-extensions.rb. It is after all
    supposed to be the "equivalent" of asciidoc.conf. I considered
    providing a "tree processor" extension and use it to set, e.g.,
    `mansource` mentioned above.

Let's instead try to stay as close as possible to what asciidoc.conf
does. We'll make it fairly obvious that we aim to inject the exact same
three lines of `<refmiscinfo/>` that asciidoc.conf provides. The only
somewhat tricky part is that we inject them *post*-processing so we need
to do the variable expansion ourselves.

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
---
 Cc Todd and Peff who had a brief exchange [1] a while ago. Apparently
 Todd saw this "[FIXME: source]" on Fedora but Peff did not on Debian.
 I'm on Ubuntu 18.04 and used to see this also on 16.04. I'm not sure
 what might make Debian so special here.

 [1] https://public-inbox.org/git/20180627164443.GK20217@zaya.teonanacatl.net/

 I've based this on ma/asciidoctor-fixes, not because it's needed for
 the patch itself, but because it provides a15ef383e7
 ("Documentation/Makefile: add missing dependency on
 asciidoctor-extensions", 2019-02-27), which ensures that the
 documentation will be rebuilt. I'm hoping this was the right call.

 Documentation/asciidoctor-extensions.rb | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/Documentation/asciidoctor-extensions.rb b/Documentation/asciidoctor-extensions.rb
index 0089e0cfb8..059279dee1 100644
--- a/Documentation/asciidoctor-extensions.rb
+++ b/Documentation/asciidoctor-extensions.rb
@@ -20,9 +20,25 @@ module Git
         end
       end
     end
+
+    class DocumentPostProcessor < Asciidoctor::Extensions::Postprocessor
+      def process document, output
+        if document.basebackend? 'docbook'
+          git_version = document.attributes['git_version']
+          replacement = "" \
+            "<refmiscinfo class=\"source\">Git</refmiscinfo>\n" \
+            "<refmiscinfo class=\"version\">#{git_version}</refmiscinfo>\n" \
+            "<refmiscinfo class=\"manual\">Git Manual</refmiscinfo>\n" \
+            "<\/refmeta>"
+          output = output.sub(/<\/refmeta>/, replacement)
+        end
+        output
+      end
+    end
   end
 end
 
 Asciidoctor::Extensions.register do
   inline_macro Git::Documentation::LinkGitProcessor, :linkgit
+  postprocessor Git::Documentation::DocumentPostProcessor
 end
-- 
2.21.0

Re: [PATCH] asciidoctor-extensions: provide `<refmiscinfo/>`

[Todd Zullinger]

Hi Martin,

Martin Ågren wrote:
> When we build with AsciiDoc, asciidoc.conf ensures that each xml-file we
> generate contains some meta-information which `xmlto` can act on, based
> on the following template:
> 
>   <refmeta>
>   <refentrytitle>{mantitle}</refentrytitle>
>   <manvolnum>{manvolnum}</manvolnum>
>   <refmiscinfo class="source">Git</refmiscinfo>
>   <refmiscinfo class="version">{git_version}</refmiscinfo>
>   <refmiscinfo class="manual">Git Manual</refmiscinfo>
>   </refmeta>
> 
> When we build with Asciidoctor, it does not honor this configuration file
> and we end up with only this (for a hypothetical git-foo.xml):
> 
>   <refmeta>
>   <refentrytitle>git-foo</refentrytitle>
>   <manvolnum>1</manvolnum>
>   </refmeta>
> 
> That is, we miss out on the `<refmiscinfo/>` tags. As a result, the
> header of each man page doesn't say "Git Manual", but "git-foo(1)"
> instead. Worse, the footers don't give the Git version number and
> instead provide the fairly ugly "[FIXME: source]".
> 
> That Asciidoctor ignores asciidoc.conf is nothing new. This is why we
> implement the `linkgit:` macro in asciidoc.conf *and* in
> asciidoctor-extensions.rb. Follow suit and provide these tags in
> asciidoctor-extensions.rb, using a "postprocessor" extension.

Nice!  I looked at this a long time ago and didn't figure
out how to use a postprocessor extension.  From my notes, I
found discussions about using ruby's tilt for templating and
it all seemed way too convoluted.

Your method looks quite simple and elegant.

> We may consider a few alternatives:
> 
>   * Provide the `mansource` attribute to Asciidoctor. This attribute
>     looks promising until one realizes that it can only be given inside
>     the source file (the .txt file in our case), *not* on the command
>     line using `-a mansource=foobar`. I toyed with the idea of injecting
>     this attribute while feeding Asciidoctor the input on stdin, but it
>     didn't feel like it was worth the complexity in the Makefile.

I played with this direction before.  Using Asciidoctor we
can convert directly from .txt to man without docbook
and xmlto.  That does have some other issues which need to
be worked out though.  Here's what I had as a start:

-- 8< --
Subject: [PATCH] WIP: doc: improve asciidoctor manpage generation

Avoid 'FIXME: Source' by setting mansource.  Skip xmlto step and render
manpages directly with asciidoctor.

TODO:
    - apply to all man pages
    - fix links to html docs, like user-manual.html in git.1 (currently
      it is listed in brackets inline rather than as a footnote)

Reference:
https://lore.kernel.org/lkml/20180424150456.17353-1-tiwai@suse.de/
Signed-off-by: Todd Zullinger <tmz@pobox.com>
---
 Documentation/Makefile                  | 8 ++++++++
 Documentation/asciidoctor-extensions.rb | 2 ++
 2 files changed, 10 insertions(+)

diff --git a/Documentation/Makefile b/Documentation/Makefile
index a9697f5146..494f8c9464 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -197,6 +197,7 @@ ASCIIDOC_DOCBOOK = docbook45
 ASCIIDOC_EXTRA += -acompat-mode -atabsize=8
 ASCIIDOC_EXTRA += -I. -rasciidoctor-extensions
 ASCIIDOC_EXTRA += -alitdd='&\#x2d;&\#x2d;'
+ASCIIDOC_EXTRA += -a mansource="Git $(GIT_VERSION)" -a manmanual="Git Manual"
 DBLATEX_COMMON =
 endif
 
@@ -354,9 +355,16 @@ $(OBSOLETE_HTML): %.html : %.txto asciidoc.conf
 manpage-base-url.xsl: manpage-base-url.xsl.in
 	$(QUIET_GEN)sed "s|@@MAN_BASE_URL@@|$(MAN_BASE_URL)|" $< > $@
 
+ifndef USE_ASCIIDOCTOR
 %.1 %.5 %.7 : %.xml manpage-base-url.xsl $(wildcard manpage*.xsl)
 	$(QUIET_XMLTO)$(RM) $@ && \
 	$(XMLTO) -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $<
+else
+%.1 %.5 %.7 : %.txt
+	$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
+	$(ASCIIDOC_COMMON) -b manpage -d manpage -o $@+ $< && \
+	mv $@+ $@
+endif
 
 %.xml : %.txt asciidoc.conf asciidoctor-extensions.rb
 	$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
diff --git a/Documentation/asciidoctor-extensions.rb b/Documentation/asciidoctor-extensions.rb
index f7a5982f8b..ebb078807a 100644
--- a/Documentation/asciidoctor-extensions.rb
+++ b/Documentation/asciidoctor-extensions.rb
@@ -12,6 +12,8 @@ module Git
         if parent.document.basebackend? 'html'
           prefix = parent.document.attr('git-relative-html-prefix')
           %(<a href="#{prefix}#{target}.html">#{target}(#{attrs[1]})</a>\n)
+        elsif parent.document.basebackend? 'manpage'
+          "#{target}(#{attrs[1]})"
         elsif parent.document.basebackend? 'docbook'
           "<citerefentry>\n" \
             "<refentrytitle>#{target}</refentrytitle>" \
-- 8< --

That was based on ma/asciidoctor-extensions, but it may be
missing other recent improvements you've made to the make
targets.  It's been a month or so since I worked on it.

I munged up doc-diff to set MANDWIDTH=1000 and set one
branch to default to asciidoctor to compare.  (Your other
recent series looks like it'll make doing asciidoc and
asciidoctor comparisons easier.)

There were a number of differences that I didn't work
through though.  Most importantly was the change in the
links noted in the commit message.

Thanks for working on asciidoctor.  I've been trying to poke
it off and on to help ensure the docs can be built if
asciidoc ever gets dropped from Fedora.

-- 
Todd

Re: [PATCH] asciidoctor-extensions: provide `<refmiscinfo/>`

[Martin Ågren]

Hi Todd,

On Sun, 17 Mar 2019 at 20:44, Todd Zullinger <tmz@pobox.com> wrote:
>
> Hi Martin,
>
> Martin Ågren wrote:
> > That is, we miss out on the `<refmiscinfo/>` tags. As a result, the
> > header of each man page doesn't say "Git Manual", but "git-foo(1)"
> > instead. Worse, the footers don't give the Git version number and
> > instead provide the fairly ugly "[FIXME: source]".
> >
> > That Asciidoctor ignores asciidoc.conf is nothing new. This is why we
> > implement the `linkgit:` macro in asciidoc.conf *and* in
> > asciidoctor-extensions.rb. Follow suit and provide these tags in
> > asciidoctor-extensions.rb, using a "postprocessor" extension.
>
> > We may consider a few alternatives:
> >
> >   * Provide the `mansource` attribute to Asciidoctor. This attribute
> >     looks promising until one realizes that it can only be given inside
> >     the source file (the .txt file in our case), *not* on the command
> >     line using `-a mansource=foobar`. I toyed with the idea of injecting
> >     this attribute while feeding Asciidoctor the input on stdin, but it
> >     didn't feel like it was worth the complexity in the Makefile.
>
> I played with this direction before.  Using Asciidoctor we
> can convert directly from .txt to man without docbook
> and xmlto.  That does have some other issues which need to
> be worked out though.  Here's what I had as a start:
>
> -- 8< --
> Subject: [PATCH] WIP: doc: improve asciidoctor manpage generation
>
> Avoid 'FIXME: Source' by setting mansource.  Skip xmlto step and render
> manpages directly with asciidoctor.
>
> TODO:
>     - apply to all man pages
>     - fix links to html docs, like user-manual.html in git.1 (currently
>       it is listed in brackets inline rather than as a footnote)
>
> Reference:
> https://lore.kernel.org/lkml/20180424150456.17353-1-tiwai@suse.de/
> Signed-off-by: Todd Zullinger <tmz@pobox.com>
> ---
>  Documentation/Makefile                  | 8 ++++++++
>  Documentation/asciidoctor-extensions.rb | 2 ++
>  2 files changed, 10 insertions(+)
>
> diff --git a/Documentation/Makefile b/Documentation/Makefile
> index a9697f5146..494f8c9464 100644
> --- a/Documentation/Makefile
> +++ b/Documentation/Makefile
> @@ -197,6 +197,7 @@ ASCIIDOC_DOCBOOK = docbook45
>  ASCIIDOC_EXTRA += -acompat-mode -atabsize=8
>  ASCIIDOC_EXTRA += -I. -rasciidoctor-extensions
>  ASCIIDOC_EXTRA += -alitdd='&\#x2d;&\#x2d;'
> +ASCIIDOC_EXTRA += -a mansource="Git $(GIT_VERSION)" -a manmanual="Git Manual"
>  DBLATEX_COMMON =
>  endif
>
> @@ -354,9 +355,16 @@ $(OBSOLETE_HTML): %.html : %.txto asciidoc.conf
>  manpage-base-url.xsl: manpage-base-url.xsl.in
>         $(QUIET_GEN)sed "s|@@MAN_BASE_URL@@|$(MAN_BASE_URL)|" $< > $@
>
> +ifndef USE_ASCIIDOCTOR
>  %.1 %.5 %.7 : %.xml manpage-base-url.xsl $(wildcard manpage*.xsl)
>         $(QUIET_XMLTO)$(RM) $@ && \
>         $(XMLTO) -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $<
> +else
> +%.1 %.5 %.7 : %.txt
> +       $(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
> +       $(ASCIIDOC_COMMON) -b manpage -d manpage -o $@+ $< && \
> +       mv $@+ $@
> +endif
>
>  %.xml : %.txt asciidoc.conf asciidoctor-extensions.rb
>         $(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
> diff --git a/Documentation/asciidoctor-extensions.rb b/Documentation/asciidoctor-extensions.rb
> index f7a5982f8b..ebb078807a 100644
> --- a/Documentation/asciidoctor-extensions.rb
> +++ b/Documentation/asciidoctor-extensions.rb
> @@ -12,6 +12,8 @@ module Git
>          if parent.document.basebackend? 'html'
>            prefix = parent.document.attr('git-relative-html-prefix')
>            %(<a href="#{prefix}#{target}.html">#{target}(#{attrs[1]})</a>\n)
> +        elsif parent.document.basebackend? 'manpage'
> +          "#{target}(#{attrs[1]})"
>          elsif parent.document.basebackend? 'docbook'
>            "<citerefentry>\n" \
>              "<refentrytitle>#{target}</refentrytitle>" \
> -- 8< --
>
> That was based on ma/asciidoctor-extensions, but it may be
> missing other recent improvements you've made to the make
> targets.  It's been a month or so since I worked on it.
>
> I munged up doc-diff to set MANDWIDTH=1000 and set one
> branch to default to asciidoctor to compare.  (Your other
> recent series looks like it'll make doing asciidoc and
> asciidoctor comparisons easier.)
>
> There were a number of differences that I didn't work
> through though.  Most importantly was the change in the
> links noted in the commit message.

Your approach looks like the correct one long term, at least.

I'll try to play with this to see if I can figure out the differences.
That will have to wait until tomorrow though.

Thanks for sharing your progress.

Martin

Re: [PATCH] asciidoctor-extensions: provide `<refmiscinfo/>`

[Martin Ågren]

On Sun, 17 Mar 2019 at 20:44, Todd Zullinger <tmz@pobox.com> wrote:
> Martin Ågren wrote:
> >   * Provide the `mansource` attribute to Asciidoctor. This attribute
> >     looks promising until one realizes that it can only be given inside
> >     the source file (the .txt file in our case), *not* on the command
> >     line using `-a mansource=foobar`. I toyed with the idea of injecting
> >     this attribute while feeding Asciidoctor the input on stdin, but it
> >     didn't feel like it was worth the complexity in the Makefile.
>
> I played with this direction before.  Using Asciidoctor we
> can convert directly from .txt to man without docbook
> and xmlto.  That does have some other issues which need to
> be worked out though.  Here's what I had as a start:

> +ASCIIDOC_EXTRA += -a mansource="Git $(GIT_VERSION)" -a manmanual="Git Manual"

So to be honest, I still don't understand how this works, but it does,
great. I really need to improve my documentation-reading skills.

> I munged up doc-diff to set MANDWIDTH=1000 and set one
> branch to default to asciidoctor to compare.  (Your other
> recent series looks like it'll make doing asciidoc and
> asciidoctor comparisons easier.)
>
> There were a number of differences that I didn't work
> through though.  Most importantly was the change in the
> links noted in the commit message.

I had some more time to look at this. Thanks for getting started on this
switch. A few things I noticed:

{litdd} now renders as &#x2d;&#x2d. We should find some other way to
produce '--'. This should then be a simple change, as we're already
providing this attribute inside an `ifdef USE_ASCIIDOCTOR`.

"+" becomes "&#43;". I didn't immediately find where we do that.

Backticks should result in monospace.

`./doc-diff HEAD^ HEAD` shows how several "git-\nfoo" become
"\ngit-foo", i.e., linkgit expansions are now treated as non-breaking.
That's arguably good, but it brings some noise to the diff. Maybe one
should try and see if it's possible to break that to have a nicer
diff, then remove that breakage in a follow-up commit. Or, if it's
possible to make "git-foo" non-breaking before the switch. Hmm, was this
why you increased MANWIDTH?

A double-space between sentences turns into a single space -- at least
in constructions such as "... to foobar. `git-foo` does ...". Not a
problem perhaps, but noise in the diff.

And I'm sure there's more lurking in that huge diff. Whether any of that
is significant or not is another matter. ;-)


Martin

Re: [PATCH] asciidoctor-extensions: provide `<refmiscinfo/>`

[Todd Zullinger]

Hi,

Martin Ågren wrote:
> On Sun, 17 Mar 2019 at 20:44, Todd Zullinger <tmz@pobox.com> wrote:
>> Martin Ågren wrote:
>> +ASCIIDOC_EXTRA += -a mansource="Git $(GIT_VERSION)" -a manmanual="Git Manual"
> 
> So to be honest, I still don't understand how this works, but it does,
> great. I really need to improve my documentation-reading skills.

Let me know if you find any good methods for perfect
retention.  I've re-read enough documentation for a
lifetime. ;)

> I had some more time to look at this. Thanks for getting started on this
> switch. A few things I noticed:
> 
> {litdd} now renders as &#x2d;&#x2d. We should find some other way to
> produce '--'. This should then be a simple change, as we're already
> providing this attribute inside an `ifdef USE_ASCIIDOCTOR`.

I noticed that one and didn't work out a good fix, but it
sounds like you have one in mind.  That's great.

> "+" becomes "&#43;". I didn't immediately find where we do that.

For this one, I was working on replacing "{plus}" with `+`
(along with " " and "-").  That's probably not ideal though.

This is what I had to test:

-- 8< --
Subject: [PATCH] WIP: wrap "[ +-]" in backticks (NOT FOR SUBMISSION)

asciidoctor's manpage output html-encodes "{plus}" which seems like a
bug.  At the least it needs some option I've yet to learn.
---
 Documentation/git-add.txt | 26 +++++++++++++-------------
 Documentation/gitweb.txt  |  6 +++---
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index 37bcab94d5..dc1dd3a91b 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -363,20 +363,20 @@ may see in a patch, and which editing operations make sense on them.
 --
 added content::
 
-Added content is represented by lines beginning with "{plus}". You can
+Added content is represented by lines beginning with `"+"`. You can
 prevent staging any addition lines by deleting them.
 
 removed content::
 
-Removed content is represented by lines beginning with "-". You can
-prevent staging their removal by converting the "-" to a " " (space).
+Removed content is represented by lines beginning with `"-"`. You can
+prevent staging their removal by converting the `"-"` to a `" "` (space).
 
 modified content::
 
-Modified content is represented by "-" lines (removing the old content)
-followed by "{plus}" lines (adding the replacement content). You can
-prevent staging the modification by converting "-" lines to " ", and
-removing "{plus}" lines. Beware that modifying only half of the pair is
+Modified content is represented by `"-"` lines (removing the old content)
+followed by `"+"` lines (adding the replacement content). You can
+prevent staging the modification by converting `"-"` lines to `" "`, and
+removing `"+"` lines. Beware that modifying only half of the pair is
 likely to introduce confusing changes to the index.
 --
 
@@ -393,29 +393,29 @@ Avoid using these constructs, or do so with extreme caution.
 removing untouched content::
 
 Content which does not differ between the index and working tree may be
-shown on context lines, beginning with a " " (space).  You can stage
-context lines for removal by converting the space to a "-". The
+shown on context lines, beginning with a `" "` (space).  You can stage
+context lines for removal by converting the space to a `"-"`. The
 resulting working tree file will appear to re-add the content.
 
 modifying existing content::
 
 One can also modify context lines by staging them for removal (by
-converting " " to "-") and adding a "{plus}" line with the new content.
-Similarly, one can modify "{plus}" lines for existing additions or
+converting `" "` to `"-"`) and adding a `"+"` line with the new content.
+Similarly, one can modify `"+"` lines for existing additions or
 modifications. In all cases, the new modification will appear reverted
 in the working tree.
 
 new content::
 
 You may also add new content that does not exist in the patch; simply
-add new lines, each starting with "{plus}". The addition will appear
+add new lines, each starting with `"+"`. The addition will appear
 reverted in the working tree.
 --
 
 There are also several operations which should be avoided entirely, as
 they will make the patch impossible to apply:
 
-* adding context (" ") or removal ("-") lines
+* adding context (`" "`) or removal (`"-"`) lines
 * deleting context or removal lines
 * modifying the contents of context or removal lines
 
diff --git a/Documentation/gitweb.txt b/Documentation/gitweb.txt
index 88450589af..d27f239242 100644
--- a/Documentation/gitweb.txt
+++ b/Documentation/gitweb.txt
@@ -80,15 +80,15 @@ continuation (newline escaping).
 * Leading and trailing whitespace are ignored.
 
 * Whitespace separated fields; any run of whitespace can be used as field
-separator (rules for Perl's "`split(" ", $line)`").
+separator (rules for Perl's "`split(`" "`, $line)`").
 
 * Fields use modified URI encoding, defined in RFC 3986, section 2.1
 (Percent-Encoding), or rather "Query string encoding" (see
 https://en.wikipedia.org/wiki/Query_string#URL_encoding[]), the difference
-being that SP (" ") can be encoded as "{plus}" (and therefore "{plus}" has to be
+being that SP (`" "`) can be encoded as `"+"` (and therefore `"+"` has to be
 also percent-encoded).
 +
-Reserved characters are: "%" (used for encoding), "{plus}" (can be used to
+Reserved characters are: "%" (used for encoding), `"+"` (can be used to
 encode SPACE), all whitespace characters as defined in Perl, including SP,
 TAB and LF, (used to separate fields in a record).

-- 8< --

> `./doc-diff HEAD^ HEAD` shows how several "git-\nfoo" become
> "\ngit-foo", i.e., linkgit expansions are now treated as non-breaking.
> That's arguably good, but it brings some noise to the diff. Maybe one
> should try and see if it's possible to break that to have a nicer
> diff, then remove that breakage in a follow-up commit. Or, if it's
> possible to make "git-foo" non-breaking before the switch. Hmm, was this
> why you increased MANWIDTH?

Yeah, I noticed a number of places where asciidoc and
asciidoctor wrapped lines at slightly different places.  I
didn't see if they were all due to wrapping at a dashed git
command, but that could certainly have been the main cause.

I set the large MANWIDTH and then I think I added
--color-words to the diff call in doc-diff.

> A double-space between sentences turns into a single space -- at least
> in constructions such as "... to foobar. `git-foo` does ...". Not a
> problem perhaps, but noise in the diff.
> 
> And I'm sure there's more lurking in that huge diff. Whether any of that
> is significant or not is another matter. ;-)

Oh my yes, I'm sure. :)

-- 
Todd

Re: [PATCH] asciidoctor-extensions: provide `<refmiscinfo/>`

[Martin Ågren]

On Wed, 20 Mar 2019 at 19:17, Todd Zullinger <tmz@pobox.com> wrote:
> Martin Ågren wrote:
> > {litdd} now renders as &#x2d;&#x2d. We should find some other way to
> > produce '--'. This should then be a simple change, as we're already
> > providing this attribute inside an `ifdef USE_ASCIIDOCTOR`.
>
> I noticed that one and didn't work out a good fix, but it
> sounds like you have one in mind.  That's great.
>
> > "+" becomes "&#43;". I didn't immediately find where we do that.
>
> For this one, I was working on replacing "{plus}" with `+`
> (along with " " and "-").  That's probably not ideal though.

The "plus" and "litdd" issues seem like they can be solved by doing:

  ASCIIDOC_EXTRA += -aplus='+'
  ASCIIDOC_EXTRA += -alitdd='\--'

> > `./doc-diff HEAD^ HEAD` shows how several "git-\nfoo" become
> > "\ngit-foo", i.e., linkgit expansions are now treated as non-breaking.
> > [...] Hmm, was this
> > why you increased MANWIDTH?
>
> Yeah, I noticed a number of places where asciidoc and
> asciidoctor wrapped lines at slightly different places.  I
> didn't see if they were all due to wrapping at a dashed git
> command, but that could certainly have been the main cause.
>
> I set the large MANWIDTH and then I think I added
> --color-words to the diff call in doc-diff.
>
> > A double-space between sentences turns into a single space -- at least
> > in constructions such as "... to foobar. `git-foo` does ...". Not a
> > problem perhaps, but noise in the diff.
> >
> > And I'm sure there's more lurking in that huge diff. Whether any of that
> > is significant or not is another matter. ;-)
>
> Oh my yes, I'm sure. :)

:)

Using origin/ma/doc-diff-doc-vs-doctor-comparison, you can do something
like

  ./doc-diff --asciidoctor --cut-header-footer foo bar --
--ignore-space-change --word-diff

to reduce the amount of noise in the diff by a great deal. Going through
the result, I noted two issues which should be fixed by Asciidoctor
1.5.7, according to some initial searching (but I haven't tested):

"--" renders as an em dash followed by a ";". Looks like it's
https://github.com/asciidoctor/asciidoctor/issues/2604

"&" turns into "&amp;". Seems to be known and fixed:
https://github.com/asciidoctor/asciidoctor/issues/2525

Plus a few other problems. There's the odd problem here and there, but
most of the diff seems to be a large number of occurences of just a few
issues.

It would probably be worthwhile to try 1.5.7+ to see how much that
improves things. Seems like you're already underway there.

Martin

Re: [PATCH] asciidoctor-extensions: provide `<refmiscinfo/>`

[Todd Zullinger]

Hi,

Martin Ågren wrote:
> On Wed, 20 Mar 2019 at 19:17, Todd Zullinger <tmz@pobox.com> wrote:
>> Martin Ågren wrote:
>>> {litdd} now renders as &#x2d;&#x2d. We should find some other way to
>>> produce '--'. This should then be a simple change, as we're already
>>> providing this attribute inside an `ifdef USE_ASCIIDOCTOR`.
>>
>> I noticed that one and didn't work out a good fix, but it
>> sounds like you have one in mind.  That's great.
>>
>>> "+" becomes "&#43;". I didn't immediately find where we do that.
>>
>> For this one, I was working on replacing "{plus}" with `+`
>> (along with " " and "-").  That's probably not ideal though.
> 
> The "plus" and "litdd" issues seem like they can be solved by doing:
> 
>   ASCIIDOC_EXTRA += -aplus='+'
>   ASCIIDOC_EXTRA += -alitdd='\--'

Hmm, setting litdd makes the html generate an em dash
followed by a zero width space (in 1.5.8 and 2.0.0)

I updated my system to asciidoctor-2.0.0 last night and now
I can't even generate the man pages properly, because the
docbook45 converter was removed.  I'll have to see if I
missed some other required update. :/

> It would probably be worthwhile to try 1.5.7+ to see how much that
> improves things. Seems like you're already underway there.

Yeah, before I knew how soon 2.0.0 was coming, I updated to
1.5.8 and built the various Fedora packages which require it
to see how they handled it.  Almost all of the changes were
fixes to bugs in previous versions.  And the one which was
not is likely to be fixed in 2.0.0 according to asciidoctor
maintainer Dan Allen.

Have you looked at diffing the html output as well?  It
seems like we'll need to check it as well to be sure any
fixes to the manpage output don't have a negative impact on
the html output, and vice versa.

I used 'links -dump' output for comparison of the various
Fedora packages which currently build with asciidoctor.
It's not perfect.  It could miss visual changes which might
be important when viewed in a graphical browser.  But it was
better than trying to diff the html files directly. :)

We probably also want to check the validity of links within
the docs, as one of the changes in 1.5.8 caused breakage of
cross interdocument references.  (This is the change I noted
above which should be fixed in 2.0.0.)

It'll be quite a while until most systems with asciidoctor
1.5.x are gone.  I doubt that upgrading to even 1.5.8 is
suitable for the currently released Fedora versions due to
incompatible changes.  I am going to try and get 2.0.0 into
Fedora 30, but the deadline for changes has just passed, so
I may not be able to do so.  If not, it'll be 6-8 months
until a released version of Fedora has an asciidoctor newer
than 1.5.6.1.

All that is just to say that even if newer asciidoctor fixes
many of the issues we've seen it will still be a long time
before we can reasonable default to asciidoctor or drop
asciidoc support.

For what it's worth, the Fedora asciidoc packages moved to
python3 using https://github.com/asciidoc/asciidoc-py3.
That's not very active, but it should at least keep asciidoc
alive beyond the approaching python2 EOL.

-- 
Todd

Re: [PATCH] asciidoctor-extensions: provide `<refmiscinfo/>`

[Jeff King]

On Sat, Mar 23, 2019 at 03:27:56PM -0400, Todd Zullinger wrote:

> I updated my system to asciidoctor-2.0.0 last night and now
> I can't even generate the man pages properly, because the
> docbook45 converter was removed.  I'll have to see if I
> missed some other required update. :/

I ran into that, too. Using ASCIIDOC_DOCBOOK=docbook5 worked. The output
looked OK to me, but I only eyeballed it briefly. It's possible there
are subtle problems.

-Peff

Re: [PATCH] asciidoctor-extensions: provide `<refmiscinfo/>`

[Todd Zullinger]

Hi,

Jeff King wrote:
> On Sat, Mar 23, 2019 at 03:27:56PM -0400, Todd Zullinger wrote:
> 
>> I updated my system to asciidoctor-2.0.0 last night and now
>> I can't even generate the man pages properly, because the
>> docbook45 converter was removed.  I'll have to see if I
>> missed some other required update. :/
> 
> I ran into that, too. Using ASCIIDOC_DOCBOOK=docbook5 worked. The output
> looked OK to me, but I only eyeballed it briefly. It's possible there
> are subtle problems.

Interesting.  I did that last night as well, but it only led
to more errors.  I'm curious why manpage builds work for you
and not for me.

On my fedora 29 test system, ASCIIDOC_DOCBOOK=docbook5 leads
to a validation failure from xmlto, since docbook5 doesn't
use a DTD¹.  I added XMLTO_EXTRA = --skip-validation to the
USE_ASCIIDOCTOR block, which can build many of the man
pages, but fails when it gets to git-blame due to use of
literal < > characters in the xml:

    git-blame.xml:423: parser error : StartTag: invalid element name
    <literallayout class="monospaced"><40-byte hex sha1> <sourceline> <resultline> <
                                       ^

While this may be a real issue in the formatting of
git-blame.txt which we should fix, I'm suspicious of that
due to the number of other files similarly affected:

    git-blame.txt
    git-diff-files.txt
    git-diff-index.txt
    git-diff-tree.txt
    git-diff.txt
    git-format-patch.txt
    git-http-fetch.txt
    git-log.txt
    git-rebase.txt
    git-rev-list.txt
    git-show.txt
    git-svn.txt

¹ Here's the failure I get without --skip-validation:
[tmz@f29 Documentation]$ make USE_ASCIIDOCTOR=1 git.1
    SUBDIR ../
make[1]: 'GIT-VERSION-FILE' is up to date.
    ASCIIDOC git.xml
    XMLTO git.1
xmlto: /home/tmz/src/git/Documentation/git.xml does not validate (status 3)
xmlto: Fix document syntax or use --skip-validation option
validity error : no DTD found!
Document /home/tmz/src/git/Documentation/git.xml does not validate
make: *** [Makefile:359: git.1] Error 13

Thanks,

-- 
Todd

Re: [PATCH] asciidoctor-extensions: provide `<refmiscinfo/>`

[Jeff King]

On Sun, Mar 24, 2019 at 12:21:31PM -0400, Todd Zullinger wrote:

> > I ran into that, too. Using ASCIIDOC_DOCBOOK=docbook5 worked. The output
> > looked OK to me, but I only eyeballed it briefly. It's possible there
> > are subtle problems.
> 
> Interesting.  I did that last night as well, but it only led
> to more errors.  I'm curious why manpage builds work for you
> and not for me.

I think it's because I'm an idiot. I must have only been using 2.0.0
when I was looking at the XML output manually (for the refmiscinfo
lines), and never actually rendered it to roff. I get the same problem
when I try a full build.

> On my fedora 29 test system, ASCIIDOC_DOCBOOK=docbook5 leads
> to a validation failure from xmlto, since docbook5 doesn't
> use a DTD¹.  I added XMLTO_EXTRA = --skip-validation to the
> USE_ASCIIDOCTOR block, which can build many of the man
> pages, but fails when it gets to git-blame due to use of
> literal < > characters in the xml:
> 
>     git-blame.xml:423: parser error : StartTag: invalid element name
>     <literallayout class="monospaced"><40-byte hex sha1> <sourceline> <resultline> <
>                                        ^

That seems like a bug in asciidoctor, which ought to be quoting the "<".
We certainly can't quote it ourselves (we don't even know that our
backend output is going to a format that needs angle brackets quoted).

-Peff

Re: [PATCH] asciidoctor-extensions: provide `<refmiscinfo/>`

[Todd Zullinger]

Hi,

Jeff King wrote:
> On Sun, Mar 24, 2019 at 12:21:31PM -0400, Todd Zullinger wrote:
>> I'm curious why manpage builds work for you and not for me.
> 
> I think it's because I'm an idiot. I must have only been using 2.0.0
> when I was looking at the XML output manually (for the refmiscinfo
> lines), and never actually rendered it to roff. I get the same problem
> when I try a full build.

Ahh.  I was hoping you'd tell me that I was a fool. :)

>> On my fedora 29 test system, ASCIIDOC_DOCBOOK=docbook5 leads
>> to a validation failure from xmlto, since docbook5 doesn't
>> use a DTD¹.  I added XMLTO_EXTRA = --skip-validation to the
>> USE_ASCIIDOCTOR block, which can build many of the man
>> pages, but fails when it gets to git-blame due to use of
>> literal < > characters in the xml:
>> 
>>     git-blame.xml:423: parser error : StartTag: invalid element name
>>     <literallayout class="monospaced"><40-byte hex sha1> <sourceline> <resultline> <
>>                                        ^
> 
> That seems like a bug in asciidoctor, which ought to be quoting the "<".
> We certainly can't quote it ourselves (we don't even know that our
> backend output is going to a format that needs angle brackets quoted).

Yep, it seems so.  I filed this upstream:

https://github.com/asciidoctor/asciidoctor/issues/3205

I updated to asciidoctor-2.0.1 this morning to test, in case
it was one of the issues fixed since the 2.0.0 release.
Alas, we're the first to hit it and report it.

-- 
Todd

Re: [PATCH] asciidoctor-extensions: provide `<refmiscinfo/>`

[Todd Zullinger]

Hi,

I wrote:
> Jeff King wrote:
>> That seems like a bug in asciidoctor, which ought to be quoting the "<".
>> We certainly can't quote it ourselves (we don't even know that our
>> backend output is going to a format that needs angle brackets quoted).
> 
> Yep, it seems so.  I filed this upstream:
> 
> https://github.com/asciidoctor/asciidoctor/issues/3205
> 
> I updated to asciidoctor-2.0.1 this morning to test, in case
> it was one of the issues fixed since the 2.0.0 release.
> Alas, we're the first to hit it and report it.

Dan Allen fixed this upstream and released 2.0.2 today.
It's very good to know that asciidoctor upstream is
incredibly responsive.  If anyone runs into Dan at a
conference, please buy him a beer. ;)

There's still the matter of 2.0 dropping docbook45.  I'll
try to get around to testing 1.5.x releases with docbook5 to
see if they work reasonably well.  If not, we can add a
version check and set ASCIIDOC_DOCBOOK appropriately.

One other issue that crops up with docbook5 is that the
xmlto toolchain now outputs:

    Note: namesp. cut : stripped namespace before processing

as documented at

    https://docbook.org/docs/howto/howto.html#dbxsl

It's mostly an annoyance which we either want to strip out,
or pass an alternate docbook5 xsl, I think.  But I'm not
very familiar with the guts of the xml/docbook toolchains.

-- 
Todd

Re: [PATCH] asciidoctor-extensions: provide `<refmiscinfo/>`

[SZEDER Gábor]

On Tue, Mar 26, 2019 at 09:06:03PM -0400, Todd Zullinger wrote:
> Dan Allen fixed this upstream and released 2.0.2 today.
> It's very good to know that asciidoctor upstream is
> incredibly responsive.  If anyone runs into Dan at a
> conference, please buy him a beer. ;)

I noticed the "Release beer" lines in the Asciidoctor relnotes... :)

> One other issue that crops up with docbook5 is that the
> xmlto toolchain now outputs:
> 
>     Note: namesp. cut : stripped namespace before processing
> 
> as documented at
> 
>     https://docbook.org/docs/howto/howto.html#dbxsl
> 
> It's mostly an annoyance which we either want to strip out,
> or pass an alternate docbook5 xsl, I think.  But I'm not
> very familiar with the guts of the xml/docbook toolchains.

In our documentation CI build jobs we check that nothing is written to
Asciidoc/Asciidoctor's standard error [1].  These "Note:" lines go to
stderr, and will trigger that check and cause the build to fail.  So
wither we should find a way to silence these notes, or filter them out
in the CI builds.


[1] 505ad91304 (travis-ci: check AsciiDoc/AsciiDoctor stderr output,
    2017-04-26), though it never actually worked as intended, but that
    is about to get fixed:

    https://public-inbox.org/git/20190324215534.9495-7-szeder.dev@gmail.com/

Re: [PATCH] asciidoctor-extensions: provide `<refmiscinfo/>`

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 1854 bytes --]

On Tue, Mar 26, 2019 at 09:06:03PM -0400, Todd Zullinger wrote:
> There's still the matter of 2.0 dropping docbook45.  I'll
> try to get around to testing 1.5.x releases with docbook5 to
> see if they work reasonably well.  If not, we can add a
> version check and set ASCIIDOC_DOCBOOK appropriately.
> 
> One other issue that crops up with docbook5 is that the
> xmlto toolchain now outputs:
> 
>     Note: namesp. cut : stripped namespace before processing
> 
> as documented at
> 
>     https://docbook.org/docs/howto/howto.html#dbxsl
> 
> It's mostly an annoyance which we either want to strip out,
> or pass an alternate docbook5 xsl, I think.  But I'm not
> very familiar with the guts of the xml/docbook toolchains.

I'm quite familiar with this area, because I used DocBook and its
stylesheets for my college papers. There are two sets of stylesheets,
the namespaced ones (for DocBook 5) and the non-namespaced ones (for
DocBook 4). They can be distinguished because the URLs (and typically
the paths) use "xsl" (for non-namespaced) or "xsl-ns" (for namespaced).

xmlto by default uses the older ones, and assuming you have a reasonably
capable XSLT processor, either set of stylesheets can be used, since
they simply transform the document into the right type (although with a
warning, as you've noticed).

Unfortunately, xmlto is hard-coded to use the non-namespaced stylesheets
with no option to specify which to use, and it doesn't appear to be
actively developed. There's an option to specify the stylesheet (-x),
but it can't take a URL, since xmlto "helpfully" fully qualifies the
path. That means we'd need to know the location on disk of those
stylesheets in order to use them, since we can't rely on catalog
resolution.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]

Re: [PATCH] asciidoctor-extensions: provide `<refmiscinfo/>`

[Todd Zullinger]

Hi,

brian m. carlson wrote:
> On Tue, Mar 26, 2019 at 09:06:03PM -0400, Todd Zullinger wrote:
>> There's still the matter of 2.0 dropping docbook45.  I'll
>> try to get around to testing 1.5.x releases with docbook5 to
>> see if they work reasonably well.  If not, we can add a
>> version check and set ASCIIDOC_DOCBOOK appropriately.
>> 
>> One other issue that crops up with docbook5 is that the
>> xmlto toolchain now outputs:
>> 
>>     Note: namesp. cut : stripped namespace before processing
>> 
>> as documented at
>> 
>>     https://docbook.org/docs/howto/howto.html#dbxsl
>> 
>> It's mostly an annoyance which we either want to strip out,
>> or pass an alternate docbook5 xsl, I think.  But I'm not
>> very familiar with the guts of the xml/docbook toolchains.
> 
> I'm quite familiar with this area, because I used DocBook and its
> stylesheets for my college papers. There are two sets of stylesheets,
> the namespaced ones (for DocBook 5) and the non-namespaced ones (for
> DocBook 4). They can be distinguished because the URLs (and typically
> the paths) use "xsl" (for non-namespaced) or "xsl-ns" (for namespaced).
> 
> xmlto by default uses the older ones, and assuming you have a reasonably
> capable XSLT processor, either set of stylesheets can be used, since
> they simply transform the document into the right type (although with a
> warning, as you've noticed).
> 
> Unfortunately, xmlto is hard-coded to use the non-namespaced stylesheets
> with no option to specify which to use, and it doesn't appear to be
> actively developed. There's an option to specify the stylesheet (-x),
> but it can't take a URL, since xmlto "helpfully" fully qualifies the
> path. That means we'd need to know the location on disk of those
> stylesheets in order to use them, since we can't rely on catalog
> resolution.

Thanks for the very useful docbook5/xmlto details!

The hard-coded use of the non-namespaced stylesheets in
xmlto is unfortunate.  I hadn't gotten far enough along to
run into that limitation of xmlto, so many thanks for saving
me from finding it myself.  I'm sure it would have taken me
far longer.

If it turns out that docbook5 causes us more pain than it's
worth, I suppose we could choose to use the builtin manpage
backend when using asciidoctor >= 2.

Or we could see about adding a docbook45 converter, which
upstream noted as a possibility in the ticket¹ which dropped
docbook45 by default.

It'll be some time before we can reasonably expect to only
support asciidoctor-2.x.  We'll have to see what method
involves the least ugly hacks to support asciidoc and the
various 1.5.x and 2.x versions of asciidoctor.

¹ https://github.com/asciidoctor/asciidoctor/issues/3005

-- 
Todd

Re: [PATCH] asciidoctor-extensions: provide `<refmiscinfo/>`

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 1556 bytes --]

On Sat, Mar 30, 2019 at 02:00:14PM -0400, Todd Zullinger wrote:
> Thanks for the very useful docbook5/xmlto details!
> 
> The hard-coded use of the non-namespaced stylesheets in
> xmlto is unfortunate.  I hadn't gotten far enough along to
> run into that limitation of xmlto, so many thanks for saving
> me from finding it myself.  I'm sure it would have taken me
> far longer.
> 
> If it turns out that docbook5 causes us more pain than it's
> worth, I suppose we could choose to use the builtin manpage
> backend when using asciidoctor >= 2.

I suspect this will be the easiest way forward.  If we produce
semantically equivalent manpages, then this is also a lot nicer for
people who would prefer not to have a full XML toolchain installed.

> Or we could see about adding a docbook45 converter, which
> upstream noted as a possibility in the ticket¹ which dropped
> docbook45 by default.

Another possibility, depending on how responsive the xmlto upstream is,
is to add some sort of DocBook 5 support to it. This shouldn't be
terribly difficult, although we'd have to disable validation. DocBook 5
uses RELAX NG, and libxml2/xmllint's support for this has some bugs,
such that it will mark some valid documents using complex interleaves as
invalid. Still, if this is the direction we want to go, I have no
problem adding this support.

Since we'd have a quite new Asciidoctor and a new xmlto, distros should
have both around the same time.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]

Re: [PATCH] asciidoctor-extensions: provide `<refmiscinfo/>`

[Todd Zullinger]

brian m. carlson wrote:
> On Sat, Mar 30, 2019 at 02:00:14PM -0400, Todd Zullinger wrote:
>> Thanks for the very useful docbook5/xmlto details!
>> 
>> The hard-coded use of the non-namespaced stylesheets in
>> xmlto is unfortunate.  I hadn't gotten far enough along to
>> run into that limitation of xmlto, so many thanks for saving
>> me from finding it myself.  I'm sure it would have taken me
>> far longer.
>> 
>> If it turns out that docbook5 causes us more pain than it's
>> worth, I suppose we could choose to use the builtin manpage
>> backend when using asciidoctor >= 2.
> 
> I suspect this will be the easiest way forward.  If we produce
> semantically equivalent manpages, then this is also a lot nicer for
> people who would prefer not to have a full XML toolchain installed.

It's a good end goal.  There are a number of differences in
the output when using the manpage backend directly verus
docbook and then xmlto.  The way links to external html
documents are presented is the main one which comes to mind.
Maybe we can adjust that via asciidoctor-extensions.rb or
something, I don't know.

Elsewhere in this thread, Jeff made the very valid point
that we're probably wise to keep using the docbook/xmlto
chain as long as we're supporting both asciidoc and
asciidoctor.  Unless it turns out that it's more work to
coax asciidoctor (and the various 1.5 and 2.0 releases in
common use) to work with that same docbook/xmlto chain than
it is to do it directly, that is.

>> Or we could see about adding a docbook45 converter, which
>> upstream noted as a possibility in the ticket¹ which dropped
>> docbook45 by default.
> 
> Another possibility, depending on how responsive the xmlto upstream is,
> is to add some sort of DocBook 5 support to it. This shouldn't be
> terribly difficult, although we'd have to disable validation. DocBook 5
> uses RELAX NG, and libxml2/xmllint's support for this has some bugs,
> such that it will mark some valid documents using complex interleaves as
> invalid. Still, if this is the direction we want to go, I have no
> problem adding this support.
> 
> Since we'd have a quite new Asciidoctor and a new xmlto, distros should
> have both around the same time.

Good point.  It can't hurt to push for improvements across
the tools.  (Well, other than time being a limited resource
and time you may spend on doc tools being time you can't
spend on the hash conversion, which I imagine is a more
interesting project.)

Thanks,

-- 
Todd

Re: [PATCH] asciidoctor-extensions: provide `<refmiscinfo/>`

[Jeff King]

On Thu, Apr 04, 2019 at 10:17:21PM -0400, Todd Zullinger wrote:

> Elsewhere in this thread, Jeff made the very valid point
> that we're probably wise to keep using the docbook/xmlto
> chain as long as we're supporting both asciidoc and
> asciidoctor.  Unless it turns out that it's more work to
> coax asciidoctor (and the various 1.5 and 2.0 releases in
> common use) to work with that same docbook/xmlto chain than
> it is to do it directly, that is.

One of my secret (maybe not so secret?) implications there was that it
might be worth dropping asciidoc support sooner rather than later. I.e.,
if it is a burden to make it work with both old and new systems, then
let's make the jump to having it work with the new system.

IMHO we can be a bit more cavalier with saying "you must have a
recent-ish asciidoctor to build the docs", because it's so easy for us
to provide a binary distribution of the built HTML and manpages (in
fact, we already do so for the install-man-quick target).

So it doesn't really leave any platforms out in the cold; it just means
they have to tweak their build procedure.

-Peff

Re: [PATCH] asciidoctor-extensions: provide `<refmiscinfo/>`

[Jeff King]

On Tue, Mar 26, 2019 at 09:06:03PM -0400, Todd Zullinger wrote:

> > I updated to asciidoctor-2.0.1 this morning to test, in case
> > it was one of the issues fixed since the 2.0.0 release.
> > Alas, we're the first to hit it and report it.
> 
> Dan Allen fixed this upstream and released 2.0.2 today.
> It's very good to know that asciidoctor upstream is
> incredibly responsive.  If anyone runs into Dan at a
> conference, please buy him a beer. ;)

Cool. I've interacted a few times with the asciidoctor project due to
our use on git-scm.com, and have always found them pleasant and
responsive.

> There's still the matter of 2.0 dropping docbook45.  I'll
> try to get around to testing 1.5.x releases with docbook5 to
> see if they work reasonably well.  If not, we can add a
> version check and set ASCIIDOC_DOCBOOK appropriately.

With the hacky patch below, I was able to doc-diff the v1.5.8 versus
v2.0.2 output. It looks like there are quite a few cosmetic whitespace
changes. Some probably intentional (extra blank between terms and
paragraphs in a definition list) and some probably not (extra whitespace
before start of content in a bulleted list).

diff --git a/Documentation/doc-diff b/Documentation/doc-diff
index 3355be4798..4c2dccf516 100755
--- a/Documentation/doc-diff
+++ b/Documentation/doc-diff
@@ -17,9 +17,11 @@ f			force rebuild; do not rely on cached results
 c,clean			cleanup temporary working files
 from-asciidoc		use asciidoc with the 'from'-commit
 from-asciidoctor	use asciidoctor with the 'from'-commit
+from-asciidoctor2	use asciidoctor2 with the 'from'-commit
 asciidoc		use asciidoc with both commits
 to-asciidoc		use asciidoc with the 'to'-commit
 to-asciidoctor		use asciidoctor with the 'to'-commit
+to-asciidoctor2		use asciidoctor with the 'to'-commit
 asciidoctor		use asciidoctor with both commits
 cut-header-footer	cut away header and footer
 "
@@ -55,6 +57,10 @@ do
 	--asciidoc)
 		from_program=-asciidoc
 		to_program=-asciidoc ;;
+	--from-asciidoctor2)
+		from_program=-asciidoctor2 ;;
+	--to-asciidoctor2)
+		to_program=-asciidoctor2 ;;
 	--cut-header-footer)
 		cut_header_footer=-cut-header-footer ;;
 	--)
@@ -112,6 +118,13 @@ construct_makemanflags () {
 	elif test "$1" = "-asciidoctor"
 	then
 		echo USE_ASCIIDOCTOR=YesPlease
+	elif test "$1" = "-asciidoctor2"
+	then
+		echo USE_ASCIIDOCTOR=YesPlease
+		echo ASCIIDOC_DOCBOOK=docbook5
+		echo XMLTO_EXTRA=--skip-validation
+		# not really portable :)
+		echo ASCIIDOCTOR=/tmp/run-asciidoctor-2
 	fi
 }
 
@@ -182,6 +195,6 @@ render_tree () {
 	fi
 }
 
-render_tree $from_oid $from_dir $from_makemanflags &&
-render_tree $to_oid $to_dir $to_makemanflags &&
+render_tree $from_oid $from_dir "$from_makemanflags" &&
+render_tree $to_oid $to_dir "$to_makemanflags" &&
 git -C $tmp/rendered diff --no-index "$@" $from_dir $to_dir

Re: [PATCH] asciidoctor-extensions: provide `<refmiscinfo/>`

[Jeff King]

On Wed, Mar 27, 2019 at 10:54:34PM -0400, Jeff King wrote:

> @@ -112,6 +118,13 @@ construct_makemanflags () {
>  	elif test "$1" = "-asciidoctor"
>  	then
>  		echo USE_ASCIIDOCTOR=YesPlease
> +	elif test "$1" = "-asciidoctor2"
> +	then
> +		echo USE_ASCIIDOCTOR=YesPlease
> +		echo ASCIIDOC_DOCBOOK=docbook5
> +		echo XMLTO_EXTRA=--skip-validation
> +		# not really portable :)
> +		echo ASCIIDOCTOR=/tmp/run-asciidoctor-2
>  	fi

Oops, this last line should be ASCIIDOC=..., of course. Which meant that
I was actually just testing the docbook5 output from v1.5.8. The results
with v2.0.2 seem similar, though.

-Peff

Re: [PATCH] asciidoctor-extensions: provide `<refmiscinfo/>`

[Jeff King]

On Sun, Mar 17, 2019 at 03:47:47PM +0100, Martin Ågren wrote:

>  Cc Todd and Peff who had a brief exchange [1] a while ago. Apparently
>  Todd saw this "[FIXME: source]" on Fedora but Peff did not on Debian.
>  I'm on Ubuntu 18.04 and used to see this also on 16.04. I'm not sure
>  what might make Debian so special here.

I think it was just that my version of asciidoctor had

  https://github.com/asciidoctor/asciidoctor/pull/2636

and Todd's did not. However, mine still does not do the _right_ thing,
because we didn't pass the right attributes in to asciidoctor. It just
didn't print an ugly "FIXME". Looking at the XML, I have:

  <refentrytitle>git-add</refentrytitle>
  <manvolnum>1</manvolnum>
  <refmiscinfo class="source">&#160;</refmiscinfo>
  <refmiscinfo class="manual">&#160;</refmiscinfo>
  </refmeta>

So it's just an nbsp instead of the real content, and the "version"
field is missing entirely.

> That Asciidoctor ignores asciidoc.conf is nothing new. This is why we
> implement the `linkgit:` macro in asciidoc.conf *and* in
> asciidoctor-extensions.rb. Follow suit and provide these tags in
> asciidoctor-extensions.rb, using a "postprocessor" extension.

Yeah, that seems sensible overall. Some thoughts on your approach:

>   * Provide the `mansource` attribute to Asciidoctor. This attribute
>     looks promising until one realizes that it can only be given inside
>     the source file (the .txt file in our case), *not* on the command
>     line using `-a mansource=foobar`. I toyed with the idea of injecting
>     this attribute while feeding Asciidoctor the input on stdin, but it
>     didn't feel like it was worth the complexity in the Makefile.

It does seem like "mansource" is the way asciidoctor expects us to do
this. Why doesn't it work from the command line? Is it a bug in
asciidoctor, or is there something more subtle going on?

I think even if it is a bug and gets fixed, though, it still wouldn't
have the version field (though that seems like something we could
contribute to asciidoctor).

>   * Similar to that last idea, we could inject these lines into the
>     xml-files from the Makefile, e.g., using `sed`. This reduces
>     repetition, but seems fairly brittle. It feels wrong to impose
>     another step and another risk on the Asciidoc-processing only to
>     benefit the Asciidoctor-one.

That's more or less what your ruby code is doing on. That said, I'd just
as soon do it in ruby as with a separate sed invocation. At least then
the external build is the same.

>   * Considering the above abandoned ideas, it seems better to put any
>     complexity inside asciidoctor-extensions.rb. It is after all
>     supposed to be the "equivalent" of asciidoc.conf. I considered
>     providing a "tree processor" extension and use it to set, e.g.,
>     `mansource` mentioned above.

This seems like the least bad option, at least for now. Your code does
do a generic regex substitution. The promise of XML is that we're
supposed to be able to do structured, robust transformations of the
document. But my experience has been that the tooling is sufficiently
difficult to work with that you just end up writing a regex.

So I'm curious if you tried to use an actual XML parser (or god forbid,
XSLT) to do the transformation. But if you spent more than 5 minutes on
it and got disgusted, I wouldn't ask you to look deeper than that. :)

I doubt we'd see any other refmeta tags (and any non-tag content would
be quoted).

> Let's instead try to stay as close as possible to what asciidoc.conf
> does. We'll make it fairly obvious that we aim to inject the exact same
> three lines of `<refmiscinfo/>` that asciidoc.conf provides. The only
> somewhat tricky part is that we inject them *post*-processing so we need
> to do the variable expansion ourselves.

One thing that asciidoctor buys us that asciidoc does not is that we
might eventually move to directly generating the manpages, without the
XML / Docbook step in between. And if we do, then all of this XML
hackery is going to have to get replaced with something else. I guess we
can cross that bridge when we come to it.

> diff --git a/Documentation/asciidoctor-extensions.rb b/Documentation/asciidoctor-extensions.rb
> index 0089e0cfb8..059279dee1 100644
> --- a/Documentation/asciidoctor-extensions.rb
> +++ b/Documentation/asciidoctor-extensions.rb
> @@ -20,9 +20,25 @@ module Git
>          end
>        end
>      end
> +
> +    class DocumentPostProcessor < Asciidoctor::Extensions::Postprocessor
> +      def process document, output
> +        if document.basebackend? 'docbook'
> +          git_version = document.attributes['git_version']
> +          replacement = "" \
> +            "<refmiscinfo class=\"source\">Git</refmiscinfo>\n" \
> +            "<refmiscinfo class=\"version\">#{git_version}</refmiscinfo>\n" \
> +            "<refmiscinfo class=\"manual\">Git Manual</refmiscinfo>\n" \
> +            "<\/refmeta>"
> +          output = output.sub(/<\/refmeta>/, replacement)
> +        end
> +        output
> +      end
> +    end

The patch itself looks sane. Would we ever need to XML-quote the
contents of git_version? I guess the asciidoc.conf version doesn't
bother. Technically the user running "make" could put whatever they want
into it, but I think this is a case of "if it hurts, don't do it", and
we can ignore it.

-Peff

Re: [PATCH] asciidoctor-extensions: provide `<refmiscinfo/>`

[Jeff King]

On Mon, Mar 18, 2019 at 10:46:46PM -0400, Jeff King wrote:

> > Let's instead try to stay as close as possible to what asciidoc.conf
> > does. We'll make it fairly obvious that we aim to inject the exact same
> > three lines of `<refmiscinfo/>` that asciidoc.conf provides. The only
> > somewhat tricky part is that we inject them *post*-processing so we need
> > to do the variable expansion ourselves.
> 
> One thing that asciidoctor buys us that asciidoc does not is that we
> might eventually move to directly generating the manpages, without the
> XML / Docbook step in between. And if we do, then all of this XML
> hackery is going to have to get replaced with something else. I guess we
> can cross that bridge when we come to it.

I see there was some discussion of that in the side-thread, too. Just to
give my two cents: I think that's where we want to go eventually, but I
also think that while we are maintaining a dual asciidoc/asciidoctor
tool chain, it's probably crazy not to go through docbook, just because
it keeps so much of the pipeline the same for both tools.

So in my mind, the endgame is that we eventually drop asciidoc in favor
of asciidoctor. The repo at:

  https://github.com/asciidoc/asciidoc

says:

  NOTE: This implementation is written in Python 2, which EOLs in Jan
  2020. AsciiDoc development is being continued under @asciidoctor.

I'm not sure when is the right time to switch. If we can get the output
at parity, I don't think asciidoctor is too onerous to install (and we
don't have to worry about ancient platforms, as they can use
pre-formatted manpages).

-Peff

Re: [PATCH] asciidoctor-extensions: provide `<refmiscinfo/>`

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> So in my mind, the endgame is that we eventually drop asciidoc in favor
> of asciidoctor. The repo at:
>
>   https://github.com/asciidoc/asciidoc
>
> says:
>
>   NOTE: This implementation is written in Python 2, which EOLs in Jan
>   2020. AsciiDoc development is being continued under @asciidoctor.

;-)

> I'm not sure when is the right time to switch. If we can get the output
> at parity, I don't think asciidoctor is too onerous to install (and we
> don't have to worry about ancient platforms, as they can use
> pre-formatted manpages).

One minor thing that bothers me abit is the continuity of the
pre-formatted pages when I switch to asciidoctor to update them.

I do not mind having to see a huge diff in the "git log -p" output
run in pre-formatted manpages and htmldocs repositories at the
boundary due to e.g. the differences how lines are broken or folded
between the formatters, but by the time we have to transition, the
efforts by you, Martin and friends to allow us compare the formatted
docs would have made the real differences to empty (or at least
negligible).  Knock knock...

Re: [PATCH] asciidoctor-extensions: provide `<refmiscinfo/>`

[Martin Ågren]

On Tue, 19 Mar 2019 at 04:55, Junio C Hamano <gitster@pobox.com> wrote:
>
> Jeff King <peff@peff.net> writes:
>
> > So in my mind, the endgame is that we eventually drop asciidoc in favor
> > of asciidoctor. The repo at:
> >
> >   https://github.com/asciidoc/asciidoc
> >
> > says:
> >
> >   NOTE: This implementation is written in Python 2, which EOLs in Jan
> >   2020. AsciiDoc development is being continued under @asciidoctor.
>
> ;-)
>
> > I'm not sure when is the right time to switch. If we can get the output
> > at parity, I don't think asciidoctor is too onerous to install (and we
> > don't have to worry about ancient platforms, as they can use
> > pre-formatted manpages).
>
> One minor thing that bothers me abit is the continuity of the
> pre-formatted pages when I switch to asciidoctor to update them.
>
> I do not mind having to see a huge diff in the "git log -p" output
> run in pre-formatted manpages and htmldocs repositories at the
> boundary due to e.g. the differences how lines are broken or folded
> between the formatters, but by the time we have to transition, the
> efforts by you, Martin and friends to allow us compare the formatted
> docs would have made the real differences to empty (or at least
> negligible).  Knock knock...

This might be a good spot to provide a bit of current status on this.

The patch under discussion here and 185f9a0ea0 ("asciidoctor-extensions:
fix spurious space after linkgit", 2019-02-27), reduce the diff
between asciidoc and asciidoctor considerably. Let's assume for the
moment that these patches or something like them enter master...

There's one larger difference in git-checkout.txt which I'm staying away
from at the moment, since Duy is doing a lot of work there at the time.
(He's not making that asciidoc/tor issue any worse, and he's not
spreading it to say git-switch.txt, so I'd rather just not rush to it.)
Let's assume I get around to this soonish...

Let's also assume that `./doc-diff --cut-header-footer --from-asciidoc
--to-asciidoctor HEAD HEAD` enters master [1]. Then what remains is a
fairly small and understandable diff where most issues are "yeah, I
guess that looks nicer" or even "they're just as fine", rather than "oh
wow, that's ugly". (IMHO.)

Martin

[1] You could already now run it to diff "master master", but you'd
trip on the Makefile thinking there's nothing to do. That'd be fixed by
9a71722b4d ("Doc: auto-detect changed build flags", 2019-03-17).

Re: [PATCH] asciidoctor-extensions: provide `<refmiscinfo/>`

[Martin Ågren]

Hi Junio

On Tue, 19 Mar 2019 at 08:33, Martin Ågren <martin.agren@gmail.com> wrote:
> between asciidoc and asciidoctor considerably. Let's assume for the
> moment that these patches or something like them enter master...

To be clear. *This* patch has a sufficiently incorrect commit message
that it really needs a makeover. You can expect a v2.

Martin

[PATCH v2 0/2] asciidoctor-extensions: provide `<refmiscinfo/>`

[Martin Ågren]

Almost half a year ago, I wrote:
> To be clear. *This* patch has a sufficiently incorrect commit message
> that it really needs a makeover. You can expect a v2.

Finally, here's that v2. I should probably refresh memories: The goal of
the main patch here is to make the headers and footers of our manpages
built by Asciidoctor look a lot more like those generated by AsciiDoc.
In particular, this gets rid of the ugly "[FIXME: source]".

I spent a little bit of time trying to work on the XML as XML, and
quite a lot of time procrastinating on that. In the end, I decided that
the outcome of my attempts wouldn't get better and that there is some
value to the stupid approach from v1 of doing a simple search-and-replace
in the text. I've preserved my attempts in the commit message.

When I posted v1, it turned into quite a thread [1] on AsciiDoc vs
Asciidoctor vs Asciidoctor 2.0 and differences in rendering. (I am on
Asciidoctor 1.5.5.)

Among other things, the v1-thread discussed switching the rendering
toolchain entirely to avoid the detour over xmlto. Doing that would
render this patch obsolete. While I agree that such a switch is the
correct long-term goal and that we can be fairly aggressive about it, I
do also think it makes sense to first make the "softer" switch to
Asciidoctor-by-default and get that particular hurdle behind us. Then,
once we're ok with dropping AsciiDoc entirely, we can do the switch to
an Asciidoctor-only toolchain.

(I'm preparing a second, independent series of patches that should halve
the difference (sans headers/footers) between these two engines -- at
least the versions of them that I'm using. The remaining differences are
then mainly, but not exclusively, in favor of Asciidoctor. That series
should appear on the list within the next couple of days. After that,
there's user-manual.html/pdf that needs looking into...)

[1] https://public-inbox.org/git/20190317144747.2418514-1-martin.agren@gmail.com/

Martin Ågren (2):
  asciidoctor-extensions: provide `<refmiscinfo/>`
  doc-diff: replace --cut-header-footer with --cut-footer

 Documentation/asciidoctor-extensions.rb | 15 +++++++++++++++
 Documentation/doc-diff                  | 17 ++++++++---------
 2 files changed, 23 insertions(+), 9 deletions(-)

-- 
2.23.0

[PATCH v2 1/2] asciidoctor-extensions: provide `<refmiscinfo/>`

[Martin Ågren]

When we build with AsciiDoc, asciidoc.conf ensures that each xml-file we
generate contains some meta-information which `xmlto` can act on, based
on the following template:

  <refmeta>
  <refentrytitle>{mantitle}</refentrytitle>
  <manvolnum>{manvolnum}</manvolnum>
  <refmiscinfo class="source">Git</refmiscinfo>
  <refmiscinfo class="version">{git_version}</refmiscinfo>
  <refmiscinfo class="manual">Git Manual</refmiscinfo>
  </refmeta>

When we build with Asciidoctor, it does not honor this configuration file
and we end up with only this (for a hypothetical git-foo.xml):

  <refmeta>
  <refentrytitle>git-foo</refentrytitle>
  <manvolnum>1</manvolnum>
  </refmeta>

That is, we miss out on the `<refmiscinfo/>` tags. As a result, the
header of each man page doesn't say "Git Manual", but "git-foo(1)"
instead. Worse, the footers don't give the Git version number and
instead provide the fairly ugly "[FIXME: source]".

That Asciidoctor ignores asciidoc.conf is nothing new. This is why we
implement the `linkgit:` macro in asciidoc.conf *and* in
asciidoctor-extensions.rb. Follow suit and provide these tags in
asciidoctor-extensions.rb, using a "postprocessor" extension where we
just search and replace in the XML, treated as text.

We may consider a few alternatives:

  * Provide the `mansource` attribute to Asciidoctor. This attribute
    is only respected (i.e., turned into those <refmiscinfo/> tags) with
    1) the "manpage" doctype and/or converter [1], which we currently do
    not use, or 2) using Asciidoctor 1.5.7 or newer [2].

  * We could inject these lines into the xml-files from the *Makefile*,
    e.g., using `sed`. That would reduce repetition, but it feels wrong
    to impose another step and another risk on the AsciiDoc-processing
    only to benefit the Asciidoctor-one.

  * I tried providing a "docinfo processor" to inject these tags, but
    could not figure out how to "merge" the two <refmeta/> sections that
    resulted. To avoid xmlto barfing on the result, I needed need to use
    `xmlto --skip-validation ...`, which seems unfortunate.

Let's instead inject the missing tags using a postprocessor. We'll make
it fairly obvious that we aim to inject the exact same three lines of
`<refmiscinfo/>` that asciidoc.conf provides. We inject them in
*post*-processing so we need to do the variable expansion ourselves. We
do introduce the bug that asciidoc.conf already has in that we won't do
any escaping, e.g., of funky versions like "some v <2.25, >2.20".

The postprocessor we add here works on the XML as raw text and doesn't
really use the full potential of XML to do a more structured injection.
This is actually precisely what the Asciidoctor User Manual does in its
postprocessor example [3]. I looked into two other approaches:

  1. The nokogiri library is apparently the "modern" way of doing XML
     in ruby. I got it working fairly easily:
        require 'nokogiri'
        doc = Nokogiri::XML(output)
        doc.search("refmeta").each { |n| n.add_child(new_tags) }
        output = doc.to_xml
     However, this adds another dependency (e.g., the "ruby-nokogiri"
     package on Ubuntu). Using Asciidoctor is not our default, but it
     will probably need to become so soon (AsciiDoc relies on Python 2,
     which is going away). Let's avoid adding a dependency just so that
     we can say "search...add_child" rather than "sub(regex...)".

  2. The older REXML is apparently always(?) bundled with ruby, but I
     couldn't even parse the original document:
        require 'rexml/document'
        doc = REXML::Document.new(output)
        ...
     The error was "no implicit conversion of nil into String" and I
     stopped there.

Having never dabbled with ruby outside of this very file, I might be
missing something obvious, but I don't think it's unlikely that doing a
plain old search-and-replace will work just as fine or better compared
to parsing XML and worrying about libraries and library versions.

[1] https://asciidoctor.org/docs/user-manual/#builtin-attributes
[2] https://public-inbox.org/git/20190319074321.GA2189@sigill.intra.peff.net/
[3] https://asciidoctor.org/docs/user-manual/#postprocessor-example

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
---
 Documentation/asciidoctor-extensions.rb | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/Documentation/asciidoctor-extensions.rb b/Documentation/asciidoctor-extensions.rb
index 0089e0cfb8..4ae130d2c6 100644
--- a/Documentation/asciidoctor-extensions.rb
+++ b/Documentation/asciidoctor-extensions.rb
@@ -20,9 +20,24 @@ module Git
         end
       end
     end
+
+    class DocumentPostProcessor < Asciidoctor::Extensions::Postprocessor
+      def process document, output
+        if document.basebackend? 'docbook'
+          git_version = document.attributes['git_version']
+          new_tags = "" \
+            "<refmiscinfo class=\"source\">Git</refmiscinfo>\n" \
+            "<refmiscinfo class=\"version\">#{git_version}</refmiscinfo>\n" \
+            "<refmiscinfo class=\"manual\">Git Manual</refmiscinfo>\n"
+          output = output.sub(/<\/refmeta>/, new_tags + "</refmeta>")
+        end
+        output
+      end
+    end
   end
 end
 
 Asciidoctor::Extensions.register do
   inline_macro Git::Documentation::LinkGitProcessor, :linkgit
+  postprocessor Git::Documentation::DocumentPostProcessor
 end
-- 
2.23.0

[PATCH v2 2/2] doc-diff: replace --cut-header-footer with --cut-footer

[Martin Ågren]

After the previous commit, AsciiDoc and Asciidoctor render the manpage
headers identically, so we no longer need the "cut the header" part of
our `--cut-header-footer` option. We do still need the "cut the footer"
part, though. The previous commit improved the rendering of the footer
in Asciidoctor by quite a bit, but the two programs still disagree on
how to format the date in the footer: 01/01/1970 vs 1970-01-01.

We could keep using `--cut-header-footer`, but it would be nice if we
had a slightly smaller hammer `--cut-footer` that would be less likely
to hide regressions. Rather than simply adding such an option, let's
also drop `--cut-header-footer`, i.e., rework it to lose the "header"
part of its name and functionality.

`--cut-header-footer` is just a developer tool and it probably has no
more than a handful of users, so we can afford be aggressive.

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
---
 Documentation/doc-diff | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/Documentation/doc-diff b/Documentation/doc-diff
index 3355be4798..88a9b20168 100755
--- a/Documentation/doc-diff
+++ b/Documentation/doc-diff
@@ -21,7 +21,7 @@ asciidoc		use asciidoc with both commits
 to-asciidoc		use asciidoc with the 'to'-commit
 to-asciidoctor		use asciidoctor with the 'to'-commit
 asciidoctor		use asciidoctor with both commits
-cut-header-footer	cut away header and footer
+cut-footer		cut away footer
 "
 SUBDIRECTORY_OK=1
 . "$(git --exec-path)/git-sh-setup"
@@ -31,7 +31,7 @@ force=
 clean=
 from_program=
 to_program=
-cut_header_footer=
+cut_footer=
 while test $# -gt 0
 do
 	case "$1" in
@@ -55,8 +55,8 @@ do
 	--asciidoc)
 		from_program=-asciidoc
 		to_program=-asciidoc ;;
-	--cut-header-footer)
-		cut_header_footer=-cut-header-footer ;;
+	--cut-footer)
+		cut_footer=-cut-footer ;;
 	--)
 		shift; break ;;
 	*)
@@ -118,8 +118,8 @@ construct_makemanflags () {
 from_makemanflags=$(construct_makemanflags "$from_program") &&
 to_makemanflags=$(construct_makemanflags "$to_program") &&
 
-from_dir=$from_oid$from_program$cut_header_footer &&
-to_dir=$to_oid$to_program$cut_header_footer &&
+from_dir=$from_oid$from_program$cut_footer &&
+to_dir=$to_oid$to_program$cut_footer &&
 
 # generate_render_makefile <srcdir> <dstdir>
 generate_render_makefile () {
@@ -169,12 +169,11 @@ render_tree () {
 		make -j$parallel -f - &&
 		mv "$tmp/rendered/$dname+" "$tmp/rendered/$dname"
 
-		if test "$cut_header_footer" = "-cut-header-footer"
+		if test "$cut_footer" = "-cut-footer"
 		then
 			for f in $(find "$tmp/rendered/$dname" -type f)
 			do
-				tail -n +3 "$f" | head -n -2 |
-				sed -e '1{/^$/d}' -e '${/^$/d}' >"$f+" &&
+				head -n -2 "$f" | sed -e '${/^$/d}' >"$f+" &&
 				mv "$f+" "$f" ||
 				return 1
 			done
-- 
2.23.0

Re: [PATCH v2 0/2] asciidoctor-extensions: provide `<refmiscinfo/>`

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 2120 bytes --]

On 2019-09-03 at 18:51:19, Martin Ågren wrote:
> Almost half a year ago, I wrote:
> > To be clear. *This* patch has a sufficiently incorrect commit message
> > that it really needs a makeover. You can expect a v2.
> 
> Finally, here's that v2. I should probably refresh memories: The goal of
> the main patch here is to make the headers and footers of our manpages
> built by Asciidoctor look a lot more like those generated by AsciiDoc.
> In particular, this gets rid of the ugly "[FIXME: source]".
> 
> I spent a little bit of time trying to work on the XML as XML, and
> quite a lot of time procrastinating on that. In the end, I decided that
> the outcome of my attempts wouldn't get better and that there is some
> value to the stupid approach from v1 of doing a simple search-and-replace
> in the text. I've preserved my attempts in the commit message.
> 
> When I posted v1, it turned into quite a thread [1] on AsciiDoc vs
> Asciidoctor vs Asciidoctor 2.0 and differences in rendering. (I am on
> Asciidoctor 1.5.5.)
> 
> Among other things, the v1-thread discussed switching the rendering
> toolchain entirely to avoid the detour over xmlto. Doing that would
> render this patch obsolete. While I agree that such a switch is the
> correct long-term goal and that we can be fairly aggressive about it, I
> do also think it makes sense to first make the "softer" switch to
> Asciidoctor-by-default and get that particular hurdle behind us. Then,
> once we're ok with dropping AsciiDoc entirely, we can do the switch to
> an Asciidoctor-only toolchain.

Yeah, this seems like a good approach.  xmlto is a neat tool, but it's
essentially unmaintained and is designed for DocBook 4.  Avoiding it
where possible seems like the right choice.

I looked at this series and it seems sane.  I agree that adding a
dependency on nokogiri isn't really desirable.  It is an extremely
common Ruby package, but it has native extensions, which causes problems
for some people if their distro doesn't support it.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]

Re: [PATCH v2 0/2] asciidoctor-extensions: provide `<refmiscinfo/>`

[Martin Ågren]

On Wed, 4 Sep 2019 at 01:16, brian m. carlson
<sandals@crustytoothpaste.net> wrote:
>
> I looked at this series and it seems sane.  I agree that adding a
> dependency on nokogiri isn't really desirable.  It is an extremely
> common Ruby package, but it has native extensions, which causes problems
> for some people if their distro doesn't support it.

Ok, nice to know I wasn't too far off with avoiding the added
dependency. I saw statements like "oh, it's available everywhere", but
also more nuanced ones like "not necessarily...". I'm glad I have a
trustworthy source now. ;-)

Martin

Re: [PATCH v2 0/2] asciidoctor-extensions: provide `<refmiscinfo/>`

[Jeff King]

On Tue, Sep 03, 2019 at 08:51:19PM +0200, Martin Ågren wrote:

> Almost half a year ago, I wrote:
> > To be clear. *This* patch has a sufficiently incorrect commit message
> > that it really needs a makeover. You can expect a v2.
> 
> Finally, here's that v2. I should probably refresh memories: The goal of
> the main patch here is to make the headers and footers of our manpages
> built by Asciidoctor look a lot more like those generated by AsciiDoc.
> In particular, this gets rid of the ugly "[FIXME: source]".

Thanks for resuming this work. The patches both look good to me.

> I spent a little bit of time trying to work on the XML as XML, and
> quite a lot of time procrastinating on that. In the end, I decided that
> the outcome of my attempts wouldn't get better and that there is some
> value to the stupid approach from v1 of doing a simple search-and-replace
> in the text. I've preserved my attempts in the commit message.

I think your hack is better than introducing yet another dependency.
And because it's isolated and you've documented it well, it will be easy
to undo later if we choose.

The doc-diff tool should tell us if we catch any false positives inside
the text (though it seems quite unlikely to me).

> When I posted v1, it turned into quite a thread [1] on AsciiDoc vs
> Asciidoctor vs Asciidoctor 2.0 and differences in rendering. (I am on
> Asciidoctor 1.5.5.)

Yes, sadly I still can't format the docs at all with 2.0.10 (which is
what ships in Debian unstable).

> Among other things, the v1-thread discussed switching the rendering
> toolchain entirely to avoid the detour over xmlto. Doing that would
> render this patch obsolete. While I agree that such a switch is the
> correct long-term goal and that we can be fairly aggressive about it, I
> do also think it makes sense to first make the "softer" switch to
> Asciidoctor-by-default and get that particular hurdle behind us. Then,
> once we're ok with dropping AsciiDoc entirely, we can do the switch to
> an Asciidoctor-only toolchain.

Yeah, I do still like that as an endgame, but I like what you have here
as an intermediate step in the right direction.

> (I'm preparing a second, independent series of patches that should halve
> the difference (sans headers/footers) between these two engines -- at
> least the versions of them that I'm using. The remaining differences are
> then mainly, but not exclusively, in favor of Asciidoctor. That series
> should appear on the list within the next couple of days. After that,
> there's user-manual.html/pdf that needs looking into...)

Yay. :)

-Peff

Re: [PATCH v2 0/2] asciidoctor-extensions: provide `<refmiscinfo/>`

[Martin Ågren]

On Wed, 4 Sep 2019 at 05:26, Jeff King <peff@peff.net> wrote:
>
> On Tue, Sep 03, 2019 at 08:51:19PM +0200, Martin Ågren wrote:
>
> > When I posted v1, it turned into quite a thread [1] on AsciiDoc vs
> > Asciidoctor vs Asciidoctor 2.0 and differences in rendering. (I am on
> > Asciidoctor 1.5.5.)
>
> Yes, sadly I still can't format the docs at all with 2.0.10 (which is
> what ships in Debian unstable).
>
> > do also think it makes sense to first make the "softer" switch to
> > Asciidoctor-by-default and get that particular hurdle behind us. Then,
> > once we're ok with dropping AsciiDoc entirely, we can do the switch to
> > an Asciidoctor-only toolchain.
>
> Yeah, I do still like that as an endgame, but I like what you have here
> as an intermediate step in the right direction.

Hmm, so this sounds like once I am happy with replacing AsciiDoc with
Asciidoctor 1(.5.5), I should rather not propose a series "let's default
to Asciidoctor!!!" but instead a slightly more careful "go with
Asciidoctor, but document that we work badly with v2 and that the 2nd
choice after Asciidoctor 1 should be AsciiDoc". Or do you see it
differently? (I wonder which Asciidoctor-version Junio would be on..)

Martin

Re: [PATCH v2 0/2] asciidoctor-extensions: provide `<refmiscinfo/>`

[Jeff King]

On Thu, Sep 05, 2019 at 09:35:10PM +0200, Martin Ågren wrote:

> > > do also think it makes sense to first make the "softer" switch to
> > > Asciidoctor-by-default and get that particular hurdle behind us. Then,
> > > once we're ok with dropping AsciiDoc entirely, we can do the switch to
> > > an Asciidoctor-only toolchain.
> >
> > Yeah, I do still like that as an endgame, but I like what you have here
> > as an intermediate step in the right direction.
> 
> Hmm, so this sounds like once I am happy with replacing AsciiDoc with
> Asciidoctor 1(.5.5), I should rather not propose a series "let's default
> to Asciidoctor!!!" but instead a slightly more careful "go with
> Asciidoctor, but document that we work badly with v2 and that the 2nd
> choice after Asciidoctor 1 should be AsciiDoc". Or do you see it
> differently? (I wonder which Asciidoctor-version Junio would be on..)

Yeah, that seems reasonable.

TBH, if making things in the middle step work turns out to be too hard,
I'm not entirely opposed to a hard switch.

The "does not work with 2.0" thing has to be a temporary step, though, I
think, since using the older versions will get harder and harder as time
goes on. I think it's OK to take such a temporary step as long as we
understand where it leads (and presumably its to directly generating the
roff with asciidoctor). The middle step of having asciidoctor+xmlto
helps us understand and isolate which changes are responsible for which
parts of the output.

-Peff

Re: [PATCH v2 0/2] asciidoctor-extensions: provide `<refmiscinfo/>`

[Martin Ågren]

On Sat, 7 Sep 2019 at 08:45, Jeff King <peff@peff.net> wrote:
>
> On Thu, Sep 05, 2019 at 09:35:10PM +0200, Martin Ågren wrote:
>
> > > Yeah, I do still like that as an endgame, but I like what you have here
> > > as an intermediate step in the right direction.
> >
> > Hmm, so this sounds like once I am happy with replacing AsciiDoc with
> > Asciidoctor 1(.5.5), I should rather not propose a series "let's default
> > to Asciidoctor!!!" but instead a slightly more careful "go with
> > Asciidoctor, but document that we work badly with v2 and that the 2nd
> > choice after Asciidoctor 1 should be AsciiDoc". Or do you see it
> > differently? (I wonder which Asciidoctor-version Junio would be on..)
>
> Yeah, that seems reasonable.
>
> TBH, if making things in the middle step work turns out to be too hard,
> I'm not entirely opposed to a hard switch.
>
> The "does not work with 2.0" thing has to be a temporary step, though, I
> think, since using the older versions will get harder and harder as time
> goes on. I think it's OK to take such a temporary step as long as we
> understand where it leads (and presumably its to directly generating the
> roff with asciidoctor). The middle step of having asciidoctor+xmlto
> helps us understand and isolate which changes are responsible for which
> parts of the output.

So of these steps:

  0. Get Asciidoctor (v1) in shape.

  1. Switch the default to Asciidoctor (v1).

  2. Drop AsciiDoc to have faster Asciidoctor-processing, avoid xmlto
     and support Asciidoctor 2. And to avoid the Python 2 EOL, too.

Step 0 is not far away, so step 1 could be done fairly soon IMHO. Step 2
would "hopefully" happen soon after -- maybe even in the same release
cycle as step 1, and if not the same then the one just after. But I
might be the wrong person to trust on that one. I currently don't even
try to build with Asciidoctor 2. I might perhaps look into installing
it, but it could also be that I'll only start using it when it happens
to arrive through my distro.

So as long as I'm not looking into Asciidoctor 2, maybe I shouldn't be
the one to impose "default to asciidoctor" on the world. Dunno. In any
case, I should be able to bring the asciidoc/tor1 differences to a state
where we trust asciidoctor 1 to be in a good shape, so that "someone
else" could pick up the ball and work on asciidoctor 2 vs 1, knowing
that it's ok if they regress AsciiDoc support or even drop it entirely
in the process.

Martin

Re: [PATCH v2 0/2] asciidoctor-extensions: provide `<refmiscinfo/>`

[Jeff King]

On Sat, Sep 07, 2019 at 04:06:48PM +0200, Martin Ågren wrote:

> So of these steps:
> 
>   0. Get Asciidoctor (v1) in shape.
> 
>   1. Switch the default to Asciidoctor (v1).
> 
>   2. Drop AsciiDoc to have faster Asciidoctor-processing, avoid xmlto
>      and support Asciidoctor 2. And to avoid the Python 2 EOL, too.
> 
> Step 0 is not far away, so step 1 could be done fairly soon IMHO. Step 2
> would "hopefully" happen soon after -- maybe even in the same release
> cycle as step 1, and if not the same then the one just after. But I
> might be the wrong person to trust on that one. I currently don't even
> try to build with Asciidoctor 2. I might perhaps look into installing
> it, but it could also be that I'll only start using it when it happens
> to arrive through my distro.
> 
> So as long as I'm not looking into Asciidoctor 2, maybe I shouldn't be
> the one to impose "default to asciidoctor" on the world. Dunno. In any
> case, I should be able to bring the asciidoc/tor1 differences to a state
> where we trust asciidoctor 1 to be in a good shape, so that "someone
> else" could pick up the ball and work on asciidoctor 2 vs 1, knowing
> that it's ok if they regress AsciiDoc support or even drop it entirely
> in the process.

We could also drop xmlto for the asciidoctor codepaths, without making
it the default. That requires a little bit more Makefile massaging, but
here's a hacky way to do it (this is on top of brian's asciidoctor 2
patch, which I've been experimenting with, but obviously we could skip
that entirely, too):

---
diff --git a/Documentation/Makefile b/Documentation/Makefile
index 485f365cbf..c3ebca6e36 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -197,12 +197,12 @@ ifdef USE_ASCIIDOCTOR
 ASCIIDOC = asciidoctor
 ASCIIDOC_CONF =
 ASCIIDOC_HTML = xhtml5
-ASCIIDOC_DOCBOOK = docbook5
+ASCIIDOC_DOCBOOK = manpage
 ASCIIDOC_EXTRA += -acompat-mode -atabsize=8
 ASCIIDOC_EXTRA += -I. -rasciidoctor-extensions
 ASCIIDOC_EXTRA += -alitdd='&\#x2d;&\#x2d;'
 DBLATEX_COMMON =
-XMLTO_EXTRA += --skip-validation
+XMLTO = ./fake-xmlto
 endif
 
 SHELL_PATH ?= $(SHELL)
diff --git a/Documentation/asciidoctor-extensions.rb b/Documentation/asciidoctor-extensions.rb
index 4ae130d2c6..aae891e8ff 100644
--- a/Documentation/asciidoctor-extensions.rb
+++ b/Documentation/asciidoctor-extensions.rb
@@ -17,6 +17,8 @@ module Git
             "<refentrytitle>#{target}</refentrytitle>" \
             "<manvolnum>#{attrs[1]}</manvolnum>\n" \
           "</citerefentry>"
+        elsif parent.document.basebackend? 'manpage'
+          "#{target}(#{attrs[1]})"
         end
       end
     end
diff --git a/Documentation/fake-xmlto b/Documentation/fake-xmlto
new file mode 100755
index 0000000000..eca7ba289d
--- /dev/null
+++ b/Documentation/fake-xmlto
@@ -0,0 +1,9 @@
+#!/bin/sh
+
+# pick last arg as file
+for file in "$@"; do
+  : nothing
+done
+
+nr=$(perl -lne '/^\.TH ".*?" "(\d)"/ and print $1' $file)
+cp $file ${file%.xml}.$nr
-- 
2.23.0.554.g6dbe768f61


That's enough to let you do:

  ./doc-diff --from-asciidoc --to-asciidoctor HEAD^ HEAD

and see what jumping to direct-manpage generation would look like. After
applying all of your recent patches, I see some improvements. E.g.:

  -           Files to add content from. Fileglobs (e.g.  *.c) can be given to
  +           Files to add content from. Fileglobs (e.g. *.c) can be given to add

and some regressions:

  -           of nothing). The other file, git-add--interactive.perl, has 403
  +           of nothing). The other file, git-add&#x2d;&#x2d;interactive.perl,

There's also a lot of noise. One curiosity is that it refuses to break
linkgit output when wrapping:

  -           Remove everything in body before a scissors line (see git-
  -           mailinfo(1)). Can be activated by default using the
  +           Remove everything in body before a scissors line (see
  +           git-mailinfo(1)). Can be activated by default using the

I think that's fine (and probably even better), but it makes the
doc-diff quite hard to read. ;) Setting MANWIDTH=1000 in doc-diff is a
hacky way to eliminate these, but I'm not sure it's a good idea in
general.

-Peff

Re: [PATCH v2 0/2] asciidoctor-extensions: provide `<refmiscinfo/>`

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 718 bytes --]

On 2019-09-04 at 03:26:10, Jeff King wrote:
> On Tue, Sep 03, 2019 at 08:51:19PM +0200, Martin Ågren wrote:
> > When I posted v1, it turned into quite a thread [1] on AsciiDoc vs
> > Asciidoctor vs Asciidoctor 2.0 and differences in rendering. (I am on
> > Asciidoctor 1.5.5.)
> 
> Yes, sadly I still can't format the docs at all with 2.0.10 (which is
> what ships in Debian unstable).

I'll look into this.  I requested the upgrade in sid.

If it's the lack of DocBook 4.5 support, then I'll probably need to
patch xmlto for that.  DocBook 5 has been around for a decade now, so
it's time for xmlto to support it properly.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]

Re: [PATCH v2 0/2] asciidoctor-extensions: provide `<refmiscinfo/>`

[Jeff King]

On Fri, Sep 06, 2019 at 11:29:47PM +0000, brian m. carlson wrote:

> On 2019-09-04 at 03:26:10, Jeff King wrote:
> > On Tue, Sep 03, 2019 at 08:51:19PM +0200, Martin Ågren wrote:
> > > When I posted v1, it turned into quite a thread [1] on AsciiDoc vs
> > > Asciidoctor vs Asciidoctor 2.0 and differences in rendering. (I am on
> > > Asciidoctor 1.5.5.)
> > 
> > Yes, sadly I still can't format the docs at all with 2.0.10 (which is
> > what ships in Debian unstable).
> 
> I'll look into this.  I requested the upgrade in sid.
> 
> If it's the lack of DocBook 4.5 support, then I'll probably need to
> patch xmlto for that.  DocBook 5 has been around for a decade now, so
> it's time for xmlto to support it properly.

Yes, it's the docbook45 thing. Switching to docbook5 lets asciidoc run,
but then xmlto chokes.

-Peff

Re: [PATCH v2 0/2] asciidoctor-extensions: provide `<refmiscinfo/>`

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 639 bytes --]

On 2019-09-07 at 04:40:22, Jeff King wrote:
> On Fri, Sep 06, 2019 at 11:29:47PM +0000, brian m. carlson wrote:
> > I'll look into this.  I requested the upgrade in sid.
> > 
> > If it's the lack of DocBook 4.5 support, then I'll probably need to
> > patch xmlto for that.  DocBook 5 has been around for a decade now, so
> > it's time for xmlto to support it properly.
> 
> Yes, it's the docbook45 thing. Switching to docbook5 lets asciidoc run,
> but then xmlto chokes.

I already have a patch for Git to fix this.  I'll send it out this weekend.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]

[PATCH] Documentation: fix build with Asciidoctor 2

[brian m. carlson]

Our documentation toolchain has traditionally been built around DocBook
4.5.  This version of DocBook is the last DTD-based version of DocBook.
In 2009, DocBook 5 was introduced using namespaces and its syntax is
expressed in RELAX-NG, which is more expressive and allows a wider
variety of syntax forms.

Asciidoctor, one of the alternatives for building our documentation,
dropped support for DocBook 4.5 in its recent 2.0 release and now only
supports DocBook 5.  This would not be a problem but for the fact that
we use xmlto, which is still stuck in the DocBook 4.5 era.

xmlto performs DTD validation as part of the build process.  This is not
problematic for DocBook 4.5, which has a valid DTD, but it clearly
cannot work for DocBook 5, since no DTD can adequately express its full
syntax.  In addition, even if xmlto did support RELAX-NG validation,
that wouldn't be sufficient because it uses the libxml2-based xmllint to
do so, which has known problems with validating interleaves in RELAX-NG.

Fortunately, there's an easy way forward: ask Asciidoctor to use its
DocBook 5 backend and tell xmlto to skip validation.  Asciidoctor has
supported DocBook 5 since v0.1.4 in 2013 and xmlto has supported
skipping validation for probably longer than that.

xmlto will still use the non-namespaced DocBook XSL stylesheets (which
are designed for DocBook 4.5) for building the documentation instead of
the namespaced ones (which are designed for DocBook 5).  This isn't
really a problem, since both forms are built from the same source and
can handle both versions, but it does mean that an ugly message about
the stylesheets stripping the namespace will be printed to standard
error.

Overall, however, this message is a relatively minor price to pay and it
means that we can continue to use the aging xmlto to drive our build
process, while still supporting newer tooling like Asciidoctor 2.

The differences in output between AsciiDoc 8.6.10 on master and
Asciidoctor 2.0.10 with this patch are, with one exception, all due to
whitespace, wrapping, or quoting and do not affect substantive content.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 Documentation/Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 76f2ecfc1b..485f365cbf 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -197,11 +197,12 @@ ifdef USE_ASCIIDOCTOR
 ASCIIDOC = asciidoctor
 ASCIIDOC_CONF =
 ASCIIDOC_HTML = xhtml5
-ASCIIDOC_DOCBOOK = docbook45
+ASCIIDOC_DOCBOOK = docbook5
 ASCIIDOC_EXTRA += -acompat-mode -atabsize=8
 ASCIIDOC_EXTRA += -I. -rasciidoctor-extensions
 ASCIIDOC_EXTRA += -alitdd='&\#x2d;&\#x2d;'
 DBLATEX_COMMON =
+XMLTO_EXTRA += --skip-validation
 endif
 
 SHELL_PATH ?= $(SHELL)

Re: [PATCH] Documentation: fix build with Asciidoctor 2

[Jeff King]

On Sat, Sep 07, 2019 at 05:07:46PM +0000, brian m. carlson wrote:

> Our documentation toolchain has traditionally been built around DocBook
> 4.5.  This version of DocBook is the last DTD-based version of DocBook.
> In 2009, DocBook 5 was introduced using namespaces and its syntax is
> expressed in RELAX-NG, which is more expressive and allows a wider
> variety of syntax forms.

Wow, this patch turned out to be much simpler than I had feared. And the
explanation turned out to be much longer and more interesting than I
expected. :) Thanks for writing it all down.

The patch worked well for me.

> Fortunately, there's an easy way forward: ask Asciidoctor to use its
> DocBook 5 backend and tell xmlto to skip validation.  Asciidoctor has
> supported DocBook 5 since v0.1.4 in 2013 and xmlto has supported
> skipping validation for probably longer than that.
> 
> xmlto will still use the non-namespaced DocBook XSL stylesheets (which
> are designed for DocBook 4.5) for building the documentation instead of
> the namespaced ones (which are designed for DocBook 5).  This isn't
> really a problem, since both forms are built from the same source and
> can handle both versions, but it does mean that an ugly message about
> the stylesheets stripping the namespace will be printed to standard
> error.

So I think this does introduce the ugly namespace message even for
people who are using older versions of asciidoctor that understand
docbook45. But it's probably sensible to keep all of our asciidoctor
builds using the same technique. We have enough trouble tracking the
minor differences between the variants as it is.

> The differences in output between AsciiDoc 8.6.10 on master and
> Asciidoctor 2.0.10 with this patch are, with one exception, all due to
> whitespace, wrapping, or quoting and do not affect substantive content.

We know already there are a lot of differences between asciidoc and
asciidoctor here. What's more interesting to me is how it changes the
output between two versions of asciidoctor. Of course we can't check for
2.0.10, because it doesn't build at all. But I did do a "doc-diff"
with and without this patch using 1.5.5. There are quite a few
whitespace changes. Some of them seem good or at least neutral, like:

  --- a/745f6812895b31c02b29bdfe4ae8e5498f776c26-asciidoctor/home/peff/share/man/man1/git-add.1
  +++ b/303729d86b69657777222bf4b3a6f95932e12648-asciidoctor/home/peff/share/man/man1/git-add.1
  [...]
  @@ -43,6 +43,7 @@ DESCRIPTION

   OPTIONS
          <pathspec>...
  +
              Files to add content from. Fileglobs (e.g.  *.c) can be given to
              add all matching files. Also a leading directory name (e.g.  dir to
              add dir/file1 and dir/file2) can be given to update the index to

Some of them seem bad, though:

  --- a/745f6812895b31c02b29bdfe4ae8e5498f776c26-asciidoctor/home/peff/share/man/man1/git-am.1
  +++ b/303729d86b69657777222bf4b3a6f95932e12648-asciidoctor/home/peff/share/man/man1/git-am.1
  [...]
  @@ -175,10 +201,10 @@ DISCUSSION

          to process. Upon seeing the first patch that does not apply, it aborts
          in the middle. You can recover from this in one of two ways:
   
  -        1. skip the current patch by re-running the command with the --skip
  +        1.  skip the current patch by re-running the command with the --skip
              option.
   
  -        2. hand resolve the conflict in the working directory, and update the
  +        2.  hand resolve the conflict in the working directory, and update the
              index file to bring it into a state that the patch should have
              produced. Then run the command with the --continue option.

I tricked doc-diff into doing a comparison against 1.5.5 without the
patch and 2.0.10 with the patch, and the diff is similar.

-Peff

Re: [PATCH] Documentation: fix build with Asciidoctor 2

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 1580 bytes --]

On 2019-09-08 at 10:48:33, Jeff King wrote:
> Some of them seem bad, though:
> 
>   --- a/745f6812895b31c02b29bdfe4ae8e5498f776c26-asciidoctor/home/peff/share/man/man1/git-am.1
>   +++ b/303729d86b69657777222bf4b3a6f95932e12648-asciidoctor/home/peff/share/man/man1/git-am.1
>   [...]
>   @@ -175,10 +201,10 @@ DISCUSSION
> 
>           to process. Upon seeing the first patch that does not apply, it aborts
>           in the middle. You can recover from this in one of two ways:
>    
>   -        1. skip the current patch by re-running the command with the --skip
>   +        1.  skip the current patch by re-running the command with the --skip
>               option.
>    
>   -        2. hand resolve the conflict in the working directory, and update the
>   +        2.  hand resolve the conflict in the working directory, and update the
>               index file to bring it into a state that the patch should have
>               produced. Then run the command with the --continue option.
> 
> I tricked doc-diff into doing a comparison against 1.5.5 without the
> patch and 2.0.10 with the patch, and the diff is similar.

I see the same thing in doc-diff, but this issue doesn't appear to
actually render in the manual pages themselves, even with MANWIDTH=80.
I'm not sure why, but it doesn't seem to misrender in the actual man
output.

I also tried with the DocBook 5 (docbook-xsl-ns) stylesheets, but that
doesn't appear to make a difference in doc-diff.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]

Re: [PATCH] Documentation: fix build with Asciidoctor 2

[Jeff King]

On Sun, Sep 08, 2019 at 05:18:07PM +0000, brian m. carlson wrote:

> >   -        2. hand resolve the conflict in the working directory, and update the
> >   +        2.  hand resolve the conflict in the working directory, and update the
> >               index file to bring it into a state that the patch should have
> >               produced. Then run the command with the --continue option.
> > 
> > I tricked doc-diff into doing a comparison against 1.5.5 without the
> > patch and 2.0.10 with the patch, and the diff is similar.
> 
> I see the same thing in doc-diff, but this issue doesn't appear to
> actually render in the manual pages themselves, even with MANWIDTH=80.
> I'm not sure why, but it doesn't seem to misrender in the actual man
> output.

Hrm. But doc-diff _is_ diffing the output of man, so that implies a bug
in that script. However, if I manually go into tmp-doc-diff/installed
and run "man -l git-am.1", I do see the extra whitespace, which is what
I'd expect. Are you sure you did your manual inspection on the right
thing?

I was also curious about the root cause at the roff level, so I did this
hack:

diff --git a/Documentation/doc-diff b/Documentation/doc-diff
index 88a9b20168..fde3ac7154 100755
--- a/Documentation/doc-diff
+++ b/Documentation/doc-diff
@@ -22,6 +22,7 @@ to-asciidoc		use asciidoc with the 'to'-commit
 to-asciidoctor		use asciidoctor with the 'to'-commit
 asciidoctor		use asciidoctor with both commits
 cut-footer		cut away footer
+raw			diff roff instead of rendered manpages
 "
 SUBDIRECTORY_OK=1
 . "$(git --exec-path)/git-sh-setup"
@@ -32,6 +33,7 @@ clean=
 from_program=
 to_program=
 cut_footer=
+raw=
 while test $# -gt 0
 do
 	case "$1" in
@@ -57,6 +59,8 @@ do
 		to_program=-asciidoc ;;
 	--cut-footer)
 		cut_footer=-cut-footer ;;
+	--raw)
+		raw=t ;;
 	--)
 		shift; break ;;
 	*)
@@ -159,6 +163,11 @@ render_tree () {
 		mv "$tmp/installed/$dname+" "$tmp/installed/$dname"
 	fi &&
 
+	if test -n "$raw"
+	then
+		return 0
+	fi &&
+
 	# As with "installed" above, we skip the render if it's already been
 	# done.  So using make here is primarily just about running in
 	# parallel.
@@ -181,6 +190,13 @@ render_tree () {
 	fi
 }
 
+if test -z "$raw"
+then
+	diffdir=$tmp/rendered
+else
+	diffdir=$tmp/installed
+fi
+
 render_tree $from_oid $from_dir $from_makemanflags &&
 render_tree $to_oid $to_dir $to_makemanflags &&
-git -C $tmp/rendered diff --no-index "$@" $from_dir $to_dir
+git -C $diffdir diff --no-index "$@" $from_dir $to_dir


The results are quite noisy compared to the rendered output (i.e.,
stylistic choices in the roff output that have equivalent outputs). But
it did let me easily find the difference in one of these cases:


  @@ -312,6 +458,7 @@ option\&.
   .sp -1
   .IP "  2." 4.2
   .\}
  +
   hand resolve the conflict in the working directory, and update the index file to bring it into a state that the patch should have produced\&. Then run the command with the
   \fB\-\-continue\fR
   option\&.


But it's not clear from that whether asciidoc's docbook5 backend inserts
an extra newline, or if it's part of the xml translation. Looking at the
actual XML, I see:

  <listitem>
  <simpara>hand resolve the conflict in the working directory, and update
  the index file to bring it into a state that the patch should
  have produced.  Then run the command with the <literal>--continue</literal> option.</simpara>
  </listitem>

which looks OK. I wondered if there should not be a break between
<listitem> and <simpara>, but it's there in the asciidoc version, too.
So I'm inclined to blame xmlto/docbook5 here.

-Peff

Re: [PATCH] Documentation: fix build with Asciidoctor 2

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 3215 bytes --]

On 2019-09-08 at 21:21:22, Jeff King wrote:
> But it's not clear from that whether asciidoc's docbook5 backend inserts
> an extra newline, or if it's part of the xml translation. Looking at the
> actual XML, I see:
> 
>   <listitem>
>   <simpara>hand resolve the conflict in the working directory, and update
>   the index file to bring it into a state that the patch should
>   have produced.  Then run the command with the <literal>--continue</literal> option.</simpara>
>   </listitem>
> 
> which looks OK. I wondered if there should not be a break between
> <listitem> and <simpara>, but it's there in the asciidoc version, too.
> So I'm inclined to blame xmlto/docbook5 here.

Trying again, I'm able to reproduce this.  I found the cause, which is
in the stylesheets.  XSLT stylesheets have the ability to specify
elements from which whitespace should be stripped (using the
xsl:strip-space directive).  In the DocBook stylesheets, listitem is
specified as such an element, so the whitespace there should be
stripped.

However, in DocBook 5, our elements are in a namespace.  Therefore, the
unnamespaced stylesheets specify only "listitem", not "d:listitem", like
the namespaced stylesheets do.  Because this happens right after the
tree has been constructed "but before it is otherwise processed by XSLT"
and isn't affected by the EXSLT extension that allows re-parsing the
modified tree, then we end up with the whitespace that we don't want.

There are a couple ways around this.

1. We can force xmlto to use the DocBook 5 stylesheets with the "-x"
flag, but we have to know where they are.  Debian and Fedora have them
in different places, so we'd need a knob to figure out where they are.

2. We can force xmlto to use a custom stylesheet with "-x" that merely
imports the DocBook 5 stylesheets using a URL.  If the user has the
DocBook 5 stylesheets installed and XML catalogs configured (the default
on Linux distributions), then everything will just work and the system
will resolve it to the local copy.  If, however, things are not properly
configured, this will result in multiple network downloads for each
manual page.

3. We can give up on xmlto and do things ourselves.  This has the same
problem as option 1, since we need to learn how to find the stylesheets.

4. We can send a patch to xmlto to make it use the proper stylesheets
for DocBook 5 and hope upstream does a release and everyone upgrades
shortly.  Since xmlto is not at all active upstream, this seems like it
may be an imprudent choice.

5. We can send a patch to the DocBook stylesheets and have them include
both the namespaced and unnamespaced versions of the element names in
both sets of stylesheets and hope everyone upgrades.

My personal preference is #2; I think that seems like the best choice
forward.  XML catalogs are well understood and well configured on Linux
distributions.  Homebrew supports them adequately, but you have to add
an environment variable to your shell configuration to enable them.  Of
course, if you're doing _anything_ with XML, you'll have them enabled.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]

Re: [PATCH] Documentation: fix build with Asciidoctor 2

[Junio C Hamano]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> There are a couple ways around this.
>
> 1. We can force xmlto to use the DocBook 5 stylesheets with the "-x"
> flag, but we have to know where they are.  Debian and Fedora have them
> in different places, so we'd need a knob to figure out where they are.
>
> 2. We can force xmlto to use a custom stylesheet with "-x" that merely
> imports the DocBook 5 stylesheets using a URL.  If the user has the
> DocBook 5 stylesheets installed and XML catalogs configured (the default
> on Linux distributions), then everything will just work and the system
> will resolve it to the local copy.  If, however, things are not properly
> configured, this will result in multiple network downloads for each
> manual page.
>
> 3. We can give up on xmlto and do things ourselves.  This has the same
> problem as option 1, since we need to learn how to find the stylesheets.
>
> 4. We can send a patch to xmlto to make it use the proper stylesheets
> for DocBook 5 and hope upstream does a release and everyone upgrades
> shortly.  Since xmlto is not at all active upstream, this seems like it
> may be an imprudent choice.
>
> 5. We can send a patch to the DocBook stylesheets and have them include
> both the namespaced and unnamespaced versions of the element names in
> both sets of stylesheets and hope everyone upgrades.
>
> My personal preference is #2; I think that seems like the best choice
> forward.  XML catalogs are well understood and well configured on Linux
> distributions.  Homebrew supports them adequately, but you have to add
> an environment variable to your shell configuration to enable them.  Of
> course, if you're doing _anything_ with XML, you'll have them enabled.

Sounds sensible and well reasoned.

Re: [PATCH] Documentation: fix build with Asciidoctor 2

[Jeff King]

On Sun, Sep 08, 2019 at 10:24:24PM +0000, brian m. carlson wrote:

> Trying again, I'm able to reproduce this.  I found the cause, which is
> in the stylesheets.  XSLT stylesheets have the ability to specify
> elements from which whitespace should be stripped (using the
> xsl:strip-space directive).  In the DocBook stylesheets, listitem is
> specified as such an element, so the whitespace there should be
> stripped.
> 
> However, in DocBook 5, our elements are in a namespace.  Therefore, the
> unnamespaced stylesheets specify only "listitem", not "d:listitem", like
> the namespaced stylesheets do.  Because this happens right after the
> tree has been constructed "but before it is otherwise processed by XSLT"
> and isn't affected by the EXSLT extension that allows re-parsing the
> modified tree, then we end up with the whitespace that we don't want.

First off, thank you again for your explanations. I dread digging into
how anything related to docbook or xml works, so having you serve it up
on a silver platter is a delight. :)

> 2. We can force xmlto to use a custom stylesheet with "-x" that merely
> imports the DocBook 5 stylesheets using a URL.  If the user has the
> DocBook 5 stylesheets installed and XML catalogs configured (the default
> on Linux distributions), then everything will just work and the system
> will resolve it to the local copy.  If, however, things are not properly
> configured, this will result in multiple network downloads for each
> manual page.

Isn't this already the case just with the docbook DTDs? I.e., if you
don't have a catalog entry, it is up to the tool (xmlto in this case) to
either fail or try to fetch it. That seems like the best we can do. And
as you note, this typically just works out of the box on modern
installs. Of course people may want to build on non-modern ones, but
IMHO we should probably be more aggressive about dropping legacy support
in the documentation and pointing people to the pre-formatted pages.

> My personal preference is #2; I think that seems like the best choice
> forward.  XML catalogs are well understood and well configured on Linux
> distributions.  Homebrew supports them adequately, but you have to add
> an environment variable to your shell configuration to enable them.  Of
> course, if you're doing _anything_ with XML, you'll have them enabled.

Yeah, agreed.

-Peff

Re: [PATCH] Documentation: fix build with Asciidoctor 2

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 1537 bytes --]

On 2019-09-10 at 18:44:22, Jeff King wrote:
> First off, thank you again for your explanations. I dread digging into
> how anything related to docbook or xml works, so having you serve it up
> on a silver platter is a delight. :)

I'm happy to do it.  I was an English major in college and virtually
every assignment I wrote in DocBook, so I'm far more familiar with it
than I'm sure most people ever want to be.

> Isn't this already the case just with the docbook DTDs? I.e., if you
> don't have a catalog entry, it is up to the tool (xmlto in this case) to
> either fail or try to fetch it. That seems like the best we can do. And
> as you note, this typically just works out of the box on modern
> installs. Of course people may want to build on non-modern ones, but
> IMHO we should probably be more aggressive about dropping legacy support
> in the documentation and pointing people to the pre-formatted pages.

Yeah, that's a good point.  Since we know that most people will have
catalogs working (since usually fetching the DTD repeatedly over the
network gets you rate-limited resulting in a failure), I think it's fair
for us to do this.  And anyway, DocBook 5 isn't exactly new: it's been
around since 2009, IIRC, so distros will have it.

I know Homebrew doesn't have this problem, since they fetch the prebuilt
docs, even for HEAD (master) builds.

I'll try to get the patch written up and sent out today or tomorrow.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]

Re: [PATCH] Documentation: fix build with Asciidoctor 2

[SZEDER Gábor]

On Sat, Sep 07, 2019 at 05:07:46PM +0000, brian m. carlson wrote:
> Our documentation toolchain has traditionally been built around DocBook
> 4.5.  This version of DocBook is the last DTD-based version of DocBook.
> In 2009, DocBook 5 was introduced using namespaces and its syntax is
> expressed in RELAX-NG, which is more expressive and allows a wider
> variety of syntax forms.
> 
> Asciidoctor, one of the alternatives for building our documentation,
> dropped support for DocBook 4.5 in its recent 2.0 release and now only
> supports DocBook 5.  This would not be a problem but for the fact that
> we use xmlto, which is still stuck in the DocBook 4.5 era.
> 
> xmlto performs DTD validation as part of the build process.  This is not
> problematic for DocBook 4.5, which has a valid DTD, but it clearly
> cannot work for DocBook 5, since no DTD can adequately express its full
> syntax.  In addition, even if xmlto did support RELAX-NG validation,
> that wouldn't be sufficient because it uses the libxml2-based xmllint to
> do so, which has known problems with validating interleaves in RELAX-NG.
> 
> Fortunately, there's an easy way forward: ask Asciidoctor to use its
> DocBook 5 backend and tell xmlto to skip validation.  Asciidoctor has
> supported DocBook 5 since v0.1.4 in 2013 and xmlto has supported
> skipping validation for probably longer than that.
> 
> xmlto will still use the non-namespaced DocBook XSL stylesheets (which
> are designed for DocBook 4.5) for building the documentation instead of
> the namespaced ones (which are designed for DocBook 5).  This isn't
> really a problem, since both forms are built from the same source and
> can handle both versions, but it does mean that an ugly message about
> the stylesheets stripping the namespace will be printed to standard
> error.

These messages from 'xmlto' look like these, and there are a lot of
them:

  Note: namesp. cut : stripped namespace before processing      Git User Manual
  Note: namesp. cut : stripped namespace before processing      git-sh-setup(1)
  Note: namesp. cut : stripped namespace before processing      git-get-tar-commit-id(1)

Unfortunately, these messages to standard error cause our CI builds to
fail [1].

In the Documentation build job we check that there was nothing printed
to standard error during 'make doc', in order to catch warnings that
are potential signs of a mis-rendered documentation, but do not cause
any asciidoc/asciidoctor/xmlto commands (and thus 'make doc') to fail.

Now, a few recent messages to standard error that indeed were signs of
mis-rendered docs [2] looked like this:

  asciidoctor: WARNING: api-config.txt: line 232: unterminated listing block

i.e. they came from Asciidoctor and were all-caps warnings.

OTOH, these "stripped namespace" messages come from 'xmlto', are not
warnings but have that "Note:" prefix, and, trusting that you did
check the results thoroughly, are apparently not a sign of any
rendering issues.  So I think it's safe to ignore them and this patch
should strip them from 'make doc's output in
'ci/test-documentation.sh'.

Related: after this patch we might want to update the CI builds to use
Asciidoctor 2 instead of sticking with 1.5.8.


[1] With Asciidoctor 1.5.8:

      https://travis-ci.org/szeder/git/jobs/582294090#L2085

    With an additional patch on top to use Asciidoctor 2:

      https://travis-ci.org/szeder/git/jobs/582294243#L2066

[2] See 'git log -3 b373e4d29b'

Re: [PATCH] Documentation: fix build with Asciidoctor 2

[Jeff King]

On Sun, Sep 08, 2019 at 04:13:08PM +0200, SZEDER Gábor wrote:

> OTOH, these "stripped namespace" messages come from 'xmlto', are not
> warnings but have that "Note:" prefix, and, trusting that you did
> check the results thoroughly, are apparently not a sign of any
> rendering issues.  So I think it's safe to ignore them and this patch
> should strip them from 'make doc's output in
> 'ci/test-documentation.sh'.

Yeah, I'd agree (though I do think there are some other issues with this
approach, at least in my tests).

If we do go this route, it might even be nice to use a small xmlto
wrapper that filters the stderr for us. Then the ci code doesn't have to
know about this distinction, and it prevents users from seeing the ugly
messages.

-Peff

[PATCH v2] Documentation: fix build with Asciidoctor 2

[brian m. carlson]

Our documentation toolchain has traditionally been built around DocBook
4.5.  This version of DocBook is the last DTD-based version of DocBook.
In 2009, DocBook 5 was introduced using namespaces and its syntax is
expressed in RELAX NG, which is more expressive and allows a wider
variety of syntax forms.

Asciidoctor, one of the alternatives for building our documentation,
moved support for DocBook 4.5 out of core in its recent 2.0 release and
now only supports DocBook 5 in the main release.  The DocBoook 4.5
converter is still available as a separate component, but this is not
available in most distro packages.  This would not be a problem but for
the fact that we use xmlto, which is still stuck in the DocBook 4.5 era.

xmlto performs DTD validation as part of the build process.  This is not
problematic for DocBook 4.5, which has a valid DTD, but it clearly
cannot work for DocBook 5, since no DTD can adequately express its full
syntax.  In addition, even if xmlto did support RELAX NG validation,
that wouldn't be sufficient because it uses the libxml2-based xmllint to
do so, which has known problems with validating interleaves in RELAX NG.

Fortunately, there's an easy way forward: ask Asciidoctor to use its
DocBook 5 backend and tell xmlto to skip validation.  Asciidoctor has
supported DocBook 5 since v0.1.4 in 2013 and xmlto has supported
skipping validation for probably longer than that.

We also need to teach xmlto how to use the namespaced DocBook XSLT
stylesheets instead of the non-namespaced ones it usually uses.
Normally these stylesheets are interchangeable, but the non-namespaced
ones have a bug that causes them not to strip whitespace automatically
from certain elements when namespaces are in use.  This results in
additional whitespace at the beginning of list elements, which is
jarring and unsightly.

We can do this by passing a custom stylesheet with the -x option that
simply imports the namespaced stylesheets via a URL.  Any system with
support for XML catalogs will automatically look this URL up and
reference a local copy instead without us having to know where this
local copy is located.  We know that anyone using xmlto will already
have catalogs set up properly since the DocBook 4.5 DTD used during
validation is also looked up via catalogs.  All major Linux
distributions distribute the necessary stylesheets and have built-in
catalog support, and Homebrew does as well, albeit with a requirement to
set an environment variable to enable catalog support.

On the off chance that someone lacks support for catalogs, it is
possible for xmlto (via xmllint) to download the stylesheets from the
URLs in question, although this will likely perform poorly enough to
attract attention.  People still have the option of using the prebuilt
documentation that we ship, so happily this should not be an impediment.

Finally, we need to filter out some messages from other stylesheets that
when invoking dblatex in the CI job.  This tool strips namespaces much
like the unnamespaced DocBook stylesheets and prints similar messages.
If we permit these messages to be printed to standard error, our
documentation CI job will because we check standard error for unexpected
output.  Due to dblatex's reliance on Python 2, we may need to revisit
its use in the future, in which case this problem may go away, but this
can be delayed until a future patch.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 Documentation/Makefile    | 4 +++-
 Documentation/manpage.xsl | 3 +++
 ci/test-documentation.sh  | 2 ++
 3 files changed, 8 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/manpage.xsl

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 76f2ecfc1b..d94f47c5c9 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -197,11 +197,13 @@ ifdef USE_ASCIIDOCTOR
 ASCIIDOC = asciidoctor
 ASCIIDOC_CONF =
 ASCIIDOC_HTML = xhtml5
-ASCIIDOC_DOCBOOK = docbook45
+ASCIIDOC_DOCBOOK = docbook5
 ASCIIDOC_EXTRA += -acompat-mode -atabsize=8
 ASCIIDOC_EXTRA += -I. -rasciidoctor-extensions
 ASCIIDOC_EXTRA += -alitdd='&\#x2d;&\#x2d;'
 DBLATEX_COMMON =
+XMLTO_EXTRA += --skip-validation
+XMLTO_EXTRA += -x manpage.xsl
 endif
 
 SHELL_PATH ?= $(SHELL)
diff --git a/Documentation/manpage.xsl b/Documentation/manpage.xsl
new file mode 100644
index 0000000000..ef64bab17a
--- /dev/null
+++ b/Documentation/manpage.xsl
@@ -0,0 +1,3 @@
+<xsl:stylesheet xmlns:xsl="http://www.w3.org/1999/XSL/Transform" version="1.0">
+	<xsl:import href="http://docbook.sourceforge.net/release/xsl-ns/current/manpages/docbook.xsl" />
+</xsl:stylesheet>
diff --git a/ci/test-documentation.sh b/ci/test-documentation.sh
index d49089832d..b3e76ef863 100755
--- a/ci/test-documentation.sh
+++ b/ci/test-documentation.sh
@@ -8,6 +8,8 @@
 filter_log () {
 	sed -e '/^GIT_VERSION = /d' \
 	    -e '/^    \* new asciidoc flags$/d' \
+	    -e '/stripped namespace before processing/d' \
+	    -e '/Attributed.*IDs for element/d' \
 	    "$1"
 }
 

Re: [PATCH v2] Documentation: fix build with Asciidoctor 2

[Jeff King]

On Fri, Sep 13, 2019 at 01:52:40AM +0000, brian m. carlson wrote:

> We also need to teach xmlto how to use the namespaced DocBook XSLT
> stylesheets instead of the non-namespaced ones it usually uses.
> Normally these stylesheets are interchangeable, but the non-namespaced
> ones have a bug that causes them not to strip whitespace automatically
> from certain elements when namespaces are in use.  This results in
> additional whitespace at the beginning of list elements, which is
> jarring and unsightly.

Thanks, this fixed most of the rendering problems I saw from the earlier
patch.

> We can do this by passing a custom stylesheet with the -x option that
> simply imports the namespaced stylesheets via a URL.  Any system with
> support for XML catalogs will automatically look this URL up and
> reference a local copy instead without us having to know where this
> local copy is located.  We know that anyone using xmlto will already
> have catalogs set up properly since the DocBook 4.5 DTD used during
> validation is also looked up via catalogs.  All major Linux
> distributions distribute the necessary stylesheets and have built-in
> catalog support, and Homebrew does as well, albeit with a requirement to
> set an environment variable to enable catalog support.

This did give me one minor hiccup: I had the debian docbook-xsl package
installed, but not docbook-xsl-ns. The error message was pretty standard
for XML: obvious if you know what catalogs are, and utterly confusing
otherwise. :)

Everything worked fine after installing docbook-xsl-ns. I wonder if
could/should provide some guidance somewhere (maybe in INSTALL, which
discusses some catalog issues?).

> Finally, we need to filter out some messages from other stylesheets that
> when invoking dblatex in the CI job.  This tool strips namespaces much

s/that/that occur/ or something?

> like the unnamespaced DocBook stylesheets and prints similar messages.
> If we permit these messages to be printed to standard error, our
> documentation CI job will because we check standard error for unexpected

s/will/will fail/?

> ---
>  Documentation/Makefile    | 4 +++-
>  Documentation/manpage.xsl | 3 +++
>  ci/test-documentation.sh  | 2 ++
>  3 files changed, 8 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/manpage.xsl

Running with this patch on asciidoctor 2.0.10, plus Martin's recent
literal-block cleanups, plus his refmiscinfo fix, I get pretty decent
output from:

  ./doc-diff --from-asciidoc --to-asciidoctor origin HEAD

The header/footer are still a little funny (but I think Martin said that
he needs to update the refmiscinfo patches for later versions of
asciidoctor, which is probably what's going on here):

  --- a/f1d4a28250629ae469fc5dd59ab843cb2fd68e12-asciidoc/home/peff/share/man/man1/git-add.1
  +++ b/6c08635fd1d38c83d3765ff05fabbfbd25ef4943-asciidoctor/home/peff/share/man/man1/git-add.1
  @@ -1,4 +1,4 @@
  -GIT-ADD(1)                        Git Manual                        GIT-ADD(1)
  +GIT-ADD(1)                                                          GIT-ADD(1)
   
   NAME
          git-add - Add file contents to the index
  @@ -356,4 +356,4 @@ SEE ALSO
   GIT
          Part of the git(1) suite
   
  -Git omitted                       01/01/1970                        GIT-ADD(1)
  +  omitted                         1970-01-01                        GIT-ADD(1)


One curiosity is that any ``smart-quotes'' now get two spaces between them
and the period of the last sentence (whereas in asciidoc they got only
one):

  -           <start> and <end> are optional. “-L <start>” or “-L <start>,” spans
  -           from <start> to end of file. “-L ,<end>” spans from start of file
  -           to <end>.
  +           <start> and <end> are optional.  “-L <start>” or “-L <start>,”
  +           spans from <start> to end of file.  “-L ,<end>” spans from start of
  +           file to <end>.

I don't think this is a big deal, but I think most of these should
actually be backticks these days (the text above is from
git-annotate.txt, which hasn't been touched in quite a while).

There are other miscellaneous indentation fixes. Most of them look
better in asciidoctor, IMHO. For example, some lists now wrap more
neatly (it looks like it's usually lists after an indented listing
block? Maybe a continuation thing?):

  -           1. This step and the next one could be combined into a single step
  -           with "checkout -b my2.6.14 v2.6.14".
  +            1. This step and the next one could be combined into a single
  +               step with "checkout -b my2.6.14 v2.6.14".

Another curiosity is that single-quote `smart-quotes' are rendered as
real smart-quotes by asciidoctor:

  -           The following features from ‘svn log’ are supported:
  +           The following features from “svn log” are supported:

The only other case I found was this one, where I think the asciidoctor
version is better (the source has literal backticks, so there shouldn't
be a visible quote; I'm guessing asciidoc got confused by the apostrophe
in "variable's"):

  -           The ‘merge.*.driver` variable’s value is used to construct a
  +           The merge.*.driver variable’s value is used to construct a command

So overall, I think we're getting very close to parity.

-Peff

Re: [PATCH v2] Documentation: fix build with Asciidoctor 2

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Fri, Sep 13, 2019 at 01:52:40AM +0000, brian m. carlson wrote:
>
>> We also need to teach xmlto how to use the namespaced DocBook XSLT
>> stylesheets instead of the non-namespaced ones it usually uses.
>> Normally these stylesheets are interchangeable, but the non-namespaced
>> ones have a bug that causes them not to strip whitespace automatically
>> from certain elements when namespaces are in use.  This results in
>> additional whitespace at the beginning of list elements, which is
>> jarring and unsightly.
>
> Thanks, this fixed most of the rendering problems I saw from the earlier
> patch.
>
>> We can do this by passing a custom stylesheet with the -x option that
>> simply imports the namespaced stylesheets via a URL.  Any system with
>> support for XML catalogs will automatically look this URL up and
>> reference a local copy instead without us having to know where this
>> local copy is located.  We know that anyone using xmlto will already
>> have catalogs set up properly since the DocBook 4.5 DTD used during
>> validation is also looked up via catalogs.  All major Linux
>> distributions distribute the necessary stylesheets and have built-in
>> catalog support, and Homebrew does as well, albeit with a requirement to
>> set an environment variable to enable catalog support.
>
> This did give me one minor hiccup: I had the debian docbook-xsl package
> installed, but not docbook-xsl-ns. The error message was pretty standard
> for XML: obvious if you know what catalogs are, and utterly confusing
> otherwise. :)
>
> Everything worked fine after installing docbook-xsl-ns. I wonder if
> could/should provide some guidance somewhere (maybe in INSTALL, which
> discusses some catalog issues?).
>
>> Finally, we need to filter out some messages from other stylesheets that
>> when invoking dblatex in the CI job.  This tool strips namespaces much
>
> s/that/that occur/ or something?
>
>> like the unnamespaced DocBook stylesheets and prints similar messages.
>> If we permit these messages to be printed to standard error, our
>> documentation CI job will because we check standard error for unexpected
>
> s/will/will fail/?
>
>> ---
>>  Documentation/Makefile    | 4 +++-
>>  Documentation/manpage.xsl | 3 +++
>>  ci/test-documentation.sh  | 2 ++
>>  3 files changed, 8 insertions(+), 1 deletion(-)
>>  create mode 100644 Documentation/manpage.xsl
>
> Running with this patch on asciidoctor 2.0.10, plus Martin's recent
> literal-block cleanups, plus his refmiscinfo fix, I get pretty decent
> output from:
>
>   ./doc-diff --from-asciidoc --to-asciidoctor origin HEAD
>
> The header/footer are still a little funny (but I think Martin said that
> he needs to update the refmiscinfo patches for later versions of
> asciidoctor, which is probably what's going on here):
> ...
> So overall, I think we're getting very close to parity.

Thanks, both.  Have queued with your log message typofixes.

Re: [PATCH v2] Documentation: fix build with Asciidoctor 2

[Martin Ågren]

On Fri, 13 Sep 2019 at 07:06, Jeff King <peff@peff.net> wrote:
>
> On Fri, Sep 13, 2019 at 01:52:40AM +0000, brian m. carlson wrote:
>
>
> >  Documentation/Makefile    | 4 +++-
> >  Documentation/manpage.xsl | 3 +++
> >  ci/test-documentation.sh  | 2 ++
> >  3 files changed, 8 insertions(+), 1 deletion(-)
> >  create mode 100644 Documentation/manpage.xsl
>
> Running with this patch on asciidoctor 2.0.10, plus Martin's recent
> literal-block cleanups, plus his refmiscinfo fix, I get pretty decent
> output from:
>
>   ./doc-diff --from-asciidoc --to-asciidoctor origin HEAD
>
> The header/footer are still a little funny (but I think Martin said that
> he needs to update the refmiscinfo patches for later versions of
> asciidoctor, which is probably what's going on here):
>
>   --- a/f1d4a28250629ae469fc5dd59ab843cb2fd68e12-asciidoc/home/peff/share/man/man1/git-add.1
>   +++ b/6c08635fd1d38c83d3765ff05fabbfbd25ef4943-asciidoctor/home/peff/share/man/man1/git-add.1
>   @@ -1,4 +1,4 @@
>   -GIT-ADD(1)                        Git Manual                        GIT-ADD(1)
>   +GIT-ADD(1)                                                          GIT-ADD(1)
>
>    NAME
>           git-add - Add file contents to the index
>   @@ -356,4 +356,4 @@ SEE ALSO
>    GIT
>           Part of the git(1) suite
>
>   -Git omitted                       01/01/1970                        GIT-ADD(1)
>   +  omitted                         1970-01-01                        GIT-ADD(1)

Yeah, I should be able to post v3 of my refmiscinfo-series this evening,
which should fix this, so that the only difference that remains here is
how the date is formatted.

Martin

Re: [PATCH v2] Documentation: fix build with Asciidoctor 2

[Junio C Hamano]

Martin Ågren <martin.agren@gmail.com> writes:

>>   -Git omitted                       01/01/1970                        GIT-ADD(1)
>>   +  omitted                         1970-01-01                        GIT-ADD(1)
>
> Yeah, I should be able to post v3 of my refmiscinfo-series this evening,
> which should fix this, so that the only difference that remains here is
> how the date is formatted.

Thanks.

Re: [PATCH v2] Documentation: fix build with Asciidoctor 2

[SZEDER Gábor]

On Fri, Sep 13, 2019 at 01:52:40AM +0000, brian m. carlson wrote:

> We also need to teach xmlto how to use the namespaced DocBook XSLT
> stylesheets instead of the non-namespaced ones it usually uses.
> Normally these stylesheets are interchangeable, but the non-namespaced
> ones have a bug that causes them not to strip whitespace automatically
> from certain elements when namespaces are in use.  This results in
> additional whitespace at the beginning of list elements, which is
> jarring and unsightly.
> 
> We can do this by passing a custom stylesheet with the -x option that
> simply imports the namespaced stylesheets via a URL.  Any system with
> support for XML catalogs will automatically look this URL up and
> reference a local copy instead without us having to know where this
> local copy is located.  We know that anyone using xmlto will already
> have catalogs set up properly since the DocBook 4.5 DTD used during
> validation is also looked up via catalogs.  All major Linux
> distributions distribute the necessary stylesheets and have built-in
> catalog support, and Homebrew does as well, albeit with a requirement to
> set an environment variable to enable catalog support.
> 
> On the off chance that someone lacks support for catalogs, it is
> possible for xmlto (via xmllint) to download the stylesheets from the
> URLs in question, although this will likely perform poorly enough to
> attract attention.  People still have the option of using the prebuilt
> documentation that we ship, so happily this should not be an impediment.


> diff --git a/Documentation/Makefile b/Documentation/Makefile
> index 76f2ecfc1b..d94f47c5c9 100644
> --- a/Documentation/Makefile
> +++ b/Documentation/Makefile
> @@ -197,11 +197,13 @@ ifdef USE_ASCIIDOCTOR
>  ASCIIDOC = asciidoctor
>  ASCIIDOC_CONF =
>  ASCIIDOC_HTML = xhtml5
> -ASCIIDOC_DOCBOOK = docbook45
> +ASCIIDOC_DOCBOOK = docbook5
>  ASCIIDOC_EXTRA += -acompat-mode -atabsize=8
>  ASCIIDOC_EXTRA += -I. -rasciidoctor-extensions
>  ASCIIDOC_EXTRA += -alitdd='&\#x2d;&\#x2d;'
>  DBLATEX_COMMON =
> +XMLTO_EXTRA += --skip-validation
> +XMLTO_EXTRA += -x manpage.xsl
>  endif
>  
>  SHELL_PATH ?= $(SHELL)
> diff --git a/Documentation/manpage.xsl b/Documentation/manpage.xsl
> new file mode 100644
> index 0000000000..ef64bab17a
> --- /dev/null
> +++ b/Documentation/manpage.xsl
> @@ -0,0 +1,3 @@
> +<xsl:stylesheet xmlns:xsl="http://www.w3.org/1999/XSL/Transform" version="1.0">
> +	<xsl:import href="http://docbook.sourceforge.net/release/xsl-ns/current/manpages/docbook.xsl" />
> +</xsl:stylesheet>

Unfortunately, five out of five CI builds failed with the following:

      XMLTO git-revert.1
  I/O error : Attempt to load network entity http://docbook.sourceforge.net/release/xsl-ns/current/manpages/docbook.xsl
  warning: failed to load external entity "http://docbook.sourceforge.net/release/xsl-ns/current/manpages/docbook.xsl"
  compilation error: file /home/travis/build/git/git/Documentation/manpage.xsl line 2 element import
  xsl:import : unable to load http://docbook.sourceforge.net/release/xsl-ns/current/manpages/docbook.xsl
  Makefile:375: recipe for target 'git-revert.1' failed

https://travis-ci.org/git/git/jobs/584794387#L1552

Re: [PATCH v2] Documentation: fix build with Asciidoctor 2

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 887 bytes --]

On 2019-09-14 at 07:53:01, SZEDER Gábor wrote:
> Unfortunately, five out of five CI builds failed with the following:
> 
>       XMLTO git-revert.1
>   I/O error : Attempt to load network entity http://docbook.sourceforge.net/release/xsl-ns/current/manpages/docbook.xsl
>   warning: failed to load external entity "http://docbook.sourceforge.net/release/xsl-ns/current/manpages/docbook.xsl"
>   compilation error: file /home/travis/build/git/git/Documentation/manpage.xsl line 2 element import
>   xsl:import : unable to load http://docbook.sourceforge.net/release/xsl-ns/current/manpages/docbook.xsl
>   Makefile:375: recipe for target 'git-revert.1' failed
> 
> https://travis-ci.org/git/git/jobs/584794387#L1552

Ah, I forgot to install the packages in CI.  I'll send a v3 with that
fixed.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]

[PATCH v3] Documentation: fix build with Asciidoctor 2

[brian m. carlson]

Our documentation toolchain has traditionally been built around DocBook
4.5.  This version of DocBook is the last DTD-based version of DocBook.
In 2009, DocBook 5 was introduced using namespaces and its syntax is
expressed in RELAX NG, which is more expressive and allows a wider
variety of syntax forms.

Asciidoctor, one of the alternatives for building our documentation,
moved support for DocBook 4.5 out of core in its recent 2.0 release and
now only supports DocBook 5 in the main release.  The DocBoook 4.5
converter is still available as a separate component, but this is not
available in most distro packages.  This would not be a problem but for
the fact that we use xmlto, which is still stuck in the DocBook 4.5 era.

xmlto performs DTD validation as part of the build process.  This is not
problematic for DocBook 4.5, which has a valid DTD, but it clearly
cannot work for DocBook 5, since no DTD can adequately express its full
syntax.  In addition, even if xmlto did support RELAX NG validation,
that wouldn't be sufficient because it uses the libxml2-based xmllint to
do so, which has known problems with validating interleaves in RELAX NG.

Fortunately, there's an easy way forward: ask Asciidoctor to use its
DocBook 5 backend and tell xmlto to skip validation.  Asciidoctor has
supported DocBook 5 since v0.1.4 in 2013 and xmlto has supported
skipping validation for probably longer than that.

We also need to teach xmlto how to use the namespaced DocBook XSLT
stylesheets instead of the non-namespaced ones it usually uses.
Normally these stylesheets are interchangeable, but the non-namespaced
ones have a bug that causes them not to strip whitespace automatically
from certain elements when namespaces are in use.  This results in
additional whitespace at the beginning of list elements, which is
jarring and unsightly.

We can do this by passing a custom stylesheet with the -x option that
simply imports the namespaced stylesheets via a URL.  Any system with
support for XML catalogs will automatically look this URL up and
reference a local copy instead without us having to know where this
local copy is located.  We know that anyone using xmlto will already
have catalogs set up properly since the DocBook 4.5 DTD used during
validation is also looked up via catalogs.  All major Linux
distributions distribute the necessary stylesheets and have built-in
catalog support, and Homebrew does as well, albeit with a requirement to
set an environment variable to enable catalog support.

On the off chance that someone lacks support for catalogs, it is
possible for xmlto (via xmllint) to download the stylesheets from the
URLs in question, although this will likely perform poorly enough to
attract attention.  People still have the option of using the prebuilt
documentation that we ship, so happily this should not be an impediment.

Finally, we need to filter out some messages from other stylesheets that
occur when invoking dblatex in the CI job.  This tool strips namespaces
much like the unnamespaced DocBook stylesheets and prints similar
messages.  If we permit these messages to be printed to standard error,
our documentation CI job will fail because we check standard error for
unexpected output.  Due to dblatex's reliance on Python 2, we may need
to revisit its use in the future, in which case this problem may go
away, but this can be delayed until a future patch.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
Range-diff against v2:
1:  6f11a79d03 ! 1:  c4935af5de Documentation: fix build with Asciidoctor 2
    @@ Commit message
         documentation that we ship, so happily this should not be an impediment.
     
         Finally, we need to filter out some messages from other stylesheets that
    -    when invoking dblatex in the CI job.  This tool strips namespaces much
    -    like the unnamespaced DocBook stylesheets and prints similar messages.
    -    If we permit these messages to be printed to standard error, our
    -    documentation CI job will because we check standard error for unexpected
    -    output.  Due to dblatex's reliance on Python 2, we may need to revisit
    -    its use in the future, in which case this problem may go away, but this
    -    can be delayed until a future patch.
    +    occur when invoking dblatex in the CI job.  This tool strips namespaces
    +    much like the unnamespaced DocBook stylesheets and prints similar
    +    messages.  If we permit these messages to be printed to standard error,
    +    our documentation CI job will fail because we check standard error for
    +    unexpected output.  Due to dblatex's reliance on Python 2, we may need
    +    to revisit its use in the future, in which case this problem may go
    +    away, but this can be delayed until a future patch.
     
         Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
     
    @@ Documentation/manpage.xsl (new)
     +	<xsl:import href="http://docbook.sourceforge.net/release/xsl-ns/current/manpages/docbook.xsl" />
     +</xsl:stylesheet>
     
    + ## azure-pipelines.yml ##
    +@@ azure-pipelines.yml: jobs:
    +        test "$GITFILESHAREPWD" = '$(gitfileshare.pwd)' || ci/mount-fileshare.sh //gitfileshare.file.core.windows.net/test-cache gitfileshare "$GITFILESHAREPWD" "$HOME/test-cache" || exit 1
    + 
    +        sudo apt-get update &&
    +-       sudo apt-get install -y asciidoc xmlto asciidoctor &&
    ++       sudo apt-get install -y asciidoc xmlto asciidoctor docbook-xsl-ns &&
    + 
    +        export ALREADY_HAVE_ASCIIDOCTOR=yes. &&
    +        export jobname=Documentation &&
    +
    + ## ci/install-dependencies.sh ##
    +@@ ci/install-dependencies.sh: StaticAnalysis)
    + 	;;
    + Documentation)
    + 	sudo apt-get -q update
    +-	sudo apt-get -q -y install asciidoc xmlto
    ++	sudo apt-get -q -y install asciidoc xmlto docbook-xsl-ns
    + 
    + 	test -n "$ALREADY_HAVE_ASCIIDOCTOR" ||
    + 	gem install --version 1.5.8 asciidoctor
    +
      ## ci/test-documentation.sh ##
     @@
      filter_log () {

 Documentation/Makefile     | 4 +++-
 Documentation/manpage.xsl  | 3 +++
 azure-pipelines.yml        | 2 +-
 ci/install-dependencies.sh | 2 +-
 ci/test-documentation.sh   | 2 ++
 5 files changed, 10 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/manpage.xsl

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 76f2ecfc1b..d94f47c5c9 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -197,11 +197,13 @@ ifdef USE_ASCIIDOCTOR
 ASCIIDOC = asciidoctor
 ASCIIDOC_CONF =
 ASCIIDOC_HTML = xhtml5
-ASCIIDOC_DOCBOOK = docbook45
+ASCIIDOC_DOCBOOK = docbook5
 ASCIIDOC_EXTRA += -acompat-mode -atabsize=8
 ASCIIDOC_EXTRA += -I. -rasciidoctor-extensions
 ASCIIDOC_EXTRA += -alitdd='&\#x2d;&\#x2d;'
 DBLATEX_COMMON =
+XMLTO_EXTRA += --skip-validation
+XMLTO_EXTRA += -x manpage.xsl
 endif
 
 SHELL_PATH ?= $(SHELL)
diff --git a/Documentation/manpage.xsl b/Documentation/manpage.xsl
new file mode 100644
index 0000000000..ef64bab17a
--- /dev/null
+++ b/Documentation/manpage.xsl
@@ -0,0 +1,3 @@
+<xsl:stylesheet xmlns:xsl="http://www.w3.org/1999/XSL/Transform" version="1.0">
+	<xsl:import href="http://docbook.sourceforge.net/release/xsl-ns/current/manpages/docbook.xsl" />
+</xsl:stylesheet>
diff --git a/azure-pipelines.yml b/azure-pipelines.yml
index c329b7218b..34031b182a 100644
--- a/azure-pipelines.yml
+++ b/azure-pipelines.yml
@@ -374,7 +374,7 @@ jobs:
        test "$GITFILESHAREPWD" = '$(gitfileshare.pwd)' || ci/mount-fileshare.sh //gitfileshare.file.core.windows.net/test-cache gitfileshare "$GITFILESHAREPWD" "$HOME/test-cache" || exit 1
 
        sudo apt-get update &&
-       sudo apt-get install -y asciidoc xmlto asciidoctor &&
+       sudo apt-get install -y asciidoc xmlto asciidoctor docbook-xsl-ns &&
 
        export ALREADY_HAVE_ASCIIDOCTOR=yes. &&
        export jobname=Documentation &&
diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index 8cc72503cb..a76f348484 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -53,7 +53,7 @@ StaticAnalysis)
 	;;
 Documentation)
 	sudo apt-get -q update
-	sudo apt-get -q -y install asciidoc xmlto
+	sudo apt-get -q -y install asciidoc xmlto docbook-xsl-ns
 
 	test -n "$ALREADY_HAVE_ASCIIDOCTOR" ||
 	gem install --version 1.5.8 asciidoctor
diff --git a/ci/test-documentation.sh b/ci/test-documentation.sh
index d49089832d..b3e76ef863 100755
--- a/ci/test-documentation.sh
+++ b/ci/test-documentation.sh
@@ -8,6 +8,8 @@
 filter_log () {
 	sed -e '/^GIT_VERSION = /d' \
 	    -e '/^    \* new asciidoc flags$/d' \
+	    -e '/stripped namespace before processing/d' \
+	    -e '/Attributed.*IDs for element/d' \
 	    "$1"
 }
 

Re: [PATCH v3] Documentation: fix build with Asciidoctor 2

[SZEDER Gábor]

On Sat, Sep 14, 2019 at 07:49:19PM +0000, brian m. carlson wrote:
> Our documentation toolchain has traditionally been built around DocBook
> 4.5.  This version of DocBook is the last DTD-based version of DocBook.
> In 2009, DocBook 5 was introduced using namespaces and its syntax is
> expressed in RELAX NG, which is more expressive and allows a wider
> variety of syntax forms.
> 
> Asciidoctor, one of the alternatives for building our documentation,
> moved support for DocBook 4.5 out of core in its recent 2.0 release and
> now only supports DocBook 5 in the main release.  The DocBoook 4.5
> converter is still available as a separate component, but this is not
> available in most distro packages.  This would not be a problem but for
> the fact that we use xmlto, which is still stuck in the DocBook 4.5 era.
> 
> xmlto performs DTD validation as part of the build process.  This is not
> problematic for DocBook 4.5, which has a valid DTD, but it clearly
> cannot work for DocBook 5, since no DTD can adequately express its full
> syntax.  In addition, even if xmlto did support RELAX NG validation,
> that wouldn't be sufficient because it uses the libxml2-based xmllint to
> do so, which has known problems with validating interleaves in RELAX NG.
> 
> Fortunately, there's an easy way forward: ask Asciidoctor to use its
> DocBook 5 backend and tell xmlto to skip validation.  Asciidoctor has
> supported DocBook 5 since v0.1.4 in 2013 and xmlto has supported
> skipping validation for probably longer than that.
> 
> We also need to teach xmlto how to use the namespaced DocBook XSLT
> stylesheets instead of the non-namespaced ones it usually uses.
> Normally these stylesheets are interchangeable, but the non-namespaced
> ones have a bug that causes them not to strip whitespace automatically
> from certain elements when namespaces are in use.  This results in
> additional whitespace at the beginning of list elements, which is
> jarring and unsightly.
> 
> We can do this by passing a custom stylesheet with the -x option that
> simply imports the namespaced stylesheets via a URL.  Any system with
> support for XML catalogs will automatically look this URL up and
> reference a local copy instead without us having to know where this
> local copy is located.  We know that anyone using xmlto will already
> have catalogs set up properly since the DocBook 4.5 DTD used during
> validation is also looked up via catalogs.  All major Linux
> distributions distribute the necessary stylesheets and have built-in
> catalog support, and Homebrew does as well, albeit with a requirement to
> set an environment variable to enable catalog support.
> 
> On the off chance that someone lacks support for catalogs, it is
> possible for xmlto (via xmllint) to download the stylesheets from the
> URLs in question, although this will likely perform poorly enough to
> attract attention.  People still have the option of using the prebuilt
> documentation that we ship, so happily this should not be an impediment.
> 
> Finally, we need to filter out some messages from other stylesheets that
> occur when invoking dblatex in the CI job.  This tool strips namespaces
> much like the unnamespaced DocBook stylesheets and prints similar
> messages.  If we permit these messages to be printed to standard error,
> our documentation CI job will fail because we check standard error for
> unexpected output.  Due to dblatex's reliance on Python 2, we may need
> to revisit its use in the future, in which case this problem may go
> away, but this can be delayed until a future patch.
> 
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>

>  Documentation/Makefile     | 4 +++-
>  Documentation/manpage.xsl  | 3 +++
>  azure-pipelines.yml        | 2 +-
>  ci/install-dependencies.sh | 2 +-
>  ci/test-documentation.sh   | 2 ++
>  5 files changed, 10 insertions(+), 3 deletions(-)
>  create mode 100644 Documentation/manpage.xsl
> 
> diff --git a/Documentation/Makefile b/Documentation/Makefile
> index 76f2ecfc1b..d94f47c5c9 100644
> --- a/Documentation/Makefile
> +++ b/Documentation/Makefile
> @@ -197,11 +197,13 @@ ifdef USE_ASCIIDOCTOR
>  ASCIIDOC = asciidoctor
>  ASCIIDOC_CONF =
>  ASCIIDOC_HTML = xhtml5
> -ASCIIDOC_DOCBOOK = docbook45
> +ASCIIDOC_DOCBOOK = docbook5
>  ASCIIDOC_EXTRA += -acompat-mode -atabsize=8
>  ASCIIDOC_EXTRA += -I. -rasciidoctor-extensions
>  ASCIIDOC_EXTRA += -alitdd='&\#x2d;&\#x2d;'
>  DBLATEX_COMMON =
> +XMLTO_EXTRA += --skip-validation
> +XMLTO_EXTRA += -x manpage.xsl
>  endif
>  
>  SHELL_PATH ?= $(SHELL)
> diff --git a/Documentation/manpage.xsl b/Documentation/manpage.xsl
> new file mode 100644
> index 0000000000..ef64bab17a
> --- /dev/null
> +++ b/Documentation/manpage.xsl
> @@ -0,0 +1,3 @@
> +<xsl:stylesheet xmlns:xsl="http://www.w3.org/1999/XSL/Transform" version="1.0">
> +	<xsl:import href="http://docbook.sourceforge.net/release/xsl-ns/current/manpages/docbook.xsl" />
> +</xsl:stylesheet>
> diff --git a/azure-pipelines.yml b/azure-pipelines.yml
> index c329b7218b..34031b182a 100644
> --- a/azure-pipelines.yml
> +++ b/azure-pipelines.yml
> @@ -374,7 +374,7 @@ jobs:
>         test "$GITFILESHAREPWD" = '$(gitfileshare.pwd)' || ci/mount-fileshare.sh //gitfileshare.file.core.windows.net/test-cache gitfileshare "$GITFILESHAREPWD" "$HOME/test-cache" || exit 1
>  
>         sudo apt-get update &&
> -       sudo apt-get install -y asciidoc xmlto asciidoctor &&
> +       sudo apt-get install -y asciidoc xmlto asciidoctor docbook-xsl-ns &&
>  
>         export ALREADY_HAVE_ASCIIDOCTOR=yes. &&
>         export jobname=Documentation &&
> diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
> index 8cc72503cb..a76f348484 100755
> --- a/ci/install-dependencies.sh
> +++ b/ci/install-dependencies.sh
> @@ -53,7 +53,7 @@ StaticAnalysis)
>  	;;
>  Documentation)
>  	sudo apt-get -q update
> -	sudo apt-get -q -y install asciidoc xmlto
> +	sudo apt-get -q -y install asciidoc xmlto docbook-xsl-ns

Ok, with this package installed the build passed on Travis CI.

>  	test -n "$ALREADY_HAVE_ASCIIDOCTOR" ||
>  	gem install --version 1.5.8 asciidoctor

So, since the documentation can now be built with Asciidoctor v2, is
it already time to remove this '--version 1.5.8'?

> diff --git a/ci/test-documentation.sh b/ci/test-documentation.sh
> index d49089832d..b3e76ef863 100755
> --- a/ci/test-documentation.sh
> +++ b/ci/test-documentation.sh
> @@ -8,6 +8,8 @@
>  filter_log () {
>  	sed -e '/^GIT_VERSION = /d' \
>  	    -e '/^    \* new asciidoc flags$/d' \
> +	    -e '/stripped namespace before processing/d' \
> +	    -e '/Attributed.*IDs for element/d' \

I haven't seen this latter message in the CI builds, neither with
Asciidoctor v1.5.8 nor with v2.  Do we really need this filter, then?
Where does this message come from?

>  	    "$1"
>  }
>  

Re: [PATCH v3] Documentation: fix build with Asciidoctor 2

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 1446 bytes --]

On 2019-09-15 at 09:59:52, SZEDER Gábor wrote:
> On Sat, Sep 14, 2019 at 07:49:19PM +0000, brian m. carlson wrote:
> >  	test -n "$ALREADY_HAVE_ASCIIDOCTOR" ||
> >  	gem install --version 1.5.8 asciidoctor
> 
> So, since the documentation can now be built with Asciidoctor v2, is
> it already time to remove this '--version 1.5.8'?

I think Martin was going to send in some more patches before we did
that.

> > diff --git a/ci/test-documentation.sh b/ci/test-documentation.sh
> > index d49089832d..b3e76ef863 100755
> > --- a/ci/test-documentation.sh
> > +++ b/ci/test-documentation.sh
> > @@ -8,6 +8,8 @@
> >  filter_log () {
> >  	sed -e '/^GIT_VERSION = /d' \
> >  	    -e '/^    \* new asciidoc flags$/d' \
> > +	    -e '/stripped namespace before processing/d' \
> > +	    -e '/Attributed.*IDs for element/d' \
> 
> I haven't seen this latter message in the CI builds, neither with
> Asciidoctor v1.5.8 nor with v2.  Do we really need this filter, then?
> Where does this message come from?

I see it and it definitely fails on my system without it.  It comes from
libxslt, which has been patched in Debian to produce deterministic IDs.
I suspect we may not have seen it on Ubuntu systems because they are
running 16.04, which is likely older than the patch.  If Travis updates
to 18.04, we may be more likely to have a problem.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]

Re: [PATCH v3] Documentation: fix build with Asciidoctor 2

[SZEDER Gábor]

On Sun, Sep 15, 2019 at 09:26:21PM +0000, brian m. carlson wrote:
> > > diff --git a/ci/test-documentation.sh b/ci/test-documentation.sh
> > > index d49089832d..b3e76ef863 100755
> > > --- a/ci/test-documentation.sh
> > > +++ b/ci/test-documentation.sh
> > > @@ -8,6 +8,8 @@
> > >  filter_log () {
> > >  	sed -e '/^GIT_VERSION = /d' \
> > >  	    -e '/^    \* new asciidoc flags$/d' \
> > > +	    -e '/stripped namespace before processing/d' \
> > > +	    -e '/Attributed.*IDs for element/d' \
> > 
> > I haven't seen this latter message in the CI builds, neither with
> > Asciidoctor v1.5.8 nor with v2.  Do we really need this filter, then?
> > Where does this message come from?
> 
> I see it and it definitely fails on my system without it.  It comes from
> libxslt, which has been patched in Debian to produce deterministic IDs.
> I suspect we may not have seen it on Ubuntu systems because they are
> running 16.04, which is likely older than the patch.  If Travis updates
> to 18.04, we may be more likely to have a problem.

Thanks.  Indeed, I kicked off a Travis CI build using their Ubuntu
18.04 image, and that "Attributed..." message was there.

I think this future-proofing is a good idea, but I also think that
this should be clarified in the commit message.

Re: [PATCH v3] Documentation: fix build with Asciidoctor 2

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 1661 bytes --]

On 2019-09-15 at 22:05:55, SZEDER Gábor wrote:
> On Sun, Sep 15, 2019 at 09:26:21PM +0000, brian m. carlson wrote:
> > > > diff --git a/ci/test-documentation.sh b/ci/test-documentation.sh
> > > > index d49089832d..b3e76ef863 100755
> > > > --- a/ci/test-documentation.sh
> > > > +++ b/ci/test-documentation.sh
> > > > @@ -8,6 +8,8 @@
> > > >  filter_log () {
> > > >  	sed -e '/^GIT_VERSION = /d' \
> > > >  	    -e '/^    \* new asciidoc flags$/d' \
> > > > +	    -e '/stripped namespace before processing/d' \
> > > > +	    -e '/Attributed.*IDs for element/d' \
> > > 
> > > I haven't seen this latter message in the CI builds, neither with
> > > Asciidoctor v1.5.8 nor with v2.  Do we really need this filter, then?
> > > Where does this message come from?
> > 
> > I see it and it definitely fails on my system without it.  It comes from
> > libxslt, which has been patched in Debian to produce deterministic IDs.
> > I suspect we may not have seen it on Ubuntu systems because they are
> > running 16.04, which is likely older than the patch.  If Travis updates
> > to 18.04, we may be more likely to have a problem.
> 
> Thanks.  Indeed, I kicked off a Travis CI build using their Ubuntu
> 18.04 image, and that "Attributed..." message was there.
> 
> I think this future-proofing is a good idea, but I also think that
> this should be clarified in the commit message.

I can do that.  I just noticed it failed on my laptop and added it,
assuming it was the stylesheets.  I had to search Google for the output
to find out that it was libxslt.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]

Re: [PATCH v3] Documentation: fix build with Asciidoctor 2

[Martin Ågren]

On Sun, 15 Sep 2019 at 23:26, brian m. carlson
<sandals@crustytoothpaste.net> wrote:
>
> On 2019-09-15 at 09:59:52, SZEDER Gábor wrote:
> > On Sat, Sep 14, 2019 at 07:49:19PM +0000, brian m. carlson wrote:
> > >     test -n "$ALREADY_HAVE_ASCIIDOCTOR" ||
> > >     gem install --version 1.5.8 asciidoctor
> >
> > So, since the documentation can now be built with Asciidoctor v2, is
> > it already time to remove this '--version 1.5.8'?
>
> I think Martin was going to send in some more patches before we did
> that.

I've got two series floating around [1] [2]. I'll be rerolling [2]
hopefully tonight and then I feel pretty good about the manpages. I've
got a third series that I'll get to then, which fixes up the rendering
of user-manual.pdf/html with Asciidoctor. Assuming those three series
graduate, I'm not aware of any reason to hold off on the switch to
"Asciidoctor by default".

As for "v2.x vs 1.5.8", that seems like a separate issue to me, though
-- and one that your patch does a very good job at! My series [2] will
make the rendering with v2.x /prettier/, but since your series makes the
docs build /at all/, maybe it's worth switching the CI-job sooner rather
than later to make sure we keep it so.

[1] https://public-inbox.org/git/cover.1567707999.git.martin.agren@gmail.com/

[2] https://public-inbox.org/git/cover.1567534373.git.martin.agren@gmail.com/

Martin

[PATCH v4] Documentation: fix build with Asciidoctor 2

[brian m. carlson]

Our documentation toolchain has traditionally been built around DocBook
4.5.  This version of DocBook is the last DTD-based version of DocBook.
In 2009, DocBook 5 was introduced using namespaces and its syntax is
expressed in RELAX NG, which is more expressive and allows a wider
variety of syntax forms.

Asciidoctor, one of the alternatives for building our documentation,
moved support for DocBook 4.5 out of core in its recent 2.0 release and
now only supports DocBook 5 in the main release.  The DocBoook 4.5
converter is still available as a separate component, but this is not
available in most distro packages.  This would not be a problem but for
the fact that we use xmlto, which is still stuck in the DocBook 4.5 era.

xmlto performs DTD validation as part of the build process.  This is not
problematic for DocBook 4.5, which has a valid DTD, but it clearly
cannot work for DocBook 5, since no DTD can adequately express its full
syntax.  In addition, even if xmlto did support RELAX NG validation,
that wouldn't be sufficient because it uses the libxml2-based xmllint to
do so, which has known problems with validating interleaves in RELAX NG.

Fortunately, there's an easy way forward: ask Asciidoctor to use its
DocBook 5 backend and tell xmlto to skip validation.  Asciidoctor has
supported DocBook 5 since v0.1.4 in 2013 and xmlto has supported
skipping validation for probably longer than that.

We also need to teach xmlto how to use the namespaced DocBook XSLT
stylesheets instead of the non-namespaced ones it usually uses.
Normally these stylesheets are interchangeable, but the non-namespaced
ones have a bug that causes them not to strip whitespace automatically
from certain elements when namespaces are in use.  This results in
additional whitespace at the beginning of list elements, which is
jarring and unsightly.

We can do this by passing a custom stylesheet with the -x option that
simply imports the namespaced stylesheets via a URL.  Any system with
support for XML catalogs will automatically look this URL up and
reference a local copy instead without us having to know where this
local copy is located.  We know that anyone using xmlto will already
have catalogs set up properly since the DocBook 4.5 DTD used during
validation is also looked up via catalogs.  All major Linux
distributions distribute the necessary stylesheets and have built-in
catalog support, and Homebrew does as well, albeit with a requirement to
set an environment variable to enable catalog support.

On the off chance that someone lacks support for catalogs, it is
possible for xmlto (via xmllint) to download the stylesheets from the
URLs in question, although this will likely perform poorly enough to
attract attention.  People still have the option of using the prebuilt
documentation that we ship, so happily this should not be an impediment.

Finally, we need to filter out some messages from other stylesheets that
occur when invoking dblatex in the CI job.  This tool strips namespaces
much like the unnamespaced DocBook stylesheets and prints similar
messages.  If we permit these messages to be printed to standard error,
our documentation CI job will fail because we check standard error for
unexpected output.  Due to dblatex's reliance on Python 2, we may need
to revisit its use in the future, in which case this problem may go
away, but this can be delayed until a future patch.

The final message we filter is due to libxslt on modern Debian and
Ubuntu.  The patch which they use to implement reproducible ID
generation also prints messages about the ID generation.  While this
doesn't affect our current CI images since they use Ubuntu 16.04 which
lacks this patch, if we upgrade to Ubuntu 18.04 or a modern Debian,
these messages will appear and, like the above messages, cause a CI
failure.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
Changes since v3:
* Further expanded the commit message to include information about
  libxslt.  It is now just over a hundred words away from classification
  as "sudden nonfiction".

 Documentation/Makefile     | 4 +++-
 Documentation/manpage.xsl  | 3 +++
 azure-pipelines.yml        | 2 +-
 ci/install-dependencies.sh | 2 +-
 ci/test-documentation.sh   | 2 ++
 5 files changed, 10 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/manpage.xsl

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 76f2ecfc1b..d94f47c5c9 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -197,11 +197,13 @@ ifdef USE_ASCIIDOCTOR
 ASCIIDOC = asciidoctor
 ASCIIDOC_CONF =
 ASCIIDOC_HTML = xhtml5
-ASCIIDOC_DOCBOOK = docbook45
+ASCIIDOC_DOCBOOK = docbook5
 ASCIIDOC_EXTRA += -acompat-mode -atabsize=8
 ASCIIDOC_EXTRA += -I. -rasciidoctor-extensions
 ASCIIDOC_EXTRA += -alitdd='&\#x2d;&\#x2d;'
 DBLATEX_COMMON =
+XMLTO_EXTRA += --skip-validation
+XMLTO_EXTRA += -x manpage.xsl
 endif
 
 SHELL_PATH ?= $(SHELL)
diff --git a/Documentation/manpage.xsl b/Documentation/manpage.xsl
new file mode 100644
index 0000000000..ef64bab17a
--- /dev/null
+++ b/Documentation/manpage.xsl
@@ -0,0 +1,3 @@
+<xsl:stylesheet xmlns:xsl="http://www.w3.org/1999/XSL/Transform" version="1.0">
+	<xsl:import href="http://docbook.sourceforge.net/release/xsl-ns/current/manpages/docbook.xsl" />
+</xsl:stylesheet>
diff --git a/azure-pipelines.yml b/azure-pipelines.yml
index c329b7218b..34031b182a 100644
--- a/azure-pipelines.yml
+++ b/azure-pipelines.yml
@@ -374,7 +374,7 @@ jobs:
        test "$GITFILESHAREPWD" = '$(gitfileshare.pwd)' || ci/mount-fileshare.sh //gitfileshare.file.core.windows.net/test-cache gitfileshare "$GITFILESHAREPWD" "$HOME/test-cache" || exit 1
 
        sudo apt-get update &&
-       sudo apt-get install -y asciidoc xmlto asciidoctor &&
+       sudo apt-get install -y asciidoc xmlto asciidoctor docbook-xsl-ns &&
 
        export ALREADY_HAVE_ASCIIDOCTOR=yes. &&
        export jobname=Documentation &&
diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index 8cc72503cb..a76f348484 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -53,7 +53,7 @@ StaticAnalysis)
 	;;
 Documentation)
 	sudo apt-get -q update
-	sudo apt-get -q -y install asciidoc xmlto
+	sudo apt-get -q -y install asciidoc xmlto docbook-xsl-ns
 
 	test -n "$ALREADY_HAVE_ASCIIDOCTOR" ||
 	gem install --version 1.5.8 asciidoctor
diff --git a/ci/test-documentation.sh b/ci/test-documentation.sh
index d49089832d..b3e76ef863 100755
--- a/ci/test-documentation.sh
+++ b/ci/test-documentation.sh
@@ -8,6 +8,8 @@
 filter_log () {
 	sed -e '/^GIT_VERSION = /d' \
 	    -e '/^    \* new asciidoc flags$/d' \
+	    -e '/stripped namespace before processing/d' \
+	    -e '/Attributed.*IDs for element/d' \
 	    "$1"
 }
 

[PATCH v3 0/3] asciidoctor-extensions: provide `<refmiscinfo/>`

[Martin Ågren]

Here's v3 of ma/asciidoctor-refmiscinfo to align the headers and footers
of Asciidoctor-rendered manpages with those from AsciiDoc. Patch 1 is
new.

It turns out that with newer (>=1.5.7) versions of Asciidoctor, we get
default values of these refmiscinfo attributes put into the xml. The
hack in patch 2/3 (previously patch 1/2) happened to inject these
attributes after the default ones and it seems like the earlier ones
happen to win when xmlto processes the xml. This explains what Peff
noticed in [1], where the attribute values introduced by the hacky patch
2 are effectively ignored and blank values are used instead.

Rather than trying to game the order of those attributes, I decided to
do a patch 1 which uses a saner way of providing these attributes to
Asciidoctor. This allowed me to reframe patch 2 from "fix all of these
attributes with Asciidoctor" to "fix some of them with some Asciidoctor
versions".

My test results:

  AsciiDoc (8.6.10): Each patch in this series is a no-op.

  Asciidoctor 1.5.5 and 1.5.6: Patch 1/3 is a no-op. Patch 2/3 fixes the
  header/footer to be as with AsciiDoc, except for how the date is
  rendered.

  Aciidoctor 1.5.7 and 2.0.10 [2]: Patch 1/3 fixes the header/footer
  partially -- the version number is still missing. Patch 2/3 fixes the
  version number, after which the header/footer are just as with
  AsciiDoc, except for how the date is rendered.

  So in short, "all" versions of Asciidoctor now render the headers and
  footers like AsciiDoc does, except for the date formatting.

[1] https://public-inbox.org/git/20190913050634.GB21172@sigill.intra.peff.net/

[2] The tests with 2.0.10 were done on top of brian's recent patch to
build at all with Asciidoctor 2:
https://public-inbox.org/git/20190915224332.103930-1-sandals@crustytoothpaste.net/

Martin Ågren (3):
  Doc/Makefile: give mansource/-version/-manual attributes
  asciidoctor-extensions: provide `<refmiscinfo/>`
  doc-diff: replace --cut-header-footer with --cut-footer

 Documentation/Makefile                  |  3 ++-
 Documentation/asciidoc.conf             |  6 +++---
 Documentation/asciidoctor-extensions.rb | 17 +++++++++++++++++
 Documentation/doc-diff                  | 17 ++++++++---------
 4 files changed, 30 insertions(+), 13 deletions(-)

-- 
2.23.0

[PATCH v3 1/3] Doc/Makefile: give mansource/-version/-manual attributes

[Martin Ågren]

Rather than hardcoding "Git Manual" and "Git" as the manual and source
in asciidoc.conf, provide them as attributes `manmanual` and
`mansource`. Rename the `git_version` attribute to `manversion`.

These new attribute names are not arbitrary, see, e.g., [1].

For AsciiDoc (8.6.10) and Asciidoctor <1.5.7, this is a no-op. Starting
with Asciidoctor 1.5.7, `manmanual` and `mansource` actually end up in
the xml-files and eventually in the rendered manpages. In particular,
the manpage headers now render just as with AsciiDoc.

No versions of Asciidoctor pick up the `manversion` [2], and older
versions don't pick up any of these attributes. -- We'll fix that with a
bit of a hack in the next commit.

[1] https://asciidoctor.org/docs/user-manual/#man-pages

[2] Note how [1] says "Not used by Asciidoctor".

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
---
 Documentation/Makefile      | 3 ++-
 Documentation/asciidoc.conf | 6 +++---
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/Documentation/Makefile b/Documentation/Makefile
index dbf5a0f276..a67bc04ae8 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -122,7 +122,8 @@ ASCIIDOC_HTML = xhtml11
 ASCIIDOC_DOCBOOK = docbook
 ASCIIDOC_CONF = -f asciidoc.conf
 ASCIIDOC_COMMON = $(ASCIIDOC) $(ASCIIDOC_EXTRA) $(ASCIIDOC_CONF) \
-		-agit_version=$(GIT_VERSION)
+		-amanversion=$(GIT_VERSION) \
+		-amanmanual='Git Manual' -amansource='Git'
 TXT_TO_HTML = $(ASCIIDOC_COMMON) -b $(ASCIIDOC_HTML)
 TXT_TO_XML = $(ASCIIDOC_COMMON) -b $(ASCIIDOC_DOCBOOK)
 MANPAGE_XSL = manpage-normal.xsl
diff --git a/Documentation/asciidoc.conf b/Documentation/asciidoc.conf
index 2c16c536ba..8fc4b67081 100644
--- a/Documentation/asciidoc.conf
+++ b/Documentation/asciidoc.conf
@@ -78,9 +78,9 @@ template::[header-declarations]
 <refmeta>
 <refentrytitle>{mantitle}</refentrytitle>
 <manvolnum>{manvolnum}</manvolnum>
-<refmiscinfo class="source">Git</refmiscinfo>
-<refmiscinfo class="version">{git_version}</refmiscinfo>
-<refmiscinfo class="manual">Git Manual</refmiscinfo>
+<refmiscinfo class="source">{mansource}</refmiscinfo>
+<refmiscinfo class="version">{manversion}</refmiscinfo>
+<refmiscinfo class="manual">{manmanual}</refmiscinfo>
 </refmeta>
 <refnamediv>
   <refname>{manname}</refname>
-- 
2.23.0

[PATCH v3 2/3] asciidoctor-extensions: provide `<refmiscinfo/>`

[Martin Ågren]

As can be seen from the previous commit, there are three attributes that
we provide to AsciiDoc through asciidoc.conf. Asciidoctor ignores that
file. After that patch, newer versions of Asciidoctor pick up the
`manmanual` and `mansource` attributes as we invoke `asciidoctor`, but
they don't pick up `manversion`. ([1] says: "Not used by Asciidoctor.")
Older versions (<1.5.7) don't handle these attributes at all. As a
result, we are missing one or three `<refmiscinfo/>` tags in each
xml-file produced when we build with Asciidoctor.

Because of this, xmlto doesn't include the Git version number in the
rendered manpages. And in particular, with versions <1.5.7, the manpage
footers instead contain the fairly ugly "[FIXME: source]".

That Asciidoctor ignores asciidoc.conf is nothing new. This is why we
implement the `linkgit:` macro in asciidoc.conf *and* in
asciidoctor-extensions.rb. Follow suit and provide these tags in
asciidoctor-extensions.rb, using a "postprocessor" extension where we
just search and replace in the XML, treated as text.

We may consider a few alternatives:

  * Inject these lines into the xml-files from the *Makefile*, e.g.,
    using `sed`. That would reduce repetition, but it feels wrong to
    impose another step and another risk on the AsciiDoc-processing only
    to benefit the Asciidoctor-one.

  * I tried providing a "docinfo processor" to inject these tags, but
    could not figure out how to "merge" the two <refmeta/> sections that
    resulted. To avoid xmlto barfing on the result, I needed to use
    `xmlto --skip-validation ...`, which seems unfortunate.

Let's instead inject the missing tags using a postprocessor. We'll make
it fairly obvious that we aim to inject the exact same three lines of
`<refmiscinfo/>` that asciidoc.conf provides. We inject them in
*post*-processing so we need to do the variable expansion ourselves. We
do introduce the bug that asciidoc.conf already has in that we won't do
any escaping, e.g., of funky versions like "some v <2.25, >2.20".

The postprocessor we add here works on the XML as raw text and doesn't
really use the full potential of XML to do a more structured injection.
This is actually precisely what the Asciidoctor User Manual does in its
postprocessor example [2]. I looked into two other approaches:

  1. The nokogiri library is apparently the "modern" way of doing XML
     in ruby. I got it working fairly easily:
        require 'nokogiri'
        doc = Nokogiri::XML(output)
        doc.search("refmeta").each { |n| n.add_child(new_tags) }
        output = doc.to_xml
     However, this adds another dependency (e.g., the "ruby-nokogiri"
     package on Ubuntu). Using Asciidoctor is not our default, but it
     will probably need to become so soon. Let's avoid adding a
     dependency just so that we can say "search...add_child" rather than
     "sub(regex...)".

  2. The older REXML is apparently always(?) bundled with ruby, but I
     couldn't even parse the original document:
        require 'rexml/document'
        doc = REXML::Document.new(output)
        ...
     The error was "no implicit conversion of nil into String" and I
     stopped there.

I don't think it's unlikely that doing a plain old search-and-replace
will work just as fine or better compared to parsing XML and worrying
about libraries and library versions.

[1] https://asciidoctor.org/docs/user-manual/#builtin-attributes

[2] https://asciidoctor.org/docs/user-manual/#postprocessor-example

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
---
 Documentation/asciidoctor-extensions.rb | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/Documentation/asciidoctor-extensions.rb b/Documentation/asciidoctor-extensions.rb
index 0089e0cfb8..85f14c7c11 100644
--- a/Documentation/asciidoctor-extensions.rb
+++ b/Documentation/asciidoctor-extensions.rb
@@ -20,9 +20,26 @@ module Git
         end
       end
     end
+
+    class DocumentPostProcessor < Asciidoctor::Extensions::Postprocessor
+      def process document, output
+        if document.basebackend? 'docbook'
+          mansource = document.attributes['mansource']
+          manversion = document.attributes['manversion']
+          manmanual = document.attributes['manmanual']
+          new_tags = "" \
+            "<refmiscinfo class=\"source\">#{mansource}</refmiscinfo>\n" \
+            "<refmiscinfo class=\"version\">#{manversion}</refmiscinfo>\n" \
+            "<refmiscinfo class=\"manual\">#{manmanual}</refmiscinfo>\n"
+          output = output.sub(/<\/refmeta>/, new_tags + "</refmeta>")
+        end
+        output
+      end
+    end
   end
 end
 
 Asciidoctor::Extensions.register do
   inline_macro Git::Documentation::LinkGitProcessor, :linkgit
+  postprocessor Git::Documentation::DocumentPostProcessor
 end
-- 
2.23.0

[PATCH v3 3/3] doc-diff: replace --cut-header-footer with --cut-footer

[Martin Ågren]

After the previous commit, AsciiDoc and Asciidoctor render the manpage
headers identically, so we no longer need the "cut the header" part of
our `--cut-header-footer` option. We do still need the "cut the footer"
part, though. The previous commits improved the rendering of the footer
in Asciidoctor by quite a bit, but the two programs still disagree on
how to format the date in the footer: 01/01/1970 vs 1970-01-01.

We could keep using `--cut-header-footer`, but it would be nice if we
had a slightly smaller hammer `--cut-footer` that would be less likely
to hide regressions. Rather than simply adding such an option, let's
also drop `--cut-header-footer`, i.e., rework it to lose the "header"
part of its name and functionality.

`--cut-header-footer` is just a developer tool and it probably has no
more than a handful of users, so we can afford to be aggressive.

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
---
 Documentation/doc-diff | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/Documentation/doc-diff b/Documentation/doc-diff
index 3355be4798..88a9b20168 100755
--- a/Documentation/doc-diff
+++ b/Documentation/doc-diff
@@ -21,7 +21,7 @@ asciidoc		use asciidoc with both commits
 to-asciidoc		use asciidoc with the 'to'-commit
 to-asciidoctor		use asciidoctor with the 'to'-commit
 asciidoctor		use asciidoctor with both commits
-cut-header-footer	cut away header and footer
+cut-footer		cut away footer
 "
 SUBDIRECTORY_OK=1
 . "$(git --exec-path)/git-sh-setup"
@@ -31,7 +31,7 @@ force=
 clean=
 from_program=
 to_program=
-cut_header_footer=
+cut_footer=
 while test $# -gt 0
 do
 	case "$1" in
@@ -55,8 +55,8 @@ do
 	--asciidoc)
 		from_program=-asciidoc
 		to_program=-asciidoc ;;
-	--cut-header-footer)
-		cut_header_footer=-cut-header-footer ;;
+	--cut-footer)
+		cut_footer=-cut-footer ;;
 	--)
 		shift; break ;;
 	*)
@@ -118,8 +118,8 @@ construct_makemanflags () {
 from_makemanflags=$(construct_makemanflags "$from_program") &&
 to_makemanflags=$(construct_makemanflags "$to_program") &&
 
-from_dir=$from_oid$from_program$cut_header_footer &&
-to_dir=$to_oid$to_program$cut_header_footer &&
+from_dir=$from_oid$from_program$cut_footer &&
+to_dir=$to_oid$to_program$cut_footer &&
 
 # generate_render_makefile <srcdir> <dstdir>
 generate_render_makefile () {
@@ -169,12 +169,11 @@ render_tree () {
 		make -j$parallel -f - &&
 		mv "$tmp/rendered/$dname+" "$tmp/rendered/$dname"
 
-		if test "$cut_header_footer" = "-cut-header-footer"
+		if test "$cut_footer" = "-cut-footer"
 		then
 			for f in $(find "$tmp/rendered/$dname" -type f)
 			do
-				tail -n +3 "$f" | head -n -2 |
-				sed -e '1{/^$/d}' -e '${/^$/d}' >"$f+" &&
+				head -n -2 "$f" | sed -e '${/^$/d}' >"$f+" &&
 				mv "$f+" "$f" ||
 				return 1
 			done
-- 
2.23.0

Re: [PATCH] asciidoctor-extensions: provide `<refmiscinfo/>`

[Jeff King]

On Mon, Mar 18, 2019 at 10:46:45PM -0400, Jeff King wrote:

> >   * Provide the `mansource` attribute to Asciidoctor. This attribute
> >     looks promising until one realizes that it can only be given inside
> >     the source file (the .txt file in our case), *not* on the command
> >     line using `-a mansource=foobar`. I toyed with the idea of injecting
> >     this attribute while feeding Asciidoctor the input on stdin, but it
> >     didn't feel like it was worth the complexity in the Makefile.
> 
> It does seem like "mansource" is the way asciidoctor expects us to do
> this. Why doesn't it work from the command line? Is it a bug in
> asciidoctor, or is there something more subtle going on?
> 
> I think even if it is a bug and gets fixed, though, it still wouldn't
> have the version field (though that seems like something we could
> contribute to asciidoctor).

I just tried with asciidoc 2.0.0.rc.2, which came out last week. It does
seem to work from the command line:

  $ make USE_ASCIIDOCTOR=Yes \
         ASCIIDOC_DOCBOOK=docbook5 \
         ASCIIDOC='asciidoctor -amansource=Git -amanmanual="Git Manual"' \
         git-add.xml
  $ sed -n '/refmeta/,/refmeta/p' git-add.xml
  <refmeta>
  <refentrytitle>git-add</refentrytitle>
  <manvolnum>1</manvolnum>
  <refmiscinfo class="source">Git</refmiscinfo>
  <refmiscinfo class="manual">Git Manual</refmiscinfo>
  </refmeta>

Still no "manversion" attribute, though.

-Peff

Re: [PATCH] asciidoctor-extensions: provide `<refmiscinfo/>`

[Martin Ågren]

On Tue, 19 Mar 2019 at 04:30, Jeff King <peff@peff.net> wrote:
>
> On Mon, Mar 18, 2019 at 10:46:45PM -0400, Jeff King wrote:
>
> > It does seem like "mansource" is the way asciidoctor expects us to do
> > this. Why doesn't it work from the command line? Is it a bug in
> > asciidoctor, or is there something more subtle going on?
> >
> > I think even if it is a bug and gets fixed, though, it still wouldn't
> > have the version field (though that seems like something we could
> > contribute to asciidoctor).
>
> I just tried with asciidoc 2.0.0.rc.2, which came out last week. It does
> seem to work from the command line:
>
>   $ make USE_ASCIIDOCTOR=Yes \
>          ASCIIDOC_DOCBOOK=docbook5 \
>          ASCIIDOC='asciidoctor -amansource=Git -amanmanual="Git Manual"' \
>          git-add.xml
>   $ sed -n '/refmeta/,/refmeta/p' git-add.xml
>   <refmeta>
>   <refentrytitle>git-add</refentrytitle>
>   <manvolnum>1</manvolnum>
>   <refmiscinfo class="source">Git</refmiscinfo>
>   <refmiscinfo class="manual">Git Manual</refmiscinfo>
>   </refmeta>

No such luck with asciidoctor 1.5.5. Seems like it really wants
"manpage" before it considers these attributes.

(That's still me holding the tool, so factor that into it.)

Martin

Re: [PATCH] asciidoctor-extensions: provide `<refmiscinfo/>`

[Jeff King]

On Tue, Mar 19, 2019 at 08:12:54AM +0100, Martin Ågren wrote:

> > I just tried with asciidoc 2.0.0.rc.2, which came out last week. It does
> > seem to work from the command line:
> >
> >   $ make USE_ASCIIDOCTOR=Yes \
> >          ASCIIDOC_DOCBOOK=docbook5 \
> >          ASCIIDOC='asciidoctor -amansource=Git -amanmanual="Git Manual"' \
> >          git-add.xml
> >   $ sed -n '/refmeta/,/refmeta/p' git-add.xml
> >   <refmeta>
> >   <refentrytitle>git-add</refentrytitle>
> >   <manvolnum>1</manvolnum>
> >   <refmiscinfo class="source">Git</refmiscinfo>
> >   <refmiscinfo class="manual">Git Manual</refmiscinfo>
> >   </refmeta>
> 
> No such luck with asciidoctor 1.5.5. Seems like it really wants
> "manpage" before it considers these attributes.
> 
> (That's still me holding the tool, so factor that into it.)

The refmiscinfo stuff didn't arrive until asciidoctor v1.5.7.

-Peff

Re: [PATCH] asciidoctor-extensions: provide `<refmiscinfo/>`

[Todd Zullinger]

Jeff King wrote:
> On Tue, Mar 19, 2019 at 08:12:54AM +0100, Martin Ågren wrote:
> 
>>> I just tried with asciidoc 2.0.0.rc.2, which came out last week. It does
>>> seem to work from the command line:
>>>
>>>   $ make USE_ASCIIDOCTOR=Yes \
>>>          ASCIIDOC_DOCBOOK=docbook5 \
>>>          ASCIIDOC='asciidoctor -amansource=Git -amanmanual="Git Manual"' \
>>>          git-add.xml
>>>   $ sed -n '/refmeta/,/refmeta/p' git-add.xml
>>>   <refmeta>
>>>   <refentrytitle>git-add</refentrytitle>
>>>   <manvolnum>1</manvolnum>
>>>   <refmiscinfo class="source">Git</refmiscinfo>
>>>   <refmiscinfo class="manual">Git Manual</refmiscinfo>
>>>   </refmeta>
>> 
>> No such luck with asciidoctor 1.5.5. Seems like it really wants
>> "manpage" before it considers these attributes.
>> 
>> (That's still me holding the tool, so factor that into it.)
> 
> The refmiscinfo stuff didn't arrive until asciidoctor v1.5.7.

Now I don't feel as bad that I didn't find any good way to
handle refmiscinfo when I first tried to use asciidoctor in
November 2017 at least.

This made me look closer at the fedora asciidoctor packages.
They have been stuck on 1.5.6.1.  So all my recent testing
has been with 1.5.6.1.

I spent some time yesterday working toward getting the
fedora packages updated to 1.5.8.  With luck that will reach
the stable updates channels before too long.

(I'm guessing the upcoming 2.0 release won't be suitable as
an update for released fedora versions, being a major
release with potentially backwards-incompatible changes.)

There are differences in the output from 1.5.6.1 and 1.5.8,
but I haven't looked closely yet.

-- 
Todd

Re: [PATCH] asciidoctor-extensions: provide `<refmiscinfo/>`

[Martin Ågren]

On Tue, 19 Mar 2019 at 03:46, Jeff King <peff@peff.net> wrote:
>
> On Sun, Mar 17, 2019 at 03:47:47PM +0100, Martin Ågren wrote:
>
> >  Cc Todd and Peff who had a brief exchange [1] a while ago. Apparently
> >  Todd saw this "[FIXME: source]" on Fedora but Peff did not on Debian.
> >  I'm on Ubuntu 18.04 and used to see this also on 16.04. I'm not sure
> >  what might make Debian so special here.
>
> I think it was just that my version of asciidoctor had
>
>   https://github.com/asciidoctor/asciidoctor/pull/2636
>
> and Todd's did not. However, mine still does not do the _right_ thing,
> because we didn't pass the right attributes in to asciidoctor. It just
> didn't print an ugly "FIXME". Looking at the XML, I have:
>
>   <refentrytitle>git-add</refentrytitle>
>   <manvolnum>1</manvolnum>
>   <refmiscinfo class="source">&#160;</refmiscinfo>
>   <refmiscinfo class="manual">&#160;</refmiscinfo>
>   </refmeta>
>
> So it's just an nbsp instead of the real content, and the "version"
> field is missing entirely.

Huh, yeah, that's a big improvement already.

> > That Asciidoctor ignores asciidoc.conf is nothing new. This is why we
> > implement the `linkgit:` macro in asciidoc.conf *and* in
> > asciidoctor-extensions.rb. Follow suit and provide these tags in
> > asciidoctor-extensions.rb, using a "postprocessor" extension.
>
> Yeah, that seems sensible overall. Some thoughts on your approach:
>
> >   * Provide the `mansource` attribute to Asciidoctor. This attribute
> >     looks promising until one realizes that it can only be given inside
> >     the source file (the .txt file in our case), *not* on the command
> >     line using `-a mansource=foobar`. I toyed with the idea of injecting
> >     this attribute while feeding Asciidoctor the input on stdin, but it
> >     didn't feel like it was worth the complexity in the Makefile.
>
> It does seem like "mansource" is the way asciidoctor expects us to do
> this. Why doesn't it work from the command line? Is it a bug in
> asciidoctor, or is there something more subtle going on?
>
> I think even if it is a bug and gets fixed, though, it still wouldn't
> have the version field (though that seems like something we could
> contribute to asciidoctor).

The bug is in my docs-reading, see below.

> >   * Considering the above abandoned ideas, it seems better to put any
> >     complexity inside asciidoctor-extensions.rb. It is after all
> >     supposed to be the "equivalent" of asciidoc.conf. I considered
> >     providing a "tree processor" extension and use it to set, e.g.,
> >     `mansource` mentioned above.
>
> This seems like the least bad option, at least for now. Your code does
> do a generic regex substitution. The promise of XML is that we're
> supposed to be able to do structured, robust transformations of the
> document. But my experience has been that the tooling is sufficiently
> difficult to work with that you just end up writing a regex.
>
> So I'm curious if you tried to use an actual XML parser (or god forbid,
> XSLT) to do the transformation. But if you spent more than 5 minutes on
> it and got disgusted, I wouldn't ask you to look deeper than that. :)

Well, I didn't spend 5 minutes on it, but my experience told me
something like that would happen. ;-)

Now I realize that I'm wrong in my "it doesn't work from the command
line". Somehow, I read the following in the user manual: "Many
attributes can only be defined in the document header (or via the API or
CLI)." And *repeatedly* read is as "(not via the API or CLI)", somehow
always expecting a "not" to follow an "only". Oh well.

But of course, I did try it out before reaching for the docs, like
anyone would. The true reason it doesn't work for me is probably this
header from the listing that contains "mansource": "Manpage attributes
(relevant only when using the manpage doctype and/or converter)".

> I doubt we'd see any other refmeta tags (and any non-tag content would
> be quoted).
>
> > Let's instead try to stay as close as possible to what asciidoc.conf
> > does. We'll make it fairly obvious that we aim to inject the exact same
> > three lines of `<refmiscinfo/>` that asciidoc.conf provides. The only
> > somewhat tricky part is that we inject them *post*-processing so we need
> > to do the variable expansion ourselves.
>
> One thing that asciidoctor buys us that asciidoc does not is that we
> might eventually move to directly generating the manpages, without the
> XML / Docbook step in between. And if we do, then all of this XML
> hackery is going to have to get replaced with something else. I guess we
> can cross that bridge when we come to it.

Todd has made a promising start in another part of this thread. There
seems to be a few wrinkles that need some care, but hopefully nothing
impossible (famous last words).

> The patch itself looks sane. Would we ever need to XML-quote the
> contents of git_version? I guess the asciidoc.conf version doesn't
> bother.

Good point. Hadn't thought of it. You're right that the asciidoc.conf
version has the same problem and a version string like "<>" goes
unescaped into the xml.

> Technically the user running "make" could put whatever they want
> into it, but I think this is a case of "if it hurts, don't do it", and
> we can ignore it.

:-)

Martin