Re: Documentation building broken in CFBot

Since recently, all Travis builds in the CFBot are broken for docs builds in
the xmllint step. Turns out, the oasis-open.org URL to the DTD is now
redirecting to HTTPS and xmllint is normally only compiled with HTTP support
which causes this to now break.

That being said, XMLLint will load the DTD locally if available so environments
which have the packages referenced in the postgres docs won't notice as they
provide the DTD. The package containing the DTD is however not installed on
the CFBot. I've opened a PR against the CFBot repo [0], but until merged the
builds will be red without necessarily being an issue with the patch being
built.

cheers ./daniel

[0] https://github.com/macdice/cfbot/pull/12

On 11/11/20 5:47 AM, Daniel Gustafsson wrote:
> Since recently, all Travis builds in the CFBot are broken for docs builds in
> the xmllint step. Turns out, the oasis-open.org URL to the DTD is now
> redirecting to HTTPS and xmllint is normally only compiled with HTTP support
> which causes this to now break.

Interesting. I ran into this issue yesterday when I was trying to build
the docs locally.

> That being said, XMLLint will load the DTD locally if available so environments
> which have the packages referenced in the postgres docs won't notice as they
> provide the DTD. The package containing the DTD is however not installed on
> the CFBot. I've opened a PR against the CFBot repo [0], but until merged the
> builds will be red without necessarily being an issue with the patch being
> built.

I do wonder if it's worth it to update the doc build documentation[1] as
well to mention this?

Jonathan

[1] /docs/current/docguide-toolsets.html

> On 11 Nov 2020, at 15:51, Jonathan S. Katz <jkatz(at)postgresql(dot)org> wrote:

> I do wonder if it's worth it to update the doc build documentation[1] as
> well to mention this?

Well, ideally you would never encounter this error if you install the packages
listed as required. That being said, the lists must be up to date of course.
Since I assume you were building on macOS, which package manager are you using?

My guess is that docbook-xml should be added to the macports install set, but I
don't use macports myself so thats just guessing based on what their packages
seem to contain.

Homebrew doesn't have a docbook-xml package, but the docbook package seems to
contain what other have in docbook-xml.

cheers ./daniel

Daniel Gustafsson <daniel(at)yesql(dot)se> writes:
>> On 11 Nov 2020, at 15:51, Jonathan S. Katz <jkatz(at)postgresql(dot)org> wrote:
>> I do wonder if it's worth it to update the doc build documentation[1] as
>> well to mention this?

> Well, ideally you would never encounter this error if you install the packages
> listed as required.

I wonder if we should start passing "--nonet" to xmllint et al. I've
always felt that that silent reach into the internet was an anti-feature
on numerous grounds: it's slow, it's not very secure, it makes the results
uncertainly reproducible, etc etc etc.

regards, tom lane

On Wed, Nov 11, 2020 at 4:43 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Daniel Gustafsson <daniel(at)yesql(dot)se> writes:
> >> On 11 Nov 2020, at 15:51, Jonathan S. Katz <jkatz(at)postgresql(dot)org> wrote:
> >> I do wonder if it's worth it to update the doc build documentation[1] as
> >> well to mention this?
>
> > Well, ideally you would never encounter this error if you install the packages
> > listed as required.
>
> I wonder if we should start passing "--nonet" to xmllint et al. I've
> always felt that that silent reach into the internet was an anti-feature
> on numerous grounds: it's slow, it's not very secure, it makes the results
> uncertainly reproducible, etc etc etc.

I think we should. It would also lead to "break early" which is easier
to diagnose than "break at a random point when someone external
changed something".

--
Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/

On 11/11/20 10:43 AM, Tom Lane wrote:
> Daniel Gustafsson <daniel(at)yesql(dot)se> writes:
>>> On 11 Nov 2020, at 15:51, Jonathan S. Katz <jkatz(at)postgresql(dot)org> wrote:
>>> I do wonder if it's worth it to update the doc build documentation[1] as
>>> well to mention this?
>
>> Well, ideally you would never encounter this error if you install the packages
>> listed as required.
>
> I wonder if we should start passing "--nonet" to xmllint et al. I've
> always felt that that silent reach into the internet was an anti-feature
> on numerous grounds: it's slow, it's not very secure, it makes the results
> uncertainly reproducible, etc etc etc.

It was still bugging out for me locally with --nonet, but that could
also be operator error.

Agree overall though, it seems to go against the grain of a lot of the
other things that we do. We do make a network request for loading the
website styles when building the docs, but that is also an optional
feature. Right now, I can't even get "make html" to successfully run.

Jonathan

"Jonathan S. Katz" <jkatz(at)postgresql(dot)org> writes:
> On 11/11/20 10:43 AM, Tom Lane wrote:
>> I wonder if we should start passing "--nonet" to xmllint et al. I've
>> always felt that that silent reach into the internet was an anti-feature
>> on numerous grounds: it's slow, it's not very secure, it makes the results
>> uncertainly reproducible, etc etc etc.

> It was still bugging out for me locally with --nonet, but that could
> also be operator error.

The point of --nonet is that then you would *have to* install the DTDs
locally in order to build the docs.

(Well, I wouldn't be quite that draconian about it; if we do this,
I think it should be more like "we use --nonet by default but you can
override that".)

regards, tom lane

On 11/11/20 11:11 AM, Tom Lane wrote:
> "Jonathan S. Katz" <jkatz(at)postgresql(dot)org> writes:
>> On 11/11/20 10:43 AM, Tom Lane wrote:
>>> I wonder if we should start passing "--nonet" to xmllint et al. I've
>>> always felt that that silent reach into the internet was an anti-feature
>>> on numerous grounds: it's slow, it's not very secure, it makes the results
>>> uncertainly reproducible, etc etc etc.
>
>> It was still bugging out for me locally with --nonet, but that could
>> also be operator error.
>
> The point of --nonet is that then you would *have to* install the DTDs
> locally in order to build the docs.

Yup, I had tried installing the DTDs locally, and clearly failed at it
:) I also did not spend too much time trying to troubleshoot my failures.

> (Well, I wouldn't be quite that draconian about it; if we do this,
> I think it should be more like "we use --nonet by default but you can
> override that".)
That sounds reasonable.

Also FWIW yesterday I had tried quickly swapping the DTD references to
use HTTPS but still hit similar issues, and per above I did not spend
much time trying to troubleshoot.

Jonathan

> On 11 Nov 2020, at 16:43, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Daniel Gustafsson <daniel(at)yesql(dot)se> writes:
>>> On 11 Nov 2020, at 15:51, Jonathan S. Katz <jkatz(at)postgresql(dot)org> wrote:
>>> I do wonder if it's worth it to update the doc build documentation[1] as
>>> well to mention this?
>
>> Well, ideally you would never encounter this error if you install the packages
>> listed as required.
>
> I wonder if we should start passing "--nonet" to xmllint et al. I've
> always felt that that silent reach into the internet was an anti-feature
> on numerous grounds: it's slow, it's not very secure, it makes the results
> uncertainly reproducible, etc etc etc.

..and it will most likely come as a surprise to most. I think we should start
doing that.

cheers ./daniel

> On 11 Nov 2020, at 17:13, Jonathan S. Katz <jkatz(at)postgresql(dot)org> wrote:
>
> On 11/11/20 11:11 AM, Tom Lane wrote:
>> "Jonathan S. Katz" <jkatz(at)postgresql(dot)org> writes:
>>> On 11/11/20 10:43 AM, Tom Lane wrote:
>>>> I wonder if we should start passing "--nonet" to xmllint et al. I've
>>>> always felt that that silent reach into the internet was an anti-feature
>>>> on numerous grounds: it's slow, it's not very secure, it makes the results
>>>> uncertainly reproducible, etc etc etc.
>>
>>> It was still bugging out for me locally with --nonet, but that could
>>> also be operator error.
>>
>> The point of --nonet is that then you would *have to* install the DTDs
>> locally in order to build the docs.
>
> Yup, I had tried installing the DTDs locally, and clearly failed at it
> :) I also did not spend too much time trying to troubleshoot my failures.

Looking at config/docbook.m4 we check for the existence of the DTDs, but we
don't seem to use the findings in have_docbook? Shouldn't we do something like
the below:

--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -492,6 +492,7 @@ DBTOEPUB = @DBTOEPUB@
FOP = @FOP@
XMLLINT = @XMLLINT@
XSLTPROC = @XSLTPROC@
+have_docbook = @have_docbook@

..and then error out in the doc/ Makefiles for the relevant targets in case
have_docbook is "no"? Seems like that could save troubleshooting efforts like
the one referenced above. Commit 7267fdd7a3a did in fact include this in the
global Makefile but has been removed at some point (which I'm currently too
undercaffeinated to find).

cheers ./daniel

On Wed, Nov 11, 2020 at 11:47 PM Daniel Gustafsson <daniel(at)yesql(dot)se> wrote:
> [0] https://github.com/macdice/cfbot/pull/12

Done, thanks! Will probably take a few hours for the results to show up.

On 2020-11-11 21:04, Daniel Gustafsson wrote:
>> On 11 Nov 2020, at 16:43, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>
>> Daniel Gustafsson <daniel(at)yesql(dot)se> writes:
>>>> On 11 Nov 2020, at 15:51, Jonathan S. Katz <jkatz(at)postgresql(dot)org> wrote:
>>>> I do wonder if it's worth it to update the doc build documentation[1] as
>>>> well to mention this?
>>
>>> Well, ideally you would never encounter this error if you install the packages
>>> listed as required.
>>
>> I wonder if we should start passing "--nonet" to xmllint et al. I've
>> always felt that that silent reach into the internet was an anti-feature
>> on numerous grounds: it's slow, it's not very secure, it makes the results
>> uncertainly reproducible, etc etc etc.
>
> ..and it will most likely come as a surprise to most. I think we should start
> doing that.

How about this patch.

> On 13 Nov 2020, at 12:16, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com> wrote:
>
> On 2020-11-11 21:04, Daniel Gustafsson wrote:
>>> On 11 Nov 2020, at 16:43, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>>
>>> Daniel Gustafsson <daniel(at)yesql(dot)se> writes:
>>>>> On 11 Nov 2020, at 15:51, Jonathan S. Katz <jkatz(at)postgresql(dot)org> wrote:
>>>>> I do wonder if it's worth it to update the doc build documentation[1] as
>>>>> well to mention this?
>>>
>>>> Well, ideally you would never encounter this error if you install the packages
>>>> listed as required.
>>>
>>> I wonder if we should start passing "--nonet" to xmllint et al. I've
>>> always felt that that silent reach into the internet was an anti-feature
>>> on numerous grounds: it's slow, it's not very secure, it makes the results
>>> uncertainly reproducible, etc etc etc.
>> ..and it will most likely come as a surprise to most. I think we should start
>> doing that.
>
> How about this patch.

LGTM, thanks.

cheers ./daniel

Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com> writes:
> On 2020-11-11 21:04, Daniel Gustafsson wrote:
>> On 11 Nov 2020, at 16:43, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> I wonder if we should start passing "--nonet" to xmllint et al.

> How about this patch.

This looks like it probably works as far as the actual build goes, but
could we change configure's call of xmllint likewise? On a lot of my test
machines (where, as a rule, I don't install DocBook; but xmllint may exist
anyway), that probe for the docbook DTDs is the slowest step in configure,
and not by a small margin.

Actually, I kinda wonder why we bother probing for that at all; it doesn't
look like anything is done with have_docbook.

regards, tom lane

I wrote:
> Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com> writes:
>> How about this patch.

> This looks like it probably works as far as the actual build goes, but
> could we change configure's call of xmllint likewise? On a lot of my test
> machines (where, as a rule, I don't install DocBook; but xmllint may exist
> anyway), that probe for the docbook DTDs is the slowest step in configure,
> and not by a small margin.

> Actually, I kinda wonder why we bother probing for that at all; it doesn't
> look like anything is done with have_docbook.

This came up again today on another thread [1], so I took a second look
and confirmed that configure's test is completely dead code. So here is
a patch to remove it outright, mooting the question of whether it ought
to use --no-net.

After removing PGAC_CHECK_DOCBOOK, there wasn't much left of docbook.m4,
and what there was had little to do with docbook; so I chose to just
remove that file altogether.

This is actually entirely independent of Peter's patch. I'm tempted
to propose that we could back-patch it and thereby save some configure
cycles for developers, since it has no impact on anybody's build
process.

regards, tom lane

[1] /message-id/flat/A55A7FC9-FA60-47FE-98B5-139CDC57CE6E%40gmail.com

On 2020-11-26 18:24, Tom Lane wrote:
> This came up again today on another thread [1], so I took a second look
> and confirmed that configure's test is completely dead code. So here is
> a patch to remove it outright, mooting the question of whether it ought
> to use --no-net.
>
> After removing PGAC_CHECK_DOCBOOK, there wasn't much left of docbook.m4,
> and what there was had little to do with docbook; so I chose to just
> remove that file altogether.
>
> This is actually entirely independent of Peter's patch. I'm tempted
> to propose that we could back-patch it and thereby save some configure
> cycles for developers, since it has no impact on anybody's build
> process.

Yes, this seems fine. docbook.m4 used to do more interesting things,
when we had DSSSL, and no catalog mechanisms, and generally more
complicated installations. It also served as a sort of test suite, when
if someone complained that they can't build the documentation, we could
look at config.log to see if their tools were installed correctly. But
right now it's not that interesting anymore.

Backpatching to PG11 should be okay. Before that, it's different tools.

On 2020-11-13 15:58, Tom Lane wrote:
> Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com> writes:
>> On 2020-11-11 21:04, Daniel Gustafsson wrote:
>>> On 11 Nov 2020, at 16:43, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>>> I wonder if we should start passing "--nonet" to xmllint et al.
>
>> How about this patch.
>
> This looks like it probably works as far as the actual build goes, but
> could we change configure's call of xmllint likewise? On a lot of my test
> machines (where, as a rule, I don't install DocBook; but xmllint may exist
> anyway), that probe for the docbook DTDs is the slowest step in configure,
> and not by a small margin.
>
> Actually, I kinda wonder why we bother probing for that at all; it doesn't
> look like anything is done with have_docbook.

We could change the call in configure to something like

PGAC_PATH_PROGS(XMLLINT, 'xmllint --nonet')

But the unfortunate effect of that would be to make it harder for casual
contributors to build the documentation. For example, on a new MacBook,
the documentation builds out of the box without needing to install
anything at all. That's a pretty big upgrade over 10 years ago. :)

I suggest if someone wants to turn off the current behavior, they can set

XMLLINT='xmllint --nonet'
XSLTPROC='xsltproc --nonet'

in the environment, and it will get picked up automatically.

Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com> writes:
> On 2020-11-26 18:24, Tom Lane wrote:
>> This came up again today on another thread [1], so I took a second look
>> and confirmed that configure's test is completely dead code. So here is
>> a patch to remove it outright, mooting the question of whether it ought
>> to use --no-net.

> Yes, this seems fine. docbook.m4 used to do more interesting things,
> when we had DSSSL, and no catalog mechanisms, and generally more
> complicated installations. It also served as a sort of test suite, when
> if someone complained that they can't build the documentation, we could
> look at config.log to see if their tools were installed correctly. But
> right now it's not that interesting anymore.

Check.

> Backpatching to PG11 should be okay. Before that, it's different tools.

Ah, I see there used to be more things in docbook.m4, so removing the
file altogether isn't feasible before v11. But we could still drop
the PGAC_CHECK_DOCBOOK macro, no? Even in the older branches, there
is nothing paying attention to have_docbook.

regards, tom lane

Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com> writes:
> We could change the call in configure to something like

> PGAC_PATH_PROGS(XMLLINT, 'xmllint --nonet')

> But the unfortunate effect of that would be to make it harder for casual
> contributors to build the documentation. For example, on a new MacBook,
> the documentation builds out of the box without needing to install
> anything at all. That's a pretty big upgrade over 10 years ago. :)

I agree that that has its advantages. To me, the big problem here is
with configure's call, because that is executed whether or not you want
to build the documentation. If we flushed PGAC_CHECK_DOCBOOK, then
I'd only about 10% care whether we make the other change at all, or
whether --nonet is the default or not.

regards, tom lane

On 2020-11-27 18:53, Tom Lane wrote:
>> Yes, this seems fine. docbook.m4 used to do more interesting things,
>> when we had DSSSL, and no catalog mechanisms, and generally more
>> complicated installations. It also served as a sort of test suite, when
>> if someone complained that they can't build the documentation, we could
>> look at config.log to see if their tools were installed correctly. But
>> right now it's not that interesting anymore.
>
> Check.
>
>> Backpatching to PG11 should be okay. Before that, it's different tools.
>
> Ah, I see there used to be more things in docbook.m4, so removing the
> file altogether isn't feasible before v11. But we could still drop
> the PGAC_CHECK_DOCBOOK macro, no? Even in the older branches, there
> is nothing paying attention to have_docbook.

Well, my point of having this as a mini test suite of the installation
still applies there, especially since it uses the old tool set that is
more difficult to install correctly. Those old tools have no network
functionality, so there is really no configure speed to be gained. I
would leave it alone.

In any case, note that there is also documentation about this at
</docs/devel/docguide-toolsets.html#DOCGUIDE-TOOLSETS-CONFIGURE>
that would need to be updated to the new behavior. I think the phrase
"If xmllint was not found then some of the following tests will be
skipped." could be removed, since it referred to the DocBook test about
to be removed.

Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com> writes:
> On 2020-11-27 18:53, Tom Lane wrote:
>> Ah, I see there used to be more things in docbook.m4, so removing the
>> file altogether isn't feasible before v11. But we could still drop
>> the PGAC_CHECK_DOCBOOK macro, no? Even in the older branches, there
>> is nothing paying attention to have_docbook.

> Well, my point of having this as a mini test suite of the installation
> still applies there, especially since it uses the old tool set that is
> more difficult to install correctly. Those old tools have no network
> functionality, so there is really no configure speed to be gained. I
> would leave it alone.

Wow, has it really only been three years that configure has acted this
way? But anyway, after testing on a system that has opensp installed
but not docbook, I confirm that the DocBook test falls through quickly
pre-v11. Agreed then that we only need this change in 11 and up.

> In any case, note that there is also documentation about this at
> </docs/devel/docguide-toolsets.html#DOCGUIDE-TOOLSETS-CONFIGURE>
> that would need to be updated to the new behavior. I think the phrase
> "If xmllint was not found then some of the following tests will be
> skipped." could be removed, since it referred to the DocBook test about
> to be removed.

Right. That also seems like a good place to add a hint about setting
XMLLINT to 'xmllint --nonet' if you don't want remote fetches to happen
while building the docs. Shall we do that instead of applying the
patch you posted earlier?

regards, tom lane

I wrote:
> Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com> writes:
>> In any case, note that there is also documentation about this at
>> </docs/devel/docguide-toolsets.html#DOCGUIDE-TOOLSETS-CONFIGURE>
>> that would need to be updated to the new behavior. I think the phrase
>> "If xmllint was not found then some of the following tests will be
>> skipped." could be removed, since it referred to the DocBook test about
>> to be removed.

> Right. That also seems like a good place to add a hint about setting
> XMLLINT to 'xmllint --nonet' if you don't want remote fetches to happen
> while building the docs. Shall we do that instead of applying the
> patch you posted earlier?

Concretely, maybe like the attached?

(Looking at this, I can't avoid the feeling that the programs are being
checked for in a completely random order. I'm tempted to rearrange
configure to probe xmllint, xsltproc, fop, dbtoepub in that order,
so it matches the proposed documentation.)

regards, tom lane

On 11/11/20 11:13 AM, Jonathan S. Katz wrote:
> On 11/11/20 11:11 AM, Tom Lane wrote:
>> "Jonathan S. Katz" <jkatz(at)postgresql(dot)org> writes:
>>> On 11/11/20 10:43 AM, Tom Lane wrote:
>>>> I wonder if we should start passing "--nonet" to xmllint et al. I've
>>>> always felt that that silent reach into the internet was an anti-feature
>>>> on numerous grounds: it's slow, it's not very secure, it makes the results
>>>> uncertainly reproducible, etc etc etc.
>>
>>> It was still bugging out for me locally with --nonet, but that could
>>> also be operator error.
>>
>> The point of --nonet is that then you would *have to* install the DTDs
>> locally in order to build the docs.
>
> Yup, I had tried installing the DTDs locally, and clearly failed at it
> :) I also did not spend too much time trying to troubleshoot my failures.
>
>> (Well, I wouldn't be quite that draconian about it; if we do this,
>> I think it should be more like "we use --nonet by default but you can
>> override that".)
> That sounds reasonable.
>
> Also FWIW yesterday I had tried quickly swapping the DTD references to
> use HTTPS but still hit similar issues, and per above I did not spend
> much time trying to troubleshoot.

FYI -- I don't know what, if anything changed, but I'm able to build
docs again on my macOS machine without modifying anything.

So if someone did something -- thanks!

Jonathan

"Jonathan S. Katz" <jkatz(at)postgresql(dot)org> writes:
> FYI -- I don't know what, if anything changed, but I'm able to build
> docs again on my macOS machine without modifying anything.

We didn't really do anything here. I wonder if oasis-open.org fixed
their HTTP redirect?

regards, tom lane

On 2/6/21 11:49 AM, Tom Lane wrote:
> "Jonathan S. Katz" <jkatz(at)postgresql(dot)org> writes:
>> FYI -- I don't know what, if anything changed, but I'm able to build
>> docs again on my macOS machine without modifying anything.
>
> We didn't really do anything here. I wonder if oasis-open.org fixed
> their HTTP redirect?

Did a quick curl and it seems that by "fixing the redirect" it's
allowing the connection over HTTP.

Either way, it just works now.

Jonathan