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

python37Packages.pyrsistent: 0.15.2 -> 0.15.4 #69364

Merged
merged 1 commit into from Oct 2, 2019

Conversation

vlaci
Copy link
Contributor

@vlaci vlaci commented Sep 24, 2019

Motivation for this change

Some tests are broken on 0.15.2 if used with pytest 5+ as discussed
in #64145. The issue is fixed in tobgu/pyrsistent#175 and released in
0.15.4.
As this package is a required dependency to build poetry this fix takes use one step closer to be able to build poetry on unstable, however it is not enough as poetry now fails to build with the following error:

ERROR: Invalid requirement: ''

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 nix-review --run "nix-review wip" NOTE poetry and python27Packages.poetry still fails to build
  • 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 @desiderius

Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

nix-review passes on NixOS (poetry broken on master)
diff LGTM

[5 built (2 failed), 95 copied (42.0 MiB), 10.0 MiB DL]
error: build of '/nix/store/w29gj5b8lww70q0yy3y7izb78lmdhv8a-env.drv' failed
https://github.com/NixOS/nixpkgs/pull/69364
2 package failed to build:
poetry python27Packages.poetry

2 package were build:
python27Packages.pyrsistent python37Packages.pyrsistent

pkgs/development/python-modules/pyrsistent/default.nix Outdated Show resolved Hide resolved
Some tests are broken on 0.15.2 if used with pytest 5+ as discussed
in NixOS#64145. The issue is fixed in tobgu/pyrsistent#175 and released in
0.15.4.
@d-goldin
Copy link
Contributor

d-goldin commented Oct 2, 2019

#68361 for visibility.

@jonringer
Copy link
Contributor

@GrahamcOfBorg build poetry python27Packages.pyrsistent python37Packages.pyrsistent

Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

nix-review passes on NixOS (python2Packages.poetry is broken on master)
poetry seems to work

[5 built (1 failed), 24 copied (4.7 MiB), 1.2 MiB DL]
error: build of '/nix/store/8k0wnh3dig0j3plss8s8pqv74qjyx1sd-env.drv' failed
https://github.com/NixOS/nixpkgs/pull/69364
1 package failed to build:
python27Packages.poetry

3 package were build:
poetry python27Packages.pyrsistent python37Packages.pyrsistent

@jonringer jonringer merged commit 9d9aa60 into NixOS:master Oct 2, 2019
@jonringer
Copy link
Contributor

python2Packages.poetry build being fixed #70225

@jonringer jonringer added the 9.needs: port to stable A PR needs a backport to the stable release. label Oct 2, 2019
@jonringer
Copy link
Contributor

technically not from this PR, but build backported and fixed in 20e214b

@vlaci
Copy link
Contributor Author

vlaci commented Oct 2, 2019

The fix has been integrated to master: 9d9aa60

There could be a potential conflict with release-19.09 though

@vlaci vlaci deleted the pyrsistent-0154 branch October 2, 2019 07:12
@jonringer
Copy link
Contributor

there's a large overlap between the two commits, essentially one uses pytest_4 and other pytest_5.

@jonringer
Copy link
Contributor

after the rebase, this PR was essentially reduced down to the substituteInPlace command

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

3 participants