docbook / xslt10-stylesheets

XSLT 1.0 Stylesheets for DocBook
97 stars 76 forks source link

1.79.2 causes xsltproc to generate misformatted manpages #65

Open ryandesign opened 6 years ago

ryandesign commented 6 years ago

Hi, I'm a developer with the MacPorts project, where it was reported to us that xsltproc(1) (from the libxslt project) is generating manpages that are misformatted due to improper whitespace having been inserted before control codes. I tracked this down to version 1.79.2 of the xslt stylesheets. Downgrading to 1.79.1 fixes the problem.

Do you have any idea what might have caused this? It's difficult for me analyze, since I'm not familiar with your code, and since between those two versions the project moved to GitHub, the NEWS file is empty, there is no ChangeLog file, the diff of changes between the two versions is over 158,000 lines long, and there are almost two years worth of commits. There aren't that many commits, but it's hard for me to bisect, since I can't seem to run your build system; building.md just says to run make, but that only generates a zillion error messages.

I see now that there is also xsl/README.BUILD with 800 lines of build instructions; I haven't attempted to follow that yet.

bobstayton commented 6 years ago

I have one idea of what might have caused this. Versions 1.79.1 and 1.79.2 should be functionally equivalent. The difference is that we switched the source from non-namespaced XML to namespaced XML, and now build the non-namespaced XML. And we renamed the packages. So docbook-xsl-nons-1.79.2 is equivalent to docbook-xsl-1.79.1, and docbook-xsl-1.79.2 is equivalent to docbook-xsl-ns-1.79.1. So see if the whitespace issue still persists if you switch the package.

mouse07410 commented 6 years ago

Besides switching the packages, is there a simple way to make docbook-xsl-1.79.2 build the non-namespaced XML? Hopefully/ideally one line in man.xsl.txt or api.css.txt?

ryandesign commented 6 years ago

So see if the whitespace issue still persists if you switch the package.

Yes, that fixes it, thanks.

In MacPorts, we currently have two ports: docbook-xsl (non-namespaced) and docbook-xsl-ns (namespaced). For whatever reason, they were added to MacPorts as two separate ports, and they haven't been kept in sync. Our docbook-xsl was updated to the namespaced version of 1.79.2, and our docbook-xsl-ns was still at 1.76.0. I will see what I can do about unifying these two ports so that they don't get out of sync again.

ryandesign commented 6 years ago

When MacPorts installs the namespaced and/or non-namespaced docbook-xsl, it then runs xmlcatmgr(1) with the catalog.xml file, which, if I understand correctly, somehow informs other software that wants to use docbook-xsl how to do that.

So when we inadvertently updated our non-namespaced docbook-xsl port to the namespaced docbook-xsl, software like libxslt's xsltproc(1) that wanted to use non-namespaced docbook-xsl should not have been able to find it, since we no longer provided it. Do I understand this correctly?

If so, why did xsltproc(1) silently use the namespaced docbook-xsl instead, which caused it to produce incorrect output? If xsltproc(1) is asking xmlcatmgr(1) to use non-namespaced docbook-xsl, why was it given the namespaced docbook-xsl, instead of an error message along the lines of "non-namespaced docbook-xsl is not installed"?

ryandesign commented 6 years ago

I think I understand now. Catalogs are looked up by their canonical URL. For docbook-xsl, that used to be http://docbook.sourceforge.net/release/xsl/current/ and for docbook-xsl-ns, it used to be http://docbook.sourceforge.net/release/xsl-ns/current/. Shortly after we updated docbook-xsl to 1.79.2, we noticed build failures of other software, with error messages like:

I/O error : Attempt to load network entity http://docbook.sourceforge.net/release/xsl/current/xhtml/docbook.xsl
warning: failed to load external entity "http://docbook.sourceforge.net/release/xsl/current/xhtml/docbook.xsl"
compilation error: file ./doc/html.xsl line 3 element import
xsl:import : unable to load http://docbook.sourceforge.net/release/xsl/current/xhtml/docbook.xsl

We realized that docbook-xsl had changed its canonical URL to cdn.docbook.org, but that existing projects in the wild used the old canonical URL at docbook.sourceforge.net. Rather than declare ourselves responsible for updating the URLs in every program that used docbook-xsl, we installed a second catalog.xml file referencing the old URL. That caused the problem, because we inadvertently mapped requests for the old URL (which was for the non-namespaced version) to the new URL (which was for the namespaced version).

I will correct our second catalog.xml, but it would be nice if docbook-xsl would ship with its own backward-compatibility catalogs.

0-wiz-0 commented 4 years ago

One upvote for "I will correct our second catalog.xml, but it would be nice if docbook-xsl would ship with its own backward-compatibility catalogs."

pemensik commented 3 years ago

Hi. I have hit this strange error sometime later. It is quite confusing to have two almost same versions, both accepting the same input but one always generating wrong manual pages. Discussion with ISC is on ISC Gitlab.

I think at least manuals/docbook.xsl template should not try to strip out xmlns of docbook. Instead, it should emit at least warning but better error, that incorrect version of stylesheets is used. It works somehow for html, but it seems it always fails with manual pages. My question is therefore, why it does not emit warning with hints different kind of stylesheet should be used? It seems multiple of people hit this bug already years ago. It seems it knows whether it is docbook5 or docbook4 format, correct? Why do I have to check contents of generated manuals to verify correctness?

Especially when default have changed with last version, it should emit warnings on improper use. I barely know how it works, but would try to prepare PR if no-one else volunteers. It would be nice, if new version contained also fixes for changed urls.

rdmark commented 1 year ago

One more upvote for backwards compatibility. This issue is affecting the netatalk project as well.