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.pynacl: 0.3.0 -> 1.2.1 #34826

Merged
merged 1 commit into from Feb 11, 2018
Merged

pythonPackages.pynacl: 0.3.0 -> 1.2.1 #34826

merged 1 commit into from Feb 11, 2018

Conversation

va1entin
Copy link
Contributor

Motivation for this change

python-packages.nix contains version 0.3.0 of pynacl, which is very old.

Things done

Created a nix expression for pynacl at version 1.2.1 in it's own file, removed old 0.3.0 version of pynacl from python-packages.nix

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

@va1entin va1entin requested a review from FRidh as a code owner February 10, 2018 23:13
dotlambda
dotlambda requested changes Feb 10, 2018
View reviewed changes
Copy link
Member

@dotlambda dotlambda left a comment •
edited

Choose a reason for hiding this comment

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

Please squash the commits and please and don't call it init. It is an update.
Also, would you want to become the maintainer yourself?

pkgs/development/python-modules/pynacl/default.nix Outdated
buildPythonPackage rec {
pname = "pynacl";
version = "1.2.1";
name = "${pname}-${version}";
Copy link
Member

Choose a reason for hiding this comment

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

no name

pkgs/development/python-modules/pynacl/default.nix Outdated
sha256 = "0z9i1z4hjzmp23igyhvg131gikbrr947506lwfb3fayf0agwfv8f";
};

buildInputs = [ pytest coverage hypothesis ];
Copy link
Member

Choose a reason for hiding this comment

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

checkInputs

pkgs/development/python-modules/pynacl/default.nix Outdated
version = "1.2.1";
name = "${pname}-${version}";

src = fetchFromGitHub {
Copy link
Member

Choose a reason for hiding this comment

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

Why not fetchPypi?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mostly personal preference, honestly. I've seen many expressions in python-modules use fetchFromGitHub so I went with that.
Recent pynacl releases were about a day early on GitHub compared to PyPi.

pkgs/development/python-modules/pynacl/default.nix Outdated
src = fetchFromGitHub {
owner = "pyca";
repo = pname;
rev = "${version}";
Copy link
Member

Choose a reason for hiding this comment

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

rev = version;

@va1entin va1entin changed the title pythonPackages.pynacl: init at 1.2.1 pythonPackages.pynacl: 0.3.0 -> 1.2.1 Feb 10, 2018
dotlambda
dotlambda reviewed Feb 10, 2018
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.

Please don't forget to squash the commits

pkgs/development/python-modules/pynacl/default.nix Outdated
'';

meta = with stdenv.lib; {
aintainers = with maintainers; [ va1entin ];
Copy link
Member

Choose a reason for hiding this comment

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

maintainers

@grahamc
Copy link
Member

grahamc commented Feb 11, 2018

@GrahamcOfBorg build python3Packages.pynacl python2Packages.pynacl

@GrahamcOfBorg
Copy link

Failure on x86_64-darwin (full log)

Partial log (click to expand)

Package ‘python3.6-pynacl-1.2.1’ in /Users/graham/nix-borg/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-ndnd/pkgs/development/python-modules/pynacl/default.nix:23 is not supported on ‘x86_64-darwin’, refusing to evaluate.

a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowBroken = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowBroken = true; }
to ~/.config/nixpkgs/config.nix.

grahamc
grahamc reviewed Feb 11, 2018
View reviewed changes
pkgs/development/python-modules/pynacl/default.nix Outdated
description = "Python binding to the Networking and Cryptography (NaCl) library";
homepage = https://github.com/pyca/pynacl/;
license = licenses.asl20;
platforms = platforms.linux;
Copy link
Member

Choose a reason for hiding this comment

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

Does the new one actually not build on MacOS?

Copy link
Member

Choose a reason for hiding this comment

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

Actually, most python packages shouldn't specify platforms at all. It is set automatically.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry I forgot that, removed platforms now.

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Partial log (click to expand)

........................................................................ [ 95%]
........................................................................ [ 96%]
........................................................................ [ 98%]
........................................................................ [ 99%]
.                                                                        [ 99%]
tests/test_utils.py ...                                                  [100%]

=================== 4520 passed, 4 skipped in 26.52 seconds ====================
/nix/store/wsg13r657x6dhni2y7ylm8n4qcxdl49x-python3.6-pynacl-1.2.1
/nix/store/0sf4kwiq251m67h0x6djsqjzf0paax05-python2.7-pynacl-1.2.1

@GrahamcOfBorg
Copy link

Failure on aarch64-linux (full log)

Partial log (click to expand)

>               '%.2fms') % (runtime, self.settings.deadline)
            )
E           hypothesis.errors.DeadlineExceeded: Test took 2947.81ms, which exceeds the deadline of 1500.00ms

/nix/store/gck74417yqh4gydgcb5mzkmqqvgmzzsy-python3.6-hypothesis-3.27.0/lib/python3.6/site-packages/hypothesis/core.py:542: DeadlineExceeded
---------------------------------- Hypothesis ----------------------------------
Falsifying example: test_pwhash_str_and_verify(password='     ')
============== 1 failed, 4519 passed, 4 skipped in 219.87 seconds ==============
builder for '/nix/store/n2iwkdx3dblrm63qxp1lr1hkrlg98wbn-python3.6-pynacl-1.2.1.drv' failed with exit code 1
error: build of '/nix/store/5i8zv2nms5019rkdjparpxs6jpjiv7wm-python2.7-pynacl-1.2.1.drv', '/nix/store/n2iwkdx3dblrm63qxp1lr1hkrlg98wbn-python3.6-pynacl-1.2.1.drv' failed

@dotlambda
Copy link
Member

The deadline seems to be a known problem: pyca/pynacl#370
I guess it's best to remove that completely for Nixpkgs

dotlambda
dotlambda reviewed Feb 11, 2018
View reviewed changes
pkgs/development/python-modules/pynacl/default.nix Outdated
@@ -11,6 +11,12 @@ buildPythonPackage rec {
sha256 = "0z9i1z4hjzmp23igyhvg131gikbrr947506lwfb3fayf0agwfv8f";
};

#remove deadline from tests
preCheck = ''
Copy link
Member

Choose a reason for hiding this comment

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

prePatch
and please link to the upstream issue in a comment

@dotlambda
Copy link
Member

Please name the commit according to https://nixos.org/nixpkgs/manual/#chap-submitting-changes, i.e. exactly as this PR is named.

@FRidh FRidh merged commit 2721727 into NixOS:master Feb 11, 2018
@va1entin va1entin deleted the pynacl branch February 11, 2018 13:15
@va1entin va1entin mentioned this pull request Feb 11, 2018
Merged
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@grahamc grahamc grahamc left review comments

@dotlambda dotlambda dotlambda requested changes

@FRidh FRidh Awaiting requested review from FRidh

Assignees
No one assigned
Labels
6.topic: python 8.has: clean-up 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@va1entin @grahamc @GrahamcOfBorg @dotlambda @FRidh