Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 1 commit into from
Closed

Conversation

crf8472
Copy link

@crf8472 crf8472 commented Sep 26, 2021

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
Copy link
Contributor

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:

if os.path.basename(file) == 'index.xml':

to:

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

        elif os.path.basename(file) == 'index.xml':

@crf8472
Copy link
Author

crf8472 commented Dec 2, 2021

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
Copy link
Owner

mosra commented Jan 9, 2022

(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 45911a1 -- 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 😅

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

Problem with parsing Doxygen.xml
3 participants