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

libxml2: don't propagate Python bindings #77474

Merged
merged 1 commit into from Jan 13, 2020
Merged

Conversation

@alyssais
Copy link
Member

@alyssais alyssais commented Jan 10, 2020

Motivation for this change

Extracted from #77149. Quoting myself there:

I suspect there will be packages that relied on the libxml2 Python
bindings being propagated, but don't know how I'd identify them. The
libxml2 change is too big for nix-review to handle. Any ideas?

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.
@FRidh
Copy link
Member

@FRidh FRidh commented Jan 11, 2020

This has existed since about 2015, 38313d5, which was when multiple outputs was introduced (#7701).

Nowadays the Python bindings are in python-packages.nix and packages should use those. Not all of them work that way though.

@@ -32,7 +32,7 @@ stdenv.mkDerivation rec {
outputs = [ "bin" "dev" "out" "man" "doc" ]
++ lib.optional pythonSupport "py"
++ lib.optional (enableStatic && enableShared) "static";
propagatedBuildOutputs = [ "out" "bin" ] ++ lib.optional pythonSupport "py";
propagatedBuildOutputs = [ "out" "bin" ];

This comment has been minimized.

@jtojnar

jtojnar Jan 11, 2020
Contributor

If the docs are correct, we should not need this line at all:

propagatedBuildOutputs of that package which by default contain $outputBin and $outputLib

This comment has been minimized.

@jtojnar

jtojnar Jan 11, 2020
Contributor

Looks like that is still he case:

local po_dirty="$outputBin $outputInclude $outputLib"

This comment has been minimized.

@jonringer

jonringer Jan 11, 2020
Contributor

i removed the whole line on master and was able to build a few hundred packages fine :)

@alyssais alyssais force-pushed the alyssais:libxml2 branch from 303edc4 to 2cc8412 Jan 13, 2020
@FRidh FRidh added this to WIP in Staging via automation Jan 13, 2020
Staging automation moved this from WIP to Ready Jan 13, 2020
@alyssais
Copy link
Member Author

@alyssais alyssais commented Jan 13, 2020

Okay then. Here we go!

I guess the staging process would pick up if this did cause catastrophic breakage anyway.

@alyssais alyssais merged commit 863fc65 into NixOS:staging Jan 13, 2020
Staging automation moved this from Ready to Done Jan 13, 2020
@alyssais alyssais deleted the alyssais:libxml2 branch Mar 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Staging
  
Done
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.