CMake modernization - platform detection#1345
Merged
hainest merged 20 commits intodyninst:cmake_modernizationfrom Jan 6, 2023
Merged
CMake modernization - platform detection#1345hainest merged 20 commits intodyninst:cmake_modernizationfrom
hainest merged 20 commits intodyninst:cmake_modernizationfrom
Conversation
These are equivalent to the existing bash, but more adaptable.
This is only used in the test suite.
We only support 64-bit ppc, so this is redundant and non-portable.
With the new variables, only one pass over the OS names is needed.
I previously thought it was only allowed on x86_64, but there is an old platform called i386-unknown-freebsd7.2.
Contributor
Author
kupsch
requested changes
Jan 6, 2023
Contributor
kupsch
left a comment
kupsch
left a comment
There was a problem hiding this comment.
looks good except one remaining use of PLATFORM that should be DYNINST_OS_i386, and two places that could be changed to DYNINST_OS_UNIX. After fixing at least the first one, I approve.
symtabAPI/CMakeLists.txt
Outdated
| if(PLATFORM MATCHES x86_64 OR PLATFORM MATCHES amd64) | ||
| if(DYNINST_ARCH_x86_64) | ||
| set(SRC_LIST ${SRC_LIST} src/emitElfStatic-x86.C src/relocationEntry-elf-x86.C) | ||
| elseif(PLATFORM MATCHES i386) |
stackwalk/CMakeLists.txt
Outdated
| src/linuxbsd-x86-swk.C | ||
| src/freebsd-x86-swk.C) | ||
| elseif(PLATFORM MATCHES linux OR PLATFORM MATCHES freebsd) | ||
| elseif(DYNINST_OS_Linux OR DYNINST_OS_FreeBSD) |
symtabAPI/CMakeLists.txt
Outdated
| src/SymtabReader.C) | ||
|
|
||
| if(PLATFORM MATCHES freebsd OR PLATFORM MATCHES linux) | ||
| if(DYNINST_OS_FreeBSD OR DYNINST_OS_Linux) |
Contributor
Author
|
I had actually fixed those, but evidently forgot to push them into this PR. Derp. Thanks for the close look! |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Closes #707
This definitely needs a careful examination. I am fairly confident I didn't break anything and it tests fine, but a tiny typo could easily cause no issues.
Eventually, we'll want to clean up the internal macro checks to simplify platform detection, but that's beyond the scope of this work.