mosra / m.css

A no-nonsense, no-JavaScript CSS framework, site and documentation theme for content-oriented websites
https://mcss.mosra.cz
MIT License
399 stars 88 forks source link

Suggested fix for #206: processing error on doxygen 1.9.x XML output #207

Closed crf8472 closed 2 years ago

crf8472 commented 2 years ago

Doxygen since 1.9.x outputs an XML-file "Doxyfile.xml" whose root element is "doxyfile" (instead of "doxygen") and its schema info refers to "doxyfile.xsd" (instead of "compound.xsd). The root element has no "compounddef" element as first child.

AFAIR this breaks m-css's document compilation with doxygen 1.9.x.

m-css processes every XML-file in doxygen's output directory by name-globbing, regardless of the file's name or content. (I did not test it but I would expect the current processing to fail if any deviating XML files are added in the directory.)

extract_metadata() assumes that each XML-file contains an element 'compounddef' and fails otherwise (except for index.xml, which is explicitly checked for).

Two assertions would also fail in parse_xml() if the root element of the parsed XML-file is not 'doxygen' or the file is not index.xml (or no 'compounddef' is present).

I would like to suggest that in case the input XML turns out to be unexpected, just discard the current file but proceed with the next input file.

The patch I would like to propose just replaces the two assertions in parse_xml() with a clause that just skips the current input file (at least this was the intention). I also added a similar skip-clause in extract_metadata() that just discards the input if no 'compounddef' element is present. This should also make it at least a little harder for m-css to fail just because of unexpected XML files in the doxygen output directory.

Proposes fix #206

SRGDamia1 commented 2 years ago

I think it would be better to just modify the run function to ignore doxyfile.xml. That way you don't have to mess with any of the other functions and you can keep all the assertion checks in place.

change: https://github.com/mosra/m.css/blob/9385194fa3392a7162e7535cc2478814e382ff8a/documentation/doxygen.py#L3693

to:

        if os.path.basename(file) == 'Doxyfile.xml':
            logging.debug("Skipping xml of doxyfile")

        elif os.path.basename(file) == 'index.xml':
crf8472 commented 2 years ago

While this would solve the actual problem with Doxyfile.xml, it would leave the behaviour untouched that any XML file (except Doxyfile.xml) in the current directory is tried to be processed with the expectation to actually match the requested doxygen XML namespace. If this expectation is not met, doxygen.py would crash.

To me it seems therefore cleaner and more robust to skip those XML files which cannot be recognized as matching the expected namespace. Checking for namespace and skipping would prevent doxygen.py from behaving differently in the presence of an XML file in the current directory that obviously should not be processed.

However, this may of course be considered a different issue. Nonetheless it would solve the problem with Doxyfile.xml in a generic manner.

mosra commented 2 years ago

(Sorry for embarrassingly late replies, finally got back to this project and to the final boss that is Doxygen.)

Thank you both! Combined together, this is quite clean, and I commited it as 45911a188292a62b49ca4b226f590e9ca8cb75fa -- the Doxyfile.xml gets ignored silently (well, with a debug log), and any other XML files that are suspicious get ignored with a warning (and no assert anymore). Hopefully this more robust handling and added regression tests will make it survive future Doxygen upgrades better :sweat_smile:

While the commit makes Doxygen 1.9 finally not blow up, please note the tool is not fully updated to it yet, there's still about 10 test errors and 4 nasty assertions triggered by changes to the XML files. I still have to patch that up, which will be happening over the next days.