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.

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

meson: set checkPhase in setup hook #33311

Merged
merged 2 commits into from Jan 7, 2018
Merged

Conversation

jtojnar
Copy link
Contributor

@jtojnar jtojnar commented Jan 2, 2018

Motivation for this change

Removing boilerplate.

Things done

Built json_glib and tests are run.

  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-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/)
  • Fits CONTRIBUTING.md.

@Mic92
Copy link
Member

Mic92 commented Jan 2, 2018

@GrahamcOfBorg build gnome-mpv

Copy link

@GrahamcOfBorg GrahamcOfBorg left a comment

Choose a reason for hiding this comment

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

Failure for system: x86_64-darwin

Package ‘gnome-mpv-0.13’ in /Users/graham/nix-borg/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-zoidberg/pkgs/applications/video/gnome-mpv/default.nix:31 is not supported on ‘x86_64-darwin’, refusing to evaluate.

a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowBroken = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowBroken = true; }
to ~/.config/nixpkgs/config.nix.

Copy link

@GrahamcOfBorg GrahamcOfBorg left a comment

Choose a reason for hiding this comment

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

Success for system: aarch64-linux

post-installation fixup
Wrapping program /nix/store/7f0nqf8n9spmx4db4kpzhwiaxy6mxc5d-gnome-mpv-0.13/bin/gnome-mpv
shrinking RPATHs of ELF executables and libraries in /nix/store/7f0nqf8n9spmx4db4kpzhwiaxy6mxc5d-gnome-mpv-0.13
shrinking /nix/store/7f0nqf8n9spmx4db4kpzhwiaxy6mxc5d-gnome-mpv-0.13/bin/.gnome-mpv-wrapped
gzipping man pages under /nix/store/7f0nqf8n9spmx4db4kpzhwiaxy6mxc5d-gnome-mpv-0.13/share/man/
strip is /nix/store/c6qj0j45xizkrx58i65j75a5ysmqhgrs-binutils-2.28.1/bin/strip
stripping (with command strip and flags -S) in /nix/store/7f0nqf8n9spmx4db4kpzhwiaxy6mxc5d-gnome-mpv-0.13/bin
patching script interpreter paths in /nix/store/7f0nqf8n9spmx4db4kpzhwiaxy6mxc5d-gnome-mpv-0.13
checking for references to /build in /nix/store/7f0nqf8n9spmx4db4kpzhwiaxy6mxc5d-gnome-mpv-0.13...
/nix/store/7f0nqf8n9spmx4db4kpzhwiaxy6mxc5d-gnome-mpv-0.13

Copy link

@GrahamcOfBorg GrahamcOfBorg left a comment

Choose a reason for hiding this comment

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

Success for system: x86_64-linux

post-installation fixup
Wrapping program /nix/store/xbi4513jww8d90mvnawhc675nqr9mq4h-gnome-mpv-0.13/bin/gnome-mpv
shrinking RPATHs of ELF executables and libraries in /nix/store/xbi4513jww8d90mvnawhc675nqr9mq4h-gnome-mpv-0.13
shrinking /nix/store/xbi4513jww8d90mvnawhc675nqr9mq4h-gnome-mpv-0.13/bin/.gnome-mpv-wrapped
gzipping man pages under /nix/store/xbi4513jww8d90mvnawhc675nqr9mq4h-gnome-mpv-0.13/share/man/
strip is /nix/store/wxn5gn8amxm1w0ikcx4gbs8a17wvss4j-binutils-2.28.1/bin/strip
stripping (with command strip and flags -S) in /nix/store/xbi4513jww8d90mvnawhc675nqr9mq4h-gnome-mpv-0.13/bin 
patching script interpreter paths in /nix/store/xbi4513jww8d90mvnawhc675nqr9mq4h-gnome-mpv-0.13
checking for references to /tmp/nix-build-gnome-mpv-0.13.drv-0 in /nix/store/xbi4513jww8d90mvnawhc675nqr9mq4h-gnome-mpv-0.13...
/nix/store/xbi4513jww8d90mvnawhc675nqr9mq4h-gnome-mpv-0.13

}

if [ -z "$dontUseMesonCheck" -a -z "$checkPhase" ]; then
setOutputFlags=
Copy link
Contributor

Choose a reason for hiding this comment

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

This line does not seem applicable to the check phase.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, I am sure why I copied it. It does not do anything even in the configurePhase.

runHook preCheck

echo "meson check flags: $mesonCheckFlags"
meson test $mesonCheckFlags
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure that mesonCheckFlags will be useful?

meson test is a more obvious label than meson check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It can be used for running tests in a wrapper (i.e. valgrind). Not sure if it is useful for packages.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As for the name, I wanted to be consistent with the rest of the Nix. There is doCheck attribute and checkPhase is also used in python derivations even though the invocation is ./setup.py test.

Copy link
Contributor

Choose a reason for hiding this comment

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

checkPhase is common to all Nixpkgs, and calling it testPhase just because the test command is … test would be confusing. However, flags are specific to tools and their variables contain the name of the tool, so passing flags to meson test in $mesonCheckFlags is surprising. (I'd rather just remove both the variable and the echo.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed. In the future we would probably want to use checkTarget like triton does but that would require changes to ninja setup hook as well.

@vcunat vcunat merged commit d75f95d into NixOS:staging Jan 7, 2018
vcunat added a commit that referenced this pull request Jan 7, 2018
@jtojnar jtojnar deleted the meson-checkphase branch January 29, 2018 01:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants