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

pythonPackages.ColanderAlchemy: fix build by pinning to colander 1.6.0 #59975

Merged
merged 1 commit into from Apr 26, 2019
Merged

pythonPackages.ColanderAlchemy: fix build by pinning to colander 1.6.0 #59975

merged 1 commit into from Apr 26, 2019

Conversation

risicle
Copy link
Contributor

@risicle risicle commented Apr 21, 2019

Motivation for this change

ColanderAlchemy doesn't seem to have been updated for colander 1.7.0, or at least its tests haven't been and we get a single, subtle test failure around null handling which I don't feel comfortable disabling. Issue filed upstream stefanofontanelli/ColanderAlchemy#107

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"
  • 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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@risicle risicle requested a review from FRidh as a code owner April 21, 2019 18:51
dotlambda
dotlambda requested changes Apr 24, 2019
View reviewed changes
Copy link
Member

@dotlambda dotlambda left a comment

Choose a reason for hiding this comment

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

We can't do this for Python packages as having mutiple versions of the same package in $PYTHONPATH is not supported.

@risicle
Copy link
Contributor Author

risicle commented Apr 24, 2019

😞 It's not ideal, but it's not as though we don't do this for any python packages: see the various versions of django or pytest.

@dotlambda
Copy link
Member

Pytest is special in that it's usually only in checkInputs, which is fine. With regards to Django, that's not ideal either.
However, I assume that depending on both, colander and ColanderAlchemy, is quite common.
I think we should set something like

meta.broken = lib.versionAtLeast colander.version "1.7.0";

and let people override the colander version themselves using packageOverrides.

@dotlambda
Copy link
Member

Alternatively, you could make a PR upstream that we can apply as a patch.

@risicle
pythonPackages.ColanderAlchemy: mark as broken for colander >1.6.0
b24445b 
ColanderAlchemy doesn't seem to have been updated for colander 1.7.0,
or at least its tests haven't been and we get a single, subtle test
failure around null handling which I don't feel comfortable skipping.
Issue filed upstream stefanofontanelli/ColanderAlchemy#107

users that really need this will have to feed in their own pinned
colander package.
@risicle
Copy link
Contributor Author

risicle commented Apr 25, 2019

Agree with the former solution - pushed.

@dotlambda dotlambda merged commit 9b69cf0 into NixOS:master Apr 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dotlambda dotlambda dotlambda requested changes

@FRidh FRidh Awaiting requested review from FRidh

Assignees
No one assigned
Labels
6.topic: python 10.rebuild-darwin: 0 10.rebuild-linux: 0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@risicle @dotlambda @GrahamcOfBorg