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

Jump to bottom

libxml2: don't propagate Python bindings #77474

Merged
merged 1 commit into from Jan 13, 2020
Merged

libxml2: don't propagate Python bindings #77474

merged 1 commit into from Jan 13, 2020

Conversation

alyssais
Copy link
Member

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.

@ofborg ofborg bot requested a review from edolstra January 10, 2020 22:12
@FRidh FRidh self-assigned this Jan 11, 2020
@FRidh
Copy link
Member

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.

jtojnar
jtojnar reviewed Jan 11, 2020
View reviewed changes
pkgs/development/libraries/libxml2/default.nix Outdated
@@ -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" ];
Copy link
Contributor

Choose a reason for hiding this comment

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

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like that is still he case:

nixpkgs/pkgs/build-support/setup-hooks/multiple-outputs.sh

Line 182 in a8d1f32

local po_dirty="$outputBin $outputInclude $outputLib"

Copy link
Contributor

Choose a reason for hiding this comment

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

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

@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

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 libxml2 branch March 4, 2020 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonringer jonringer jonringer left review comments

@jtojnar jtojnar jtojnar approved these changes

@zimbatm zimbatm Awaiting requested review from zimbatm

@domenkozar domenkozar Awaiting requested review from domenkozar

@edolstra edolstra Awaiting requested review from edolstra

Assignees

@FRidh FRidh

Labels
6.topic: closure-size 10.rebuild-darwin: 501+ 10.rebuild-darwin: 5001+ 10.rebuild-darwin-stdenv 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+
Projects
Staging
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@alyssais @FRidh @jtojnar @jonringer