doc, buildsystem: Add a Version Check for Doxygen, Fix doc* Target Execution #21277
+40
−15
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I like the idea. However, I think you need to tweak the warning a bit to make it more visible. Right now the output of |
|
I like the red font idea. I'm not sure about the "Note", maybe rather "WARNING:" in front? to make it clear that it isn't an error per sé, but could lead to issues.
|
|
I tend to go for "Warning" (no caps) to emphasize that the user should pay attention but it is only a warning (no error). In the end, probably >90% of all developers who ever run |
|
This is what the output looks like now. |
crasbe
added
Type: enhancement
crasbe
force-pushed
the
pr/doxygen_versioncheck
branch
from
3cdddec to
f9fb9ef
Compare
mguetschow
requested changes
Mar 7, 2025
mguetschow
reviewed
Mar 7, 2025
mguetschow
reviewed
Mar 7, 2025
No, that was an oversight :-/ |
No worries. Without having to distinguish between the doc-targets, the Makefile got a little simpler, so that's a good thing :) |
|
Some test traces to verify that everything works as it should: The important part here is that the |
mguetschow
reviewed
Mar 7, 2025
Makefile
Outdated
| # Extract the documentation type that is to be generated and export it for | ||
| # the doxygen Makefile | ||
| ifneq ($(findstring doc-,$(MAKECMDGOALS)),) | ||
| DOCUMENTATION_FORMAT := $(patsubst doc-%,%,$(filter doc-%, $(MAKECMDGOALS))) |
There was a problem hiding this comment.
Wouldn't his break with make doc doc-man doc-latex?
There was a problem hiding this comment.
Yes. It broke in many ways :D
I pushed a commit to fix it. Now the approach is to give the Doxygen Makefile the original target and have it determine the DOCUMENTATION_FORMAT itself. Otherwise it is impossible to distinguish it.
I added an option to have html for make doc as well, but now the issue I had was that got called nine times, three times for the targets and three times due to the DOCUMENTATION_FORMAT.
There was a problem hiding this comment.
The trace of make doc doc-man doc-latex with stderr piped to /dev/null to get rid of the Doxygen warnings:
$ make doc doc-man doc-latex 2>/dev/null
"make" -BC doc/doxygen doc
make[1]: Entering directory '/home/cbuec/RIOTstuff/riot-debug-doc/RIOT/doc/doxygen'
awk 'NR == 1 {print $0,"{#coc}"} NR > 1 {print $0}' ../../CODE_OF_CONDUCT.md > src/coc.md
( cat riot.doxyfile ; echo "GENERATE_HTML = yes" ) | doxygen -
RIOT documentation successfully generated at file:///home/cbuec/RIOTstuff/riot-debug-doc/RIOT/doc/doxygen/html/index.html
Warning: Doxygen version 1.12.0 is too old. It is recommended to use at least version 1.14.0 to avoid incorrectly formatted output.
make[1]: Leaving directory '/home/cbuec/RIOTstuff/riot-debug-doc/RIOT/doc/doxygen'
"make" -BC doc/doxygen doc-man
make[1]: Entering directory '/home/cbuec/RIOTstuff/riot-debug-doc/RIOT/doc/doxygen'
awk 'NR == 1 {print $0,"{#coc}"} NR > 1 {print $0}' ../../CODE_OF_CONDUCT.md > src/coc.md
( cat riot.doxyfile ; echo "GENERATE_MAN = yes" ) | doxygen -
RIOT documentation successfully generated at file:///home/cbuec/RIOTstuff/riot-debug-doc/RIOT/doc/doxygen/man/man3
Warning: Doxygen version 1.12.0 is too old. It is recommended to use at least version 1.14.0 to avoid incorrectly formatted output.
make[1]: Leaving directory '/home/cbuec/RIOTstuff/riot-debug-doc/RIOT/doc/doxygen'
"make" -BC doc/doxygen doc-latex
make[1]: Entering directory '/home/cbuec/RIOTstuff/riot-debug-doc/RIOT/doc/doxygen'
awk 'NR == 1 {print $0,"{#coc}"} NR > 1 {print $0}' ../../CODE_OF_CONDUCT.md > src/coc.md
( cat riot.doxyfile ; echo "GENERATE_LATEX= yes" ) | doxygen -
RIOT documentation successfully generated at file:///home/cbuec/RIOTstuff/riot-debug-doc/RIOT/doc/doxygen/latex/index.tex
Warning: Doxygen version 1.12.0 is too old. It is recommended to use at least version 1.14.0 to avoid incorrectly formatted output.
make[1]: Leaving directory '/home/cbuec/RIOTstuff/riot-debug-doc/RIOT/doc/doxygen'
|
Oh and unfortunately this PR also breaks |
This does not seem to work for me with |
|
I also added the "Documentation successfully generated" strings for the |
So what happens is that Lines 882 to 883 in 999220c And for some reason, when the This does not change even when I add the typical Even when commenting out the includes, this error is shown: |
|
With all debug information enabled, we get some more clues: Make LogThe important part here is in the lower part, when The But IMO this is way out of scope for this PR already 😅 |
|
@mguetschow let me know when I can squash this. I created a separate issue I added the fix for the issue to this PR and added the testing procedure to the first message. |
crasbe
force-pushed
the
pr/doxygen_versioncheck
branch
from
bef9041 to
1e3202d
Compare
crasbe
changed the title
Sorry for not interfering before, but yeah indeed, this would make more sense as a separate PR 🙈 |
mguetschow
approved these changes
Mar 7, 2025
crasbe
force-pushed
the
pr/doxygen_versioncheck
branch
from
1e3202d to
7728ef2
Compare
On one hand it would've made more sense as a separate PR, but on the other hand it would've created a hen-and-egg situation again like the STM32 ADC PRs. You need one PR to properly test the other and vice versa. With the two commits, it's somewhat separated I guess. |
|
Thank you for your inputs and patience. I learned a lot about Make and the RIOT build system again and we even found some bugs along the way :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Contribution description
Doxygen receives a lot of updates (and lately some updates that fixed issues found when generating the RIOT documentation) and releases happen more recently than a couple of years ago.
Unfortunately some (cough debian based) distributions don't update it. Ubuntu 22.04 LTS for example will probably be forever stuck at version 1.9.1 from 2021.
To avoid issues with malformatted documentation due to an outdated Doxygen version, this PR adds a warning if the Doxygen version is too old. The warning is delibrately put at the end of the Make process due to the wall of warnings generated by Doxygen.
Request for comments: I don't think the solution with
awkis super pretty, but doing stuff like that with Make is quite annoying. Another option would be to have a dedicatedcheck_version.sh.It probably makes sense to wait with merging this until after Doxygen 1.14.0 is released. But on the other hand, the note is still valid, even before the release 🤷♂️
During the development of this PR, some issues with the build system surfaced regarding the
docanddoc-targets. Thebuildsystem: fix execution of doc targetscommit fixes that. The details are outlined in issue #21278.Testing procedure
Run
make docon a machine with a current release version of Doxygen (1.14.0 is not released yet) and observe the output:If you updated to the latest master branch of Doxygen and compiled it yourself, the warning will not be generated:
The second part of this PR can be tested with the following commands:
Current master:
This PR:
Issues/PRs references
Related to the work in #21106 and #21220.
Fixes #21278.