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.pybase64: init at 0.2.1 #34589

Merged
merged 2 commits into from Feb 5, 2018
Merged

Conversation

Ma27
Copy link
Member

@Ma27 Ma27 commented Feb 4, 2018

Motivation for this change

Adds the pybase64 package, a simple python-based libbase64 wrapper.
Please have a look at the commit messages for further reference.

Things done
  • 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.

@Ma27
Copy link
Member Author

Ma27 commented Feb 4, 2018

currently running nox-review pr 34589...

@Ma27
Copy link
Member Author

Ma27 commented Feb 4, 2018

ok, the package flexget fails for me, however this seems unrelated (I also tested this on the latest nixpkgs master without this change), I'd submit a separate PR to fix this.

@@ -3839,7 +3839,8 @@ in {
};
});

nose-parameterized = callPackage ../development/python-modules/nose-parameterized {};
nose-parameterized = builtins.trace "Warning: `nose-parameterized` is deprecated! Use `parameterized` instead."
Copy link
Member

Choose a reason for hiding this comment

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

lib.warn

@Ma27
Copy link
Member Author

Ma27 commented Feb 4, 2018

@FRidh done. The flexget package is still broken, however this is an issue on master and I already started investigating, however I need some after-FOSDEM sleep first :-)

# Tests require some python3-isms but code works without.
doCheck = isPy3k;

buildInputs = [ nose glibcLocales ];
Copy link
Member

Choose a reason for hiding this comment

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

checkInputs

`pythonPackages.parameterized` is the successor of `nose-parameterized`
as the authors of the module decided to support more testing frameworks
and stopped focusing on `noes` only. `nose-parameterized` is still
available in `pypi` with version `0.6.0`, but is officially deprecated.

However the renaming happened quite recently so it is possible that
there are still folks relying on `nose-parameterized`. Therefore I moved
the expression to provide a `pythonPackages.parameterized` derivation
and added a package override which builds `nose-parameterized` after
yielding a deprecation warning.
`pybase64` is a tiny wrapper for `libbase64` written in python.

/cc @ironpinguin
@Ma27
Copy link
Member Author

Ma27 commented Feb 5, 2018

done.

@FRidh FRidh merged commit 422f99b into NixOS:master Feb 5, 2018
@Ma27 Ma27 deleted the package-pybase64 branch February 5, 2018 13:25
@dotlambda
Copy link
Member

This causes a Hydra build to fail: https://hydra.nixos.org/build/68773666

@Ma27
Copy link
Member Author

Ma27 commented Feb 6, 2018

@dotlambda so, yielding warnings (in this case because of a deprecation by the package author) causes our builds to fail? oO

For me it seems weird to me to break a build because of warnings (in this case because of a deprecation warning due to a deprecated package which is something that happens from time to time), but @FRidh might know more about this.

However if there's no simple workaround to keep the deprecation warning without breaking the build I'd rather drop the warning for now as I think that patching the parameterized package in order to look like nose-parameterized is IMO not a better solution since we just patch around to satisfy some minor warnings.

@Ma27
Copy link
Member Author

Ma27 commented Feb 7, 2018

@FRidh FYI I currently don't know how to fix the flexget package as the tests during nix-build are fialing, however everything is perfectly fine when using virtualenv. As I don't know to much about Python I'll stop working on this for now...

@dotlambda
Copy link
Member

I think we might be able to fix the flexget package by pinning pytest to a certain version (probably 3.2.3 or 3.2.4). I'll try this and report whether it succeeds.

@Ma27
Copy link
Member Author

Ma27 commented Feb 7, 2018

👍 I think we could also create a package set using ‘pypi2nix’ as this would solve the dependency version problem

@dotlambda
Copy link
Member

I think the best solution would be to just disable tests completely. Even when up-/downgrading pytest, I have to disable about half of the tests.

@Ma27
Copy link
Member Author

Ma27 commented Feb 8, 2018 via email

@dotlambda
Copy link
Member

I don't think derivations generated by pypi2nix are fit for inclusion in Nixpkgs yet.

@Ma27
Copy link
Member Author

Ma27 commented Feb 8, 2018 via email

@FRidh
Copy link
Member

FRidh commented Feb 8, 2018

can you elaborate, please?

Security and testing. Give it some thought how these are affected by the use of pypi2nix.

such applicatiojs pin their dependencies explicitly

Which is bad practice. They should not pin on patch level, but unfortunately it is often done.

have to patch and test such applications whenever we bump a python library

That's why most of our packages have tests, so we can see this happen, and we typically see it happen! It just requires maintainers of these packages to actively maintain their package.

@Ma27
Copy link
Member Author

Ma27 commented Feb 8, 2018

Security and testing. Give it some thought how these are affected by the use of pypi2nix.

ACK, you're right.

That's why most of our packages have tests, so we can see this happen, and we typically see it happen! It just requires maintainers of these packages to actively maintain their package.

Okay then, I already started chasing and fixing some build/test issues of flexget, however I lack the inside knowledge in how Python packaging works in Nix, so I had some broken tests where I didn't know how to fix them.
Will push the branch, then we can see what we can do next ok? :-)

@dotlambda dotlambda mentioned this pull request Feb 9, 2018
8 tasks
@Ma27 Ma27 mentioned this pull request Mar 3, 2018
8 tasks
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