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

Add checkPhase for pipenv #73349

Merged
merged 3 commits into from Nov 13, 2019
Merged

Add checkPhase for pipenv #73349

merged 3 commits into from Nov 13, 2019

Conversation

zupo
Copy link
Contributor

@zupo zupo commented Nov 13, 2019

Motivation for this change

This is to prevent regressions such as #73254
by using pipenv to install a simple Python package, thus testing
that pipenv was built correctly.

I tested the patch locally against nixpgs master (pipenv built successfully) and against nixpgs 19.09 (pipenv build failed, as expected, with ModuleNotFoundError: No module named 'pip._internal.main').

Refs #73254

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nix-review --run "nix-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)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @FRidh

Many thanks to @infinisil and @domenkozar for hand-holding!

This is to prevent regressions such as #73254
by using pipenv to install a simple Python package, thus testing
that pipenv was built correctly.

Many thanks to @infinisil and @domenkozar for hand-holding!
Copy link
Member

@FRidh FRidh left a comment

Choose a reason for hiding this comment

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

Where will this test install to? We do not want it to create any additional content in $out.

pkgs/development/tools/pipenv/default.nix Outdated Show resolved Hide resolved
@zupo
Copy link
Contributor Author

zupo commented Nov 13, 2019

Where will this test install to? We do not want it to create any additional content in $out.

It seems $out does not get polluted. Which, according to my very basic understanding, is expected, otherwise most packages' checkPhase tests would potentially pollute their $out.

[nix-shell:/nix/store/l8q8pp62v2a87qzzp70ddsl07i6pmzb6-pipenv-2018.11.26]$ grep -ri joke ./

[nix-shell:/nix/store/l8q8pp62v2a87qzzp70ddsl07i6pmzb6-pipenv-2018.11.26]$ ls lib/python3.7/site-packages/
pipenv	pipenv-2018.11.26.dist-info

[nix-shell:/nix/store/l8q8pp62v2a87qzzp70ddsl07i6pmzb6-pipenv-2018.11.26]$ ls bin/
pipenv	pipenv-resolver

@infinisil
Copy link
Member

It seems that pipenv wants to write to the directory it gets in some cases, but ${wheel.src} is a path in /nix/store, which isn't writable. To get around this:

cp -r --no-preserve=mode ${wheel.src} $PWD/wheel-src
$out/bin/pipenv install $PWD/wheel-src

We need an egg to install to test that pipenv works. Instead of
downloading & installing pyjokes, let's use wheel since we already
have it.

Refs #73349 (comment)
@infinisil
Copy link
Member

@GrahamcOfBorg build pipenv

@zupo zupo requested a review from FRidh November 13, 2019 21:25
@FRidh FRidh merged commit 1c40651 into NixOS:master Nov 13, 2019
@FRidh
Copy link
Member

FRidh commented Nov 13, 2019

ugh should have squashed these commits...

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

4 participants