-
Notifications
You must be signed in to change notification settings - Fork 69
cmake: make DaemonPlatform, Architecture, Compiler, etc. autonomous and reusable #1641
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
base: master
Are you sure you want to change the base?
Conversation
|
Why would we need a variable called NACL_PIE? There is no PIE for NaCl and never will be. |
Note that some of the facilities here duplicate CMake built-in functionality, or have more fully developed alternatives available from other projects. Effort is likely to be wasted in such cases:
DaemonArchitecture looks most likely to be useful. I think I saw a similar thing with SDL, but Daemon's looked more complete. |
Well maybe I'm wrong about that: the native_client scons build has a |
|
Then again, there is a comment explaining that it is not useful: Also there is another comment confirming that Saigo does not have PIC. |
007d0ff to
745bede
Compare
745bede to
d2b73ca
Compare
|
I removed the unused NaCl PIE thing. |
f01bf1f to
10ae04b
Compare
| # Collection of reusable CMake helpers written for the Dæmon engine and related | ||
| # projects. | ||
| ################################################################################ | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need this file? Are some of the files dependent on each other and so have to be included in a specific order?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to include them in specific order, and the Compiler stuff (and maybe other things) are writing constants (like compiler name and version) that is generated as source with SourceGenerator to be embedded in the binary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also Architecture requires System to be included first so it can select amd64 as NACL_ARCH on arm64 macOS, and things like that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we use this instead? https://cmake.org/cmake/help/v3.17/command/include_guard.html
Then each file can include its own dependencies and the user does not need to take care to call them in a special order.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You'll end up loading them anyway.
The only thing that seems to be interesting to do is to provide an option to not generate the source files with the various discovered things, for projects not caring about reporting the build environment to the user. For example we don't need such reporting in NaCl.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the latest version, if you only include Detection.cmake without having included SourceGenerator.cmake first (or the Platform.cmake entry point which is meant to include everything supported), then no source is generated when detecting the build environment.
5d31fb8 to
a9191be
Compare
We already renamed “unsupported” as “unknown” so it was just by luck we didn't stop the build (what we wanted when changing that).
a9191be to
cd64470
Compare
|
I landed my rewrite of This defines variables like A good example of usage of them is to avoid the ambiguity of Instead of It may also help to do things differently with MinGW on Linux and MinGW on Windows, when needed. It may also help in the future as cross-compiler evolves, for example the day Zig becomes suitable for us to cross-compile for macOS on Linux, and things like that. I also renamed |
3227432 to
5712ef4
Compare
87532c9 to
d5f8fbc
Compare
|
I also landed my mutualisation of Architecture/Compiler/System detection, all of them now using the same syntax in the C detection source file, and now using the same function to extract the data from the compilation log. |
d5f8fbc to
fc75870
Compare
|
There is now a unique |
|
Actually I noticed that we can report the native architecture built with Saigo (since Saigo does direct-to-nexe): |
3c3ea82 to
1ed9ae1
Compare
48ad53e to
3cbaa2e
Compare
3cbaa2e to
6eecf0a
Compare
Some revamp of the
DaemonPlatform/DaemonArchitecture/DaemonCompiler/DaemonBuildInfohelper code for CMake.I want this framework to be reusable for other projects, the first user outside of the engine may be the NaCl loader itself, and I may use it for NetRadiant as well at some point. So I made the code fully autonomous and contained in a single folder (so it can be copied and kept it sync easily).
It happens that the endian stuff we did in the
DaemonCompilercode usingEndian.hwas useless (it was not doing better than what theqprocessordetection.hcode does anyway), so I removed it.I also renamed
GAME_PIEtoNACL_PIEbecause it's an NaCl option, it was namedGAME_PIEbecause we only build games as NaCl, but for a self-contained framework we better name the NaCl optionNACL. For now this code setsGAME_PIEto theNACL_PIEvalue but once the game is modified to useNACL_PIEthe code setting theGAME_PIEalias will be deleted.