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

pythonPackages.pytest: fix .pytest_cache, again #45898

Merged
merged 2 commits into from Sep 4, 2018
Merged

Conversation

FRidh
Copy link
Member

@FRidh FRidh commented Sep 1, 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.

@@ -29,7 +29,7 @@ buildPythonPackage rec {

# Remove .pytest_cache when using py.test in a Nix build
setupHook = writeText "pytest-hook" ''
postFixupHooks+=(
postCheckHooks+=(
Copy link
Member

@Mic92 Mic92 Sep 1, 2018

Choose a reason for hiding this comment

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

At the moment we do not run this for most manual pytest check phases. Does this mean we have to add runHook to every python package with pytest now?

Copy link
Member

Choose a reason for hiding this comment

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

This is a bit boiler code one has to remember.

Copy link
Member Author

@FRidh FRidh Sep 1, 2018

Choose a reason for hiding this comment

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

Yes, we would have to, unless we get NixOS/rfcs#32. The other option is to set PYTEST_ADDOPTS again with nocacheprovider. This will, however, break packages that require it, so in those cases we would have to unset PYTEST_ADDOPTS.

Neither option is nice, but I prefer us to do what upstream documents, "the cache is created by default", even though it affects reproducibility and causes issues with python.buildEnv.

Copy link
Contributor

Choose a reason for hiding this comment

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

Before NixOS/rfcs#32 the non-boilerplate option to add this to a custom phase instead and put that phase into $preInstallPhases.

Copy link
Member Author

Choose a reason for hiding this comment

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

Because the installPhases are to early I added a preDistPhase.

It was reported that the 2nd solution wasn't working as expected because
it was ran in the wrong phase.

This commit creates a new phase, in between the installCheckPhase and distPhase.
@FRidh FRidh changed the base branch from staging-next to staging September 2, 2018 10:11
@FRidh FRidh merged commit b75ea62 into NixOS:staging Sep 4, 2018
@FRidh FRidh deleted the pytest branch September 4, 2018 17:18
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