Update build instructions to better use CMake#1017
Conversation
|
|
||
| ```bash | ||
| $ make test | ||
| $ cmake --build "build" --config Release --target test |
There was a problem hiding this comment.
I have checked the new commands on both Linux and Windows and all seem to work fine (well, the installation failed expectedly with the access rights but looked correct up to that point) except for running tests. I guess it works fine with Makefile (or Ninja) build systems where there is a test target. But Visual Studio doesn't have such a target and fails for the new command.
However, it would fail with the old one just as well obviously, so no regress here.
I'm afraid progress could be made here only by using ctest instead.
There was a problem hiding this comment.
Because we use enable_testing() instead of include(CTest). I think it would be better to do the other, but it would go beyond just changing README.md. Perhaps as a separate change?
dmah42
left a comment
dmah42
left a comment
There was a problem hiding this comment.
Thanks for the changes. I agree this is much nicer.
README.md
Outdated
| $ cmake ../ | ||
| $ cmake -E make_directory "build" | ||
| # Generate build system files with cmake. | ||
| $ cmake -E chdir "build" cmake -DCMAKE_BUILD_TYPE=Release -DBENCHMARK_ENABLE_LTO=true ../ |
There was a problem hiding this comment.
ENABLE_LTO is considered an advanced feature which is why it's not in the initial instructions.
I think there's a separate discussion about whether it should just be defaulted to true :)
There was a problem hiding this comment.
Well, as a new user coming to the library I didn't have this impression reading the README.md. Instead, I was like: "well, why didn't you tell me this before I built it without the flag!" :)
So, perhaps we should change the README.md so that it explains what is "advanced" about it and why should one consider not using it. Or we should follow with adding it by default as you mentioned.
Anyway, I'm fine with removing it here if this is what you prefer.
There was a problem hiding this comment.
filed #1022 to consider changing the default. i think we should remove it from here until we can resolve that so the initial build instructions are as short as possible.
There was a problem hiding this comment.
OK, I will prepare an update for this pull request.
There was a problem hiding this comment.
I have fixed this in the new version.
|
|
||
| ```bash | ||
| $ make test | ||
| $ cmake --build "build" --config Release --target test |
adambadura
force-pushed
the
BetterUseOfCMakeCommandLine
branch
from
342e2c3 to
f38fb79
Compare
Build instructions needlessly referred to make when CMake offers a command-line interface to abstract away from the specific build system. Furthermore, CMake offers command-line "tool mode" which performs basic filesystem operations. While the syntax is a bit more verbose than Linux commands it is platform-independent. Now the commands can be copy-pasted on both Linux and Windows and will just work. Finally, the Release build type and link-time optimizations are included in initial commands. A natural flow for a new-comer is to read and execute the commands and only then learn that one has to go back and redo them again this time with proper parameters. Now instead the parameters are only explained later but present already in the initial commands.
adambadura
force-pushed
the
BetterUseOfCMakeCommandLine
branch
from
f38fb79 to
492c310
Compare
|
thanks! |
Build instructions needlessly referred to make when CMake offers a command-line interface to abstract away from the specific build system. Furthermore, CMake offers command-line "tool mode" which performs basic filesystem operations. While the syntax is a bit more verbose than Linux commands it is platform-independent. Now the commands can be copy-pasted on both Linux and Windows and will just work. Finally, the Release build type is included in initial commands. A natural flow for a new-comer is to read and execute the commands and only then learn that one has to go back and redo them again this time with proper parameters. Now instead the parameters are only explained later but present already in the initial commands.
2 participants
Build instructions needlessly referred to make when CMake offers a command-line interface to abstract away from the specific build system.
Furthermore, CMake offers command-line "tool mode" which performs basic filesystem operations. While the syntax is a bit more verbose than Linux commands it is platform-independent. Now the commands can be copy-pasted on both Linux and Windows and will just work.
Finally, the Release build type and link-time optimizations are included in initial commands. A natural flow for a new-comer is to read and execute the commands and only then learn that one has to go back and redo them again this time with proper parameters. Now instead the parameters are only explained later but present already in the initial commands.