Skip to content
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.

Sign up for GitHub

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

Jump to bottom

cmake: add ctest support to setup hook #50323

Closed
wants to merge 1 commit into from
Closed

cmake: add ctest support to setup hook #50323

wants to merge 1 commit into from

Conversation

jtojnar
Copy link
Contributor

@jtojnar jtojnar commented Nov 13, 2018

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Fits CONTRIBUTING.md.

@matthewbauer
Copy link
Member

Is there any way to get CMake to generate Makefiles with a check target? That would be nice if possible.

@jtojnar
Copy link
Contributor Author

jtojnar commented Nov 13, 2018

Using ctest is probably a better idea, since it works across backends.

@GrahamcOfBorg
Copy link

Timed out, unknown build status on x86_64-linux (full log)

Attempted: cmake

Partial log (click to expand)

cannot build derivation '/nix/store/a1x4lyy63m2cli86w14kmrdi4rkn8mv2-flex-2.6.4.drv': 7 dependencies couldn't be built
cannot build derivation '/nix/store/vpwinnpnkvja99gcnc933dz70hgyxrqv-intltool-0.51.0.drv': 5 dependencies couldn't be built
cannot build derivation '/nix/store/a67wph067lb2vjphhlk70v4b0210vkfm-gnome-doc-utils-0.20.10.drv': 10 dependencies couldn't be built
cannot build derivation '/nix/store/vzqvj33z84fq2va84sgx5kr7qmfhj405-linux-pam-1.3.0.drv': 6 dependencies couldn't be built
cannot build derivation '/nix/store/0kqkmaja6aa75vf16gsdfnvv5kj0vqrq-shadow-4.6.drv': 11 dependencies couldn't be built
cannot build derivation '/nix/store/zhyq10n8lav4955n1q4qxq9axh9zqxwr-util-linux-2.32.1.drv': 5 dependencies couldn't be built
cannot build derivation '/nix/store/7skcr05xb68jfamcq6s8xy62mhc6412y-e2fsprogs-1.44.4.drv': 7 dependencies couldn't be built
cannot build derivation '/nix/store/h0qazb06wz1bjfsyc2xr9wv11l87xmxw-libarchive-3.3.2.drv': 12 dependencies couldn't be built
cannot build derivation '/nix/store/dqv1pwsbzh21bip96px6ax4wc4qdqhfx-cmake-3.12.1.drv': 11 dependencies couldn't be built
error: build of '/nix/store/dqv1pwsbzh21bip96px6ax4wc4qdqhfx-cmake-3.12.1.drv' failed

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: cmake

Partial log (click to expand)

shrinking /nix/store/w7dn07m9g6rihpa6xkwkv573q3r4pq6c-cmake-3.12.1/bin/cpack
shrinking /nix/store/w7dn07m9g6rihpa6xkwkv573q3r4pq6c-cmake-3.12.1/bin/ctest
shrinking /nix/store/w7dn07m9g6rihpa6xkwkv573q3r4pq6c-cmake-3.12.1/bin/cmake
strip is /nix/store/a6cc0wimnbsrhbgy5axg7lq81akfy32y-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/w7dn07m9g6rihpa6xkwkv573q3r4pq6c-cmake-3.12.1/bin
patching script interpreter paths in /nix/store/w7dn07m9g6rihpa6xkwkv573q3r4pq6c-cmake-3.12.1
/nix/store/w7dn07m9g6rihpa6xkwkv573q3r4pq6c-cmake-3.12.1/share/cmake-3.12/Modules/CPack.STGZ_Header.sh.in: interpreter directive changed from "/bin/sh" to "/nix/store/qvjs7hccx24r8cwhvm3h243jq0zw97r5-bash-4.4-p23/bin/sh"
/nix/store/w7dn07m9g6rihpa6xkwkv573q3r4pq6c-cmake-3.12.1/share/cmake-3.12/Modules/Squish4RunTestCase.sh: interpreter directive changed from "/bin/sh" to "/nix/store/qvjs7hccx24r8cwhvm3h243jq0zw97r5-bash-4.4-p23/bin/sh"
/nix/store/w7dn07m9g6rihpa6xkwkv573q3r4pq6c-cmake-3.12.1/share/cmake-3.12/Modules/SquishRunTestCase.sh: interpreter directive changed from "/bin/sh" to "/nix/store/qvjs7hccx24r8cwhvm3h243jq0zw97r5-bash-4.4-p23/bin/sh"
checking for references to /build in /nix/store/w7dn07m9g6rihpa6xkwkv573q3r4pq6c-cmake-3.12.1...

@GrahamcOfBorg
Copy link

Timed out, unknown build status on x86_64-darwin (full log)

Attempted: cmake

Partial log (click to expand)

cannot build derivation '/nix/store/3xmsnhizm25664h8klhz7bsj55i8pkjb-ps-1003.1-2008.drv': 2 dependencies couldn't be built
cannot build derivation '/nix/store/ix8db15d4h8zc4v4gdlk104cjqpbzg5k-xnu-osx-10.11.6.drv': 8 dependencies couldn't be built
cannot build derivation '/nix/store/i5bsx5zvy0s4vzf20lkgrzrmih7ihlir-IOKit-osx-10.11.6.drv': 2 dependencies couldn't be built
cannot build derivation '/nix/store/3mpq2g37f62gb2d3gi0fmnznz7qdvg1v-curl-7.62.0.drv': 7 dependencies couldn't be built
cannot build derivation '/nix/store/y6l9l1m90pqzxcfk390icp1bz8yw4kxi-configd-osx-10.8.5.drv': 8 dependencies couldn't be built
cannot build derivation '/nix/store/svc7m7gvdigj4xn7s7h8cn86aj56a6lz-python-2.7.15.drv': 8 dependencies couldn't be built
cannot build derivation '/nix/store/yfgd8gisn0jmqcp1fvwm72bsidjlh52v-libxml2-2.9.8.drv': 3 dependencies couldn't be built
cannot build derivation '/nix/store/7919r7n2qr6zvlvb1zhm5wdmy3lbdarw-libarchive-3.3.2.drv': 6 dependencies couldn't be built
cannot build derivation '/nix/store/rz86a4mpbf1kjdbrkjs459z56iz9dhck-cmake-3.12.1.drv': 9 dependencies couldn't be built
error: build of '/nix/store/rz86a4mpbf1kjdbrkjs459z56iz9dhck-cmake-3.12.1.drv' failed

@matthewbauer
Copy link
Member

matthewbauer commented Nov 15, 2018 •
edited

We really only support one backend in makefile, right now. According to this doc, "make test" should already work:

https://gitlab.kitware.com/cmake/community/wikis/doc/ctest/Testing-With-CTest#simple-testing

Do you have any specific packages where tests are not being run right now when they should be?

@matthewbauer
Copy link
Member

matthewbauer commented Nov 15, 2018 via email •
edited

@Mic92
Copy link
Member

Mic92 commented Nov 15, 2018 •
edited

Can you elaborate why ninja is a problem? Make was not really designed as a backend for generated Makefiles. On the other hand ninja is not supposed to be handwritten.

@matthewbauer
Copy link
Member

matthewbauer commented Nov 16, 2018 •
edited

Mostly for legacy reasons - Make has been around for long enough that it seems worth supporting. Ninja depends on the Python runtime without really offering any clear advantages to Make. The time difference appears to be almost negligible:

http://david.rothlis.net/ninja-benchmark/

Of course, it's still worth supporting Ninja as a setup hook. I just think we should use Makefiles when they're available to us for consistency.

@Mic92
Copy link
Member

Mic92 commented Nov 16, 2018 •
edited

  • Ninja does not depend on python. I have seen projects that ship a single static executable of it to bootstrap their project
  • this benchmark looks rather synthetic to me, when I add ninja to neovim the build takes 22.943 instead of 26.484, this is a 15 % percent speedup over the whole build.

@matthewbauer matthewbauer mentioned this pull request Nov 17, 2018
Merged
@matthewbauer
Copy link
Member

I suppose it's not hurting anything too much. My main rationale is that you should use as few dependencies as possible. Make is an implicit dependency on anything that use stdenv. Might as well use that when it is available.

It would definitely be nice if we could do some bootstrapping and avoid the build-time python dependency. Basic shell scripts could probably work? A static executable doesn't seem very cross platform though.

Anyway, I've opened a separate PR that adds the checPhase in the ninja setup hook:

#50527

@jtojnar
Copy link
Contributor Author

jtojnar commented Nov 18, 2018

We could also set checkTarget to test in cmake setup hook, but ctest looks cleaner to me and is analogous to what meson does.

@matthewbauer
Copy link
Member

We could also set checkTarget to test in cmake setup hook, but ctest looks cleaner to me and is analogous to what meson does.

I didn't realize that meson already had a check phase! That does change things but I am still partial to doing this all in Ninja. In setup.sh, build, install, & check are all handled by the same program (make). Ninja is supposed to be a make replacement so it should also handle the same phases. CMake, Meson, gyp, configure, etc. are all "generators" so they should only override the configure phase. This should hopefully lead to less phases in total and a clearer separation between build "generators" like CMake & Meson from "runners" like Make & Ninja.

orivej
orivej reviewed Nov 19, 2018
View reviewed changes
pkgs/development/tools/build-managers/cmake/setup-hook.sh
cmakeCheckPhase() {
runHook preCheck

ctest --extra-verbose
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe ctest --output-on-failure, without --extra-verbose?

@jtojnar
Copy link
Contributor Author

jtojnar commented Nov 20, 2018

Closing in favour of #50527

@jtojnar jtojnar closed this Nov 20, 2018
@jtojnar jtojnar deleted the cmake-ctest branch November 20, 2018 22:41
@jtojnar jtojnar mentioned this pull request Apr 17, 2020
Open
@knedlsepp knedlsepp mentioned this pull request Jul 27, 2021
Closed
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@orivej orivej orivej left review comments

Assignees
No one assigned
Labels
10.rebuild-darwin: 501+ 10.rebuild-darwin-stdenv 10.rebuild-linux: 501+
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@jtojnar @matthewbauer @GrahamcOfBorg @Mic92 @orivej