Skip to content

Add doxygen docs to Github Pages#8329

Open
yohannd1 wants to merge 21 commits intoLMMS:masterfrom
yohannd1:doxygen
Open

Add doxygen docs to Github Pages#8329
yohannd1 wants to merge 21 commits intoLMMS:masterfrom
yohannd1:doxygen

Conversation

@yohannd1
Copy link
Copy Markdown
Contributor

@yohannd1 yohannd1 commented Mar 25, 2026

This PR adds a github workflow that runs doxygen on the codebase and uploads the generated HTML pages to Github Pages. My aim with this is to make reading the docs easier and browsing through the documentation simpler.

This workflow was based off the build workflow already present in the repo, plus some prior Github Pages experiments I've worked on.

Things to consider:

@yohannd1
Copy link
Copy Markdown
Contributor Author

Note that the CI run inside the PR does not work fully, as it is denied acess to github pages. Not sure if there's a way to work around that. In fact, I'll be disabling this run on PRs for now.

image

@messmerd
Copy link
Copy Markdown
Member

Following the advice from this Stack Overflow article, I just set the GitHub Pages source for this repo to GitHub Actions.

Try again and see if it works now.

@JohannesLorenz JohannesLorenz self-requested a review March 26, 2026 00:08
@yohannd1
Copy link
Copy Markdown
Contributor Author

That seems to not have worked - it did go farther, but it still has permission-related errors:
image
I will now be trying rossjrw/pr-preview-action, which might be able to solve this.

@messmerd
Copy link
Copy Markdown
Member

messmerd commented Mar 26, 2026

I will now be trying rossjrw/pr-preview-action, which might be able to solve this.

I don't think that will work, unless we set things up differently here.

From that action's readme:
image

@yohannd1
Copy link
Copy Markdown
Contributor Author

Ah, I see. From what I've read here it seems it is possible to adapt the workflow here, but I'm not sure if it would be worth it - pushing tons of generated HTML and graph files of tons of PRs is probably going to bloat the repo a lot.

Copy link
Copy Markdown
Contributor

@JohannesLorenz JohannesLorenz left a comment

Choose a reason for hiding this comment

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

What I checked:

  • Code
  • Style

name: linux-x86_64
runs-on: ubuntu-22.04
env:
CMAKE_OPTS: >-
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All the dependencies and make options should not be relevant to Doxygen? (except the doxygen dependency)

Copy link
Copy Markdown
Contributor Author

@yohannd1 yohannd1 Mar 26, 2026

Choose a reason for hiding this comment

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

I believe the make options indeed are not relevant, I'll be removing these.
As for the other dependencies, I'm not sure. I think some are needed so Doxygen can figure things out, or at least to list them (this depends on the other discussion).
In any case, the wine dependency is probably not needed?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Very likely not.

Can you run once with and once without dependencies, and compare the results which doxygen produces?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ran it without most dependencies, and it seems the output hasn't changed. The class list, at least, seems identical. My concern was that plugins such as ZynAddSubFx wouldn't be included there but that hasn't happened.
Note that I'm still including the Ubuntu 20.04 dependency set, as it includes dependencies on Qt and other libraries, but I'll briefly try removing most of these as well.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

but I'll briefly try removing most of these as well.

Okay, that didn't work.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks like we need to provide the minimum dependencies to let CMake pass, and that's all we need...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Managed to remove a few more dependencies. I still don't know about removing libraries used by the code though, so I'm leaving these for now.

run: |
cmake --build fltk/build
sudo cmake --install fltk/build --prefix /usr
- name: Configure
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A lot of classes/types by Doxygen (almost all) are not by us (Lv2, Sord, Asio, Calf, ...).

As you said in Discord, you would like to keep it. I leave the comment here for discussion.

Comment on lines +68 to +69
- name: Build docs
run: cmake --build build --target doc
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

doc/CMakeLists.txt queries DOXYGEN_FOUND, but my system never sets this variable:

$ grep -i Doxygen CMakeCache.txt 
//Dot tool for use with Doxygen
DOXYGEN_DOT_EXECUTABLE:FILEPATH=DOXYGEN_DOT_EXECUTABLE-NOTFOUND
//Doxygen documentation generation tool (https://www.doxygen.nl)
DOXYGEN_EXECUTABLE:FILEPATH=/usr/bin/doxygen
//ADVANCED property for variable: DOXYGEN_DOT_EXECUTABLE
DOXYGEN_DOT_EXECUTABLE-ADVANCED:INTERNAL=1
//ADVANCED property for variable: DOXYGEN_EXECUTABLE
DOXYGEN_EXECUTABLE-ADVANCED:INTERNAL=1

The documentation says you should query Doxygen_FOUND, so I think we should query both.

Still, that does not work on my system. Might be out of scope for this PR though.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Testing this right now with this commit. I do think it is close enough to what this PR is touching, so I think it's worth it to try it here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It does error out, but it seems it is erroring even when the docs are not being built. I think it is being run even when the doc target is not among the specified.

image

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Managed to fix it on 069f3ab, through a dummy target that runs an echo and false. I'm just not sure if it works cleanly on pure windows, because afaik false isn't a command there. It should still error out, I guess, but the error will be more confusing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants