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

Have doxygen run in the doxygen directory #1284

Merged

Conversation

k-stachowiak
Copy link
Contributor

Description

When the Doxywizzard GUI is used and the doxyfile is loaded, the
workind directory for doxygen is set to the location of the doxyfile.
However the Make and CMake build systems expect doxygen to be ran
from the top level directory.
This commit unifies the build system and the Doxywizzard GUI so that
all of them expect doxygen to be executed in the doxygen directory.

Status

READY

Requires Backporting

This change may be considered backporting, but it's impact is limited to the user experience while generating documentation.

Migrations

NO

Additional comments

NONE

Todos

NONE

Steps to test or reproduce

Perform all of the following, deleting the apidoc directory in the top level directory after each step:

  1. Create the documentation using make apidoc command in the top level directory; verify the documentation in the apidoc directory.
  2. Create the documentation using CMake; verify the result as above.
  3. Create the documentation by loading the doxygen/mbedtls.doxyfile into the Doxywizzard and hitting the "Run doxygen" button; again - verify the result.

The results should be identical in all the above steps.

When the Doxywizzard GUI is used and the doxyfile is loaded, the
workind directory for doxygen is set to the location of the doxyfile.
However the Make and CMake build systems expect doxygen to be ran
from the top level directory.
This commit unifies the build system and the Doxywizzard GUI so that
all of them expect doxygen to be executed in the doxygen directory.
@Patater
Copy link
Contributor

Patater commented Jan 19, 2018

I ran "./tests/scripts/doxygen.sh", and ran into many warnings and failures. The development branch doesn't have any of these.

@k-stachowiak Could you please investigate?

Copy link
Contributor

@gilles-peskine-arm gilles-peskine-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the same vein as @Patater : run make apidoc in development and after this change, and compare the outputs — they should be identical. Same with scripts/apidoc_full.sh. With scripts/apidoc_full.sh, they aren't, for example the files in configs appear on the “Files” tab which is not desirable.

Makefile Outdated
@@ -103,7 +103,7 @@ lcov:

apidoc:
mkdir -p apidoc
doxygen doxygen/mbedtls.doxyfile
cd doxygen; doxygen mbedtls.doxyfile
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change cd doxygen; to cd doxygen && so that the second command is not executed if the first fails. This works both under Unix/Posix/Linux and under Windows

@k-stachowiak
Copy link
Contributor Author

Thank you for insightful comments.

@Patater I tried reproducing the errors you mentioned, but I failed. I hope it was a false positive, but I will investigate the possible cause. There are some warnings to be expected though regarding the commented defines in config.h, that will say there is a documentation for unknown defines.

@gilles-peskine-arm I fixed yet unadjusted paths in the exclusion link, which prevents the header files from configs directory from being included in the output. It also fixes come diagrams including these headers. Now the text/binary diff presents no substantial differences.

Copy link
Contributor

@gilles-peskine-arm gilles-peskine-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good now, no regression when running on Linux.

@Patater
Copy link
Contributor

Patater commented Jan 23, 2018

I can confirm that ./tests/scripts/doxygen.sh and ./tests/scripts/check-doxy-blocks.pl scripts pass now. The generated HTML output also looks fine, from a couple clicks around.

I ran make apidoc with the default Makefile and the CMake generated one, and verified HTML output looks alright in both cases.

@Patater Patater merged commit 64c3703 into Mbed-TLS:development Jan 26, 2018
davidhorstmann-arm added a commit that referenced this pull request Aug 30, 2024
Mbedtls 2.28.9rc0 pr DO NOT MERGE
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants