Skip to content

Doc sanity checks#419

Merged
jimklimov merged 18 commits intonetworkupstools:masterfrom
jimklimov:doc-sanity
Oct 25, 2020
Merged

Doc sanity checks#419
jimklimov merged 18 commits intonetworkupstools:masterfrom
jimklimov:doc-sanity

Conversation

@jimklimov
Copy link
Copy Markdown
Member

@jimklimov jimklimov commented Apr 9, 2017

Inspired by discussion at #418, this PR adds recipes to sanity-check generated (or "tarball-released") documentation (HTML-single, HTML-chunked, PDF, MAN). This just tests the corresponding files are not empty and their file (libmagic) signature in C locale matches expectations.

Roughly tested to do the work and not explode in builds with enabled, skipped and disabled doc scenarios; hope Travis would check this all better ;)

Tested to do report an empty PDF file as an error (cat /dev/null > FAQ.pdf && make check).

Also fixes a couple of older issues.

Comment thread docs/man/Makefile.am Outdated
check-man-txt: $(SRC_ALL_PAGES)
@FAILED=""; LANG=C; LC_ALL=C; export LANG; export LC_ALL; \
for F in $(SRC_ALL_PAGES) ; do \
test -s "$$F" && { file "$$F" | egrep '(ASCII|UTF-8|Unicode).* text' > /dev/null ; } || FAILED="$$FAILED $$F" ; \
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From Buildbot on OpenBSD:

make  check-local
FAILED man-source sanity check for: nutdrv_qx.txt rhino.txt solis.txt upscode2.txt nutdrv_qx.txt
nutdrv_qx.txt: ISO-8859 English text
rhino.txt:     ISO-8859 English text
solis.txt:     ISO-8859 English text
upscode2.txt:  ISO-8859 English text
nutdrv_qx.txt: ISO-8859 English text

@clepple
Copy link
Copy Markdown
Member

clepple commented Apr 10, 2017

Other than the ISO-8859 thing on OpenBSD, looks good.

@clepple
Copy link
Copy Markdown
Member

clepple commented Apr 10, 2017

Meh, the comment shows up as outdated, but I can't figure out how to comment on that line in the latest changeset. Still applicable as of commit 74f19fc.

Some systems detect text sources as "ISO-8859 English" text - please them.
@jimklimov
Copy link
Copy Markdown
Member Author

Thanks @clepple, posted a fix hopefully :)

Are buildbots launched manually nowadays agains select branches/PRs/commits, or are they integrated in general PR testing and reporting?

@clepple
Copy link
Copy Markdown
Member

clepple commented Apr 10, 2017

Are buildbots launched manually nowadays agains select branches/PRs/commits, or are they integrated in general PR testing and reporting?

Builds are started a minute or so after a branch is pushed to the main NUT repository (the delay allows the scheduler to combine builds for commits on the same branch). This is why we have a lot of branches that shadow the pull requests. I just updated pull_419_doc_sanity, but feel free to push fast-forward updates. (I would prefer a new branch instead of a force push, but that's only a big deal if I have to dive into the logs.)

It would be nice to be able to poll all of the NUT developers' forks, but at one point, corporate firewall policies were forcing us to rewrite the pull URL scheme from git to HTTPS. The naive rewrite rule prevents a lot of features from working properly. Now that none of the Eaton buildslaves are active, it is probably worth revisiting this.

@clepple
Copy link
Copy Markdown
Member

clepple commented Apr 10, 2017

Also failing on check-man-pages: OpenBSD stdio@df0a98f1ca

Whitespace mess
@jimklimov
Copy link
Copy Markdown
Member Author

@clepple : I have no idea how to coerce buildbots into building a PR :\ Can this only happen by making and bumping a real branch in NUT upstream git? Can that existing buildbot's config be extended with support for PRs like at https://github.com/alalek/buildbot-pullrequest-sample/blob/master/config/pr_github.py#L105 or was there some reason against that (e.g. can't spare resources for all PRs coming and going, so just processing updates of code responsibly merged into main repo)?

@clepple
Copy link
Copy Markdown
Member

clepple commented Oct 14, 2020

@jimklimov yes, it only builds branches pushed to the main networkupstools/nut repository (not just master, though). Previously I was creating temporary branches for PRs after a quick sanity check. I'd rather not open that system up to any random passer-by, especially given how much further along the Travis build infrastructure is.

@jimklimov
Copy link
Copy Markdown
Member Author

jimklimov commented Oct 14, 2020

Ok, I'll add this branch to upstream repo then, to trigger a buildbot build and see how it goes in the multiverse.

@jimklimov
Copy link
Copy Markdown
Member Author

jimklimov commented Oct 15, 2020

@clepple : so, as far as I see all buildbot builds that ran this branch passed the "configure" phase green. But it seems only a couple of bots ran the pull_419 branch.

However while the FreeBSD-x64 build at http://buildbot.networkupstools.org/public/nut/builders/FreeBSD-x64/builds/634/steps/compile/logs/stdio succeeded to find tools needed for man pages and build/check passed green, the FreeBSD-x86 one failed because it lacked both the tools to make manpages and the pre-built pages too: http://buildbot.networkupstools.org/public/nut/builders/FreeBSD-x86/builds/921/steps/compile/logs/stdio

I am frankly not sure what make distcheck(-light) should do in such situation; assuming that it should (check an ability to) provide a dist archive and due to tools and inputs available could not, failing could the right thing to do. I am not certain if prebuilt man-pages, like prebuilt configure script and Makefiles, must be part of such archive.

The way it happened to do so might not be that nice, however (exposed a typo, will fix in a sec):

PASSED man-source sanity check
PASSED man-page sanity check
echo "Man-page generation was not done, so pregenerated pages were sanity-checked (if any)" >&@
>>> /bin/sh: Syntax error: Bad fd number <<<
make[5]: *** [Makefile:1400: check-local] Error 2

Then I guess it is good to merge, at least syntax-wise?..

Slightly off-topic, many of recent days' runs (for master mostly) exposed a number of warnings that I don't think I've seen earlier, in vastly different amounts per distro however. The ones about code issues make sense (like comparison of 500 to an uint8_t variable), I'll hopefully take a look at those. Maybe addressing ones about "obsolete headers" (e.g. preferring termios.h to sys/termios.h nowadays) can also be a quick win to remove tens of warnings from BSD builds ;)

@clepple
Copy link
Copy Markdown
Member

clepple commented Oct 16, 2020

@jimklimov I think the intent was for make distcheck-light to just not include things it couldn't build, but to be honest, I haven't kept tabs on that. We also had talked about having one Buildbot builder create the tarball, and just let the other builders use that tarball (since the slower builders don't really need to be able to create tarballs), but that is non-trivial, and again, maybe it's just as easy to have the Travis builders install whatever dependencies are needed for a full make distcheck.

@clepple
Copy link
Copy Markdown
Member

clepple commented Oct 16, 2020

Maybe addressing ones about "obsolete headers" (e.g. preferring termios.h to sys/termios.h nowadays) can also be a quick win to remove tens of warnings from BSD builds ;)

Sounds reasonable, but we really should test after this change. #833

@jimklimov
Copy link
Copy Markdown
Member Author

As far as this PR is concerned, the line for eb6c6dbd6851... docs/man/Makefile.am : typo fix ">&@" => ">&2" for stderr at http://buildbot.networkupstools.org/public/nut/console is green on all test systems. Waiting for current resync to master to complete, and I think this PR is good to merge.

According to that earlier build's log at http://buildbot.networkupstools.org/public/nut/builders/FreeBSD-x86/builds/922/steps/compile/logs/stdio now the diagnostic message was emitted correctly (so the yet earlier fault was the typo fixed by recent commits):

PASSED man-source sanity check (checked 125 files)
PASSED man-page sanity check (checked 0 files)
Man-page generation was not done, so pregenerated pages were sanity-checked (if any)

@jimklimov jimklimov merged commit 82ba170 into networkupstools:master Oct 25, 2020
@jimklimov
Copy link
Copy Markdown
Member Author

All green in both CI's

@jimklimov jimklimov deleted the doc-sanity branch November 15, 2020 10:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants