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

python3Packages.cryptography: 3.3.2 -> 3.4.2 #112402

Closed
wants to merge 1 commit into from

Conversation

primeos
Copy link
Member

@primeos primeos commented Feb 8, 2021

Backwards incompatible changes: Support for Python 2 has been removed.
Note: This isn't a problem for Nixpkgs though because
pythonPackages.cryptography is frozen at version 3.3.2.

Other important packaging changes: "Cryptography now incorporates Rust
code. Users building cryptography themselves will need to have the Rust
toolchain installed. Users who use an officially produced wheel will not
need to make any changes. The minimum supported Rust version is 1.45.0."
Note: Fetching Rust dependencies via Cargo during a Python build will
likely be a challenge for Nixpkgs. For Cryptography 3.4.x we can disable
the Rust support but that is only a temporarily supported workaround.

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-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)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

Backwards incompatible changes: Support for Python 2 has been removed.
Note: This isn't a problem for Nixpkgs because
pythonPackages.cryptography is frozen at version 3.3.2.

Other important packaging changes: "Cryptography now incorporates Rust
code. Users building cryptography themselves will need to have the Rust
toolchain installed. Users who use an officially produced wheel will not
need to make any changes. The minimum supported Rust version is 1.45.0."
Note: Fetching Rust dependencies via Cargo during a Python build will
likely be a challenge for Nixpkgs. For Cryptography 3.4.x we can disable
the Rust support but that is only a temporarily supported workaround.
@primeos primeos force-pushed the python-cryptography branch from a626116 to eec8ba6 Compare February 8, 2021 18:09
@primeos primeos changed the title python3Packages.cryptography: 3.3.2 -> 3.4.1 python3Packages.cryptography: 3.3.2 -> 3.4.2 Feb 8, 2021
@primeos
Copy link
Member Author

primeos commented Feb 8, 2021

@GrahamcOfBorg build python3Packages.cryptography

@@ -35,15 +34,13 @@ buildPythonPackage rec {
cffi
];

buildInputs = [ openssl ]
buildInputs = [ openssl setuptools-rust ]
Copy link
Contributor

Choose a reason for hiding this comment

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

if this isn't meant to be used at runtime, it should probably go into nativeBuildInputs

Copy link
Member Author

Choose a reason for hiding this comment

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

I was actually never sure about this for buildPythonPackage. Is our manual outdated in this regard?

Because e.g. https://nixos.org/manual/nixpkgs/unstable/#python ("15.20.1.2.2. Handling dependencies") states:

According to the manual, buildPythonPackage uses the arguments buildInputs and propagatedBuildInputs to specify dependencies. If something is exclusively a build-time dependency, then the dependency should be included in buildInputs, but if it is (also) a runtime dependency, then it should be added to propagatedBuildInputs. Test dependencies are considered build-time dependencies and passed to checkInputs.

(I'm also not sure if "According to the manual" in the manual is an accidental self-reference :D)

Copy link
Member Author

Choose a reason for hiding this comment

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

@jonringer @FRidh should setuptools-rust be part of nativeBuildInputs or buildInputs now?

Copy link
Member

Choose a reason for hiding this comment

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

nativeBuildInputs

@FRidh
Copy link
Member

FRidh commented Feb 8, 2021

About the rust code #83614.

@danieldk
Copy link
Contributor

danieldk commented Feb 9, 2021

@primeos I cherry-picked your commit in #112500 to test the proposed cargoSetupHook, hope you don't mind!

@primeos
Copy link
Member Author

primeos commented Feb 9, 2021

@danieldk no problem at all and thank you a lot for working on this!
Can I proceed with this PR as a temporary solution for the meantime or should I close it in favor of #112500 (or some other PR)?

@FRidh
Copy link
Member

FRidh commented Feb 9, 2021

I think #112500 is as good as done, so I suggest we go forward with that one.

@primeos
Copy link
Member Author

primeos commented Feb 9, 2021

Ok, awesome! Closing this PR then :)

@primeos primeos closed this Feb 9, 2021
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.

4 participants