| Message ID | 20190329123520.27549-1-szeder.dev@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Asciidoctor-related formatting and CI fixes | expand |
Hi, On Fri, 29 Mar 2019, SZEDER Gábor wrote: > [...] > - Mention in patch 4's commit message that Asciidoctor is still only > installed in the Documentation build job. > [...] > > Range-diff: > 1: 8026f62876 = 1: 8026f62876 Documentation/git-diff-tree.txt: fix formatting > 2: fd19cf4b24 = 2: fd19cf4b24 Documentation/technical/api-config.txt: fix formatting > 3: 638dcd64e9 = 3: 638dcd64e9 Documentation/technical/protocol-v2.txt: fix formatting > 4: 6f8c6ff398 ! 4: 2e94e2b7b6 ci: install Asciidoctor in 'ci/install-dependencies.sh' > @@ -6,8 +6,9 @@ > installation of the 'asciidoctor' gem somehow ended up in > 'ci/test-documentation.sh'. > > - Install it in 'ci/install-dependencies.sh', where we install > - everything else. > + Install it in 'ci/install-dependencies.sh', where we install other > + dependencies of the Documentation build job as well (asciidoc, > + xmlto). I would have wished for something that more explicitly said that it still *only* installs it for the Documentation job. But I can live with the current wording. > [1] 657343a602 (travis-ci: move Travis CI code into dedicated scripts, > 2017-09-10) > [...] > > @@ -66,15 +77,32 @@ > +++ b/ci/test-documentation.sh > @@ > > + . ${0%/*}/lib.sh > + > ++filter_log () { > ++ sed -e '/^GIT_VERSION = /d' \ > ++ -e '/^ \* new asciidoc flags$/d' \ > ++ "$1" > ++} > ++ > + make check-builtins > + make check-docs > + > # Build docs with AsciiDoc > - make doc > >(tee stdout.log) 2> >(tee stderr.log >&2) > +-make doc > >(tee stdout.log) 2> >(tee stderr.log >&2) > -! test -s stderr.log > -+cat stderr.log > ++make doc > >(tee stdout.log) 2> >(tee stderr.raw >&2) > ++cat stderr.raw > ++filter_log stderr.raw >stderr.log > +test ! -s stderr.log > test -s Documentation/git.html > test -s Documentation/git.xml > test -s Documentation/git.1 > -@@ > + grep '<meta name="generator" content="AsciiDoc ' Documentation/git.html > + > +-rm -f stdout.log stderr.log > ++rm -f stdout.log stderr.log stderr.raw > + check_unignored_build_artifacts > > # Build docs with AsciiDoctor > make clean > @@ -82,8 +110,8 @@ > -sed '/^GIT_VERSION = / d' stderr.log > -! test -s stderr.log > +make USE_ASCIIDOCTOR=1 doc > >(tee stdout.log) 2> >(tee stderr.raw >&2) > -+sed '/^GIT_VERSION = / d' stderr.raw >stderr.log > -+cat stderr.log > ++cat stderr.raw > ++filter_log stderr.raw >stderr.log > +test ! -s stderr.log > test -s Documentation/git.html > grep '<meta name="generator" content="Asciidoctor ' Documentation/git.html Wow. Subtle. And a bit hard to read without color ;-) But from what I understand, this does the right thing. So from my side, this patch series is good to go! Thanks, Dscho
The first three patches fix formatting issues with Asciidoctor, while the last three patches are a small cleanup and fixes to the documentation CI build jobs; notably patch 5 un-breaks the documentation build job on Travis CI in the era of Asciidoctor v2.0.0 [1], and patch 6 fixes some forever-broken checks. Changes since v2: - The fixed stderr check in patch 6 is triggered when merged with 'pu', in particular with commit 9a71722b4d (Doc: auto-detect changed build flags, 2019-03-17) in 'ma/doc-diff-doc-vs-doctor-comparison'. The changes to patch 6 deal with that. - Mention in patch 4's commit message that Asciidoctor is still only installed in the Documentation build job. [1] https://public-inbox.org/git/20190324162131.GL4047@pobox.com/ SZEDER Gábor (6): Documentation/git-diff-tree.txt: fix formatting Documentation/technical/api-config.txt: fix formatting Documentation/technical/protocol-v2.txt: fix formatting ci: install Asciidoctor in 'ci/install-dependencies.sh' ci: stick with Asciidoctor v1.5.8 for now ci: fix AsciiDoc/Asciidoctor stderr check in the documentation build job Documentation/git-diff-tree.txt | 1 + Documentation/technical/api-config.txt | 2 +- Documentation/technical/protocol-v2.txt | 52 ++++++++++++------------- ci/install-dependencies.sh | 3 ++ ci/test-documentation.sh | 24 +++++++----- 5 files changed, 46 insertions(+), 36 deletions(-) Range-diff: 1: 8026f62876 = 1: 8026f62876 Documentation/git-diff-tree.txt: fix formatting 2: fd19cf4b24 = 2: fd19cf4b24 Documentation/technical/api-config.txt: fix formatting 3: 638dcd64e9 = 3: 638dcd64e9 Documentation/technical/protocol-v2.txt: fix formatting 4: 6f8c6ff398 ! 4: 2e94e2b7b6 ci: install Asciidoctor in 'ci/install-dependencies.sh' @@ -6,8 +6,9 @@ installation of the 'asciidoctor' gem somehow ended up in 'ci/test-documentation.sh'. - Install it in 'ci/install-dependencies.sh', where we install - everything else. + Install it in 'ci/install-dependencies.sh', where we install other + dependencies of the Documentation build job as well (asciidoc, + xmlto). [1] 657343a602 (travis-ci: move Travis CI code into dedicated scripts, 2017-09-10) 5: b9eb371f81 = 5: 95476591dd ci: stick with Asciidoctor v1.5.8 for now 6: ba2a005a05 ! 6: 04ee6831fd ci: fix AsciiDoc/Asciidoctor stderr check in the documentation build job @@ -2,51 +2,62 @@ ci: fix AsciiDoc/Asciidoctor stderr check in the documentation build job - In our 'ci/*' build scripts we rely on 'set -e' aborting the build job - when a command exits with error, while in 'ci/test-documentation.sh' - we tried to check the emptiness of AsciiDoc's and Asciidoctor's - standard error with '! test -s stderr.log'. Unfortunately, the - combination of the two doesn't work as intended, because, according to - POSIX [1]: + In 'ci/test-documentation.sh' we save the standard error of 'make + doc', and, in an attempt to make sure that neither AsciiDoc nor + Asciidoctor printed any warnings, we check the emptiness of the + resulting file with '! test -s stderr.log'. This check has never + actually worked, because in our 'ci/*' build scripts we rely on 'set + -e' aborting the build job when a command exits with error, and, + unfortunately, the combination of the two doesn't work as intended. + According to POSIX [1]: "The -e setting shall be ignored when executing [...] a pipeline beginning with the ! reserved word" [2] Watch and learn: - $ cat e.sh - #!/bin/sh - set -ex - echo unexpected >file - ! test -s file - test ! -s file - echo "should not reach this" - $ ./e.sh - + echo unexpected - + test -s file - + test ! -s file - $ + $ echo unexpected >file + $ ( set -e; ! test -s file ; echo "should not reach this" ) ; echo $? + should not reach this + 0 This is why we haven't noticed the warnings from Asciidoctor that were fixed in the first patches of this patch series, though some of them were already there in the build of v2.18.0-rc0 [3]. Check the emptiness of that file with 'test ! -s' instead, which works - properly with 'set -e'. Furthermore, dump the contents of that file - to the log for our convenience, so if it were to unexpectedly end up - being non-empty, then we wouldn't have to scroll through all that long - build log looking for warnings, but could see them right away near the - end of the log. - - And one more thing: we build the docs with Asciidoctor right after a - 'make clean', meaning that 'make doc' starts with running - 'GIT-VERSION-GEN', which in turn prints the version to its standard - error. This version then goes to 'stderr.log' as well, and a 'sed' - command is supposed to remove it to prevent it from interfering with - that (previously defunct) emptiness check. Alas, this command doesn't - work as intended, either, because it leaves the file to be checked - intact, but the defunct emptiness check hid this issue, too... This - patch deals with this as well. + properly with 'set -e': + + $ ( set -e; test ! -s file ; echo "should not reach this" ) ; echo $? + 1 + + Furthermore, dump the contents of that file to the log for our + convenience, so if it were to unexpectedly end up being non-empty, + then we wouldn't have to scroll through all that long build log + looking for warnings, but could see them right away near the end of + the log. + + Note that we are only really interested in the standard error of + AsciiDoc and Asciidoctor, but by saving the stderr of 'make doc' we + also save any error output from the make rules. Currently there is + only one such line: we build the docs with Asciidoctor right after a + 'make clean', meaning that 'make USE_ASCIIDOCTOR=1 doc' always starts + with running 'GIT-VERSION-GEN', which in turn prints the version to + stderr. A 'sed' command was supposed to remove this version line to + prevent it from triggering that (previously defunct) emptiness check, + but, unfortunately, this command doesn't work as intended, either, + because it leaves the file to be checked intact, but that defunct + emptiness check hid this issue, too... Furthermore, in the near + future there will be an other line on stderr, because commit + 9a71722b4d (Doc: auto-detect changed build flags, 2019-03-17) in the + currently cooking branch 'ma/doc-diff-doc-vs-doctor-comparison' will + print "* new asciidoc flags" at the beginning of both 'make doc' + invokations. + + Extend that 'sed' command to remove this line, too, wrap it in a + helper function so the output of both 'make doc' is filtered the same + way, and change its invokation to actually write the logfile to be + checked. [1] http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#set @@ -66,15 +77,32 @@ +++ b/ci/test-documentation.sh @@ + . ${0%/*}/lib.sh + ++filter_log () { ++ sed -e '/^GIT_VERSION = /d' \ ++ -e '/^ \* new asciidoc flags$/d' \ ++ "$1" ++} ++ + make check-builtins + make check-docs + # Build docs with AsciiDoc - make doc > >(tee stdout.log) 2> >(tee stderr.log >&2) +-make doc > >(tee stdout.log) 2> >(tee stderr.log >&2) -! test -s stderr.log -+cat stderr.log ++make doc > >(tee stdout.log) 2> >(tee stderr.raw >&2) ++cat stderr.raw ++filter_log stderr.raw >stderr.log +test ! -s stderr.log test -s Documentation/git.html test -s Documentation/git.xml test -s Documentation/git.1 -@@ + grep '<meta name="generator" content="AsciiDoc ' Documentation/git.html + +-rm -f stdout.log stderr.log ++rm -f stdout.log stderr.log stderr.raw + check_unignored_build_artifacts # Build docs with AsciiDoctor make clean @@ -82,8 +110,8 @@ -sed '/^GIT_VERSION = / d' stderr.log -! test -s stderr.log +make USE_ASCIIDOCTOR=1 doc > >(tee stdout.log) 2> >(tee stderr.raw >&2) -+sed '/^GIT_VERSION = / d' stderr.raw >stderr.log -+cat stderr.log ++cat stderr.raw ++filter_log stderr.raw >stderr.log +test ! -s stderr.log test -s Documentation/git.html grep '<meta name="generator" content="Asciidoctor ' Documentation/git.html