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
Some minor CMake improvements #2080
Closed
Closed
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
trunk is a term from version control systems and has nothing to do with CMake. Now the variable PROJECT_NAME defined by CMake and PACKAGE_NAME are the same, therefore replaced PACKAGE_NAME with PROJECT_NAME
see note in project command documentation https://cmake.org/cmake/help/v3.3/command/project.html
as the comment and https://crascit.com/2016/04/09/using-ccache-with-cmake/ suggest, setting CCache should be done before project command, because then CMake checks if the compiler can be invoked through CCache
append to it to allow setting it from outside
couldn't find DEBUG/RELEASE_MAIN_OUTPUT_PATH anywhere else
Excerpt from CMake documentation: The if command was written very early in CMake’s history, predating the ${} variable evaluation syntax, and for convenience evaluates variables named by its arguments as shown in the above signatures. Note that normal variable evaluation with ${} applies before the if command even receives the arguments. Therefore code like: set(var1 OFF) set(var2 "var1") if(${var2}) appears to the if command as: if(var1) and is evaluated according to the if(<variable>) case documented above. The result is OFF which is false. However, if we remove the ${} from the example then the command sees: if(var2) which is true because var2 is defined to “var1” which is not a false constant.
TODO: there is still something wrong with indentation, there are some IFs on the same level without a ENDIF in between
BuildAll.bat was removed with commit daee73a
wasn't touched for 5 years, probably not needed anymore
this should work for all generators on all systems
project was renamed in commit 98e3027
Merged. |
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.
Thank you for creating a pull request to contribute to FreeCAD! To ease integration, please confirm the following:
git pull --rebase upstream master
./bin/FreeCAD --run-test 0
issue #<id>
orfixes #<id>
where<id>
is the associated MantisBT issue id if one existsAnd please remember to update the Wiki with the features added or changed once this PR is merged.
Note: If you don't have wiki access, then please mention your contribution on the 0.19 Changelog Forum Thread.
Did some minor CMake improvements
structuring of project command and policies, project name, removing unused code, indentation