[cmake] Small improvements and fixes #10078
Merged
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
CMake 3.5.1 suffers from a crash at configure time that happens on some Systems: http://cmake.org/Bug/view.php?id=16044 This is fixed in 3.5.2. Darwin requires CMake at least 3.4 or the usage of the patched version in depends.
fetzerch
added
Type: Fix
| endforeach() | ||
| endmacro() | ||
|
|
||
| # Get all propreties that cmake supports and convert them to a list. |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
|
@fetzerch thanks! Sorry, I did not notice the non-standard on review. |
Variable-length arrays are not part of C++ and only allowed in C99. While they are supported as a GCC extension (if -pedantic is not set), they cannot be used with MSVC. This commit replaces the VLA in ColorManager by a std::vector.
For example on Darwin platforms this can indicate a missing toolchain file.
In-source builds were broken due to a mistake in the export-files logic. Even though this commit resolves the problem, it is recommended building out-of-source.
fetzerch
force-pushed
the
cmake_fixes_warnings
branch
from
48993cc
to
bfb7355
Compare
|
@hudokkow: Fixed 2 typos, thanks. |
fetzerch
referenced
this pull request
Jul 7, 2016
The export-files target mirrors files from the source tree to the build tree (such as system/ userdata/ addons/) by doing a byte wise diff which is slow. Instead generate a CMake script that uses file(COPY) which only copies if the source is newer. Measurements on Linux machine (real time, cmake + make export-files): - first run (with cleaned page caches): 19+27s vs. 19+6s - consecutive run (with cleaned page caches): 12+35s vs. 12+3s - consecutive run: 6+18s vs. 6+0.3s Measurements on Windows machine (real time, cmake + make export-files): - first run: 43+65s vs. 52+8s - consecutive run: 19+62s vs. 29+1.5s
|
jenkins build and merge |
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.
See commit messages for details.
Pings:
@laurimyllari, @FernetMenta for the build fix 0789567. This ensures that ColorManager can be built on Windows, but also for Linux I think it's better not to rely on non-standard techniques. Would be great if you could test if I didn't break anything, I've just compile tested it here.
@Fneufneu: 48993cc fixes the in-source builds, as discussed in cc735b7#commitcomment-18124684
For the rest, maybe @hudokkow want's to have a quick look :)