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

build: use parallelization except on Windows #17

Merged
merged 1 commit into from
Nov 3, 2024

Conversation

cathyjf
Copy link
Contributor

@cathyjf cathyjf commented Nov 3, 2024

This is similar to LizardByte/Sunshine#3361, except now multithreaded
processing is not used on Windows.

Unlike LizardByte/Sunshine#3361, we're now enabling parallelization
for both dot graph generation and other doxygen processing tasks as well
(except on Windows).

To check for Windows, we use the macro CMAKE_HOST_WIN32 rather than
the more well-known WIN32, because what matters here is the host
platform, not the target platform. In most cases, these will be the
same, but the issue with parallelization is related to Windows as a
host, not a target. In other words, using Windows to build the
documentation for a macOS project should still be single-threaded.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Dependency update (updates to dependencies)
  • Documentation update (changes to documentation)
  • Repository update (changes to repository files, e.g. .github/...)

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated the in code docstring/documentation-blocks for new or existing methods/components

Copy link
Member

@ReenigneArcher ReenigneArcher left a comment

Choose a reason for hiding this comment

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

Thank you again! Just a couple of suggestions below.

I don't remember if there's a way to set a default value if the env value is empty or not. If NOT the env value also needs to be set before we run this command.

doxygen doxyconfig-Doxyfile

The build in readthedocs does not use cmake, since it would require installing all the project dependencies and such, wasting a lot of time.

CMakeLists.txt Outdated Show resolved Hide resolved
doxyconfig-Doxyfile Outdated Show resolved Hide resolved
doxyconfig-Doxyfile Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
@cathyjf cathyjf force-pushed the doxygen-multithreading branch 2 times, most recently from 8c37a35 to bf179c1 Compare November 3, 2024 22:29
This is similar to LizardByte/Sunshine#3361, except now multithreaded
processing is not used on Windows.

Unlike LizardByte/Sunshine#3361, we're now enabling parallelization
for both dot graph generation and other doxygen processing tasks as well
(except on Windows).

To check for Windows, we use the macro `CMAKE_HOST_WIN32` rather than
the more well-known `WIN32`, because what matters here is the host
platform, not the target platform. In most cases, these will be the
same, but the issue with parallelization is related to Windows as a
host, not a target. In other words, using Windows to build the
documentation for a macOS project should still be single-threaded.
@cathyjf cathyjf force-pushed the doxygen-multithreading branch from bf179c1 to af6a422 Compare November 3, 2024 22:30
Copy link

sonarqubecloud bot commented Nov 3, 2024

@cathyjf
Copy link
Contributor Author

cathyjf commented Nov 3, 2024

I don't remember if there's a way to set a default value if the env value is empty or not. If NOT the env value also needs to be set before we run this command.

I can't find any way to set a default value in the doxygen config file. However, if the environment variable is blank, that doesn't cause a syntax error. Instead, it's equivalent to setting a blank value for the variable. This in turn causes the default value for the variable to be used. This is fine for our case because it means that multithreading will be enabled by default, unless disabled by the cmake variable. Your readthedocs configuration suggests that Ubuntu will be used to generate the docs, which is not Windows, so using multithreading there should be fine.

@ReenigneArcher ReenigneArcher merged commit 9483349 into LizardByte:master Nov 3, 2024
8 checks passed
@cathyjf cathyjf deleted the doxygen-multithreading branch November 4, 2024 00:54
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.

2 participants