-
-
Notifications
You must be signed in to change notification settings - Fork 353
cmake: (Draft) Various adaptations to build markdown docs #5643
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
Closed
Conversation
This file contains hidden or 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
…_custom_command()
…_custom_command() in same line
This allows to see the changes made in the PR even when on a fork. Otherwise, it is uses the default branch without the changes (except for the workflow file itself)
…o cmake variables
…s_${class_name}_md targets
The modules_list variable is updated with CACHE INTERNAL to append one module at a time when used. When using an IDE that automatically configures on save, this means that starting from a clean build directory (without cache), and wanting to check "WITH_GUI" to FALSE, will have the builds fail as the modules_list will contain gui targets, like g.gui.vdigit. The target ALL_MODULES would try to build g.gui.vdigit, but it doesn't appear anymore in the targets to build. It stays in the modules list, as it is stored in the CACHE.
…alue The modules_list variable is updated with CACHE INTERNAL to append one module at a time when used. When using an IDE that automatically configures on save, this means that starting from a clean build directory (without cache), and wanting to check "WITH_GUI" to FALSE, will have the builds fail as the modules_list will contain gui targets, like g.gui.vdigit. The target ALL_MODULES would try to build g.gui.vdigit, but it doesn't appear anymore in the targets to build. It stays in the modules list, as it is stored in the CACHE. The solution is to clear the cache variable, so using set(modules_list) defines a normal variable that is empty (and not having the cache variable previously hidden exposed)
… script builds for both extensions with the same call
…llowing Makefile builds
The command line invoked multiple times is already long enough
Variables should already be scoped because of the use of add_subdirectory
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.
I don't intend to have this merged, and I'll close this unmerged soon. I just wanted to post it here for reference. I made various attempts and exploration to replace the grass build in the docs workflow with a cmake build. It doesn't really work, as addons aren't ready for it, and from the point I was looking at (before ccache/mold linker), it was also slower than the autotools one (at least the way I set it, building html AND markdown even if it could've been avoided).
One of the ways I explored, knowing it wouldn't be robust, was to see if I could successfully populate the Platform.make.in makefile with the available CMake variables, and allow a cmake build to use unmodified addons (that expect the makefile structure). Obviously, it didn't really work, as the addons compilation were not really successful (some didn't show up red in the downloadable report though), but took about 10 seconds. Errors are now like "g.parser: not found", or the "scriptstrings" directory not found, way less frightening than earlier.
But still managed to make it to a point where it was the make call to sphinx's makefile that requested having a valid Platform.make file. That's when I stopped. As I'm sure I cannot hack around this one, to have grass core makefiles work with cmake in the same way I hoped addons would survive through (as they are not building grass).
In between all of this is various tries and improvements (or not) in cmake files. As I said earlier, I do not intend to have it merged, it is only a branched-off version of my exploration done this weekend. It isn't clean, nor minimal, nor rebased with perfect commit messages and squashed fixups. But I think some of this could be valuable for reference when needing to support the markdown parts of the build in cmake.
I think I can still extract some good fixes on the warnings shown when using recent cmake version (like 4.0.2). Or that ninja can't clean since some of the commands do not define the dependencies well (as they are written as POST_BUILD without BYPRODUCTS). Changing to have a target with a dependency on the OUTPUT form of the add_custom_command, and avoiding the extra "cmake -E make_directory" seems to not have the problem with ninja's cleaning (for the places I fixed).