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

RFC: Force "dev" output in case of static builds #83793

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

RFC: Force "dev" output in case of static builds #83793

wants to merge 1 commit into from

Conversation

FRidh
Copy link
Member

@FRidh FRidh commented Mar 30, 2020 •
edited

Static builds propagate all buildInputs resulting in a
nix-support/propagated-build-inputs for nearly every package,
retaining references to the entire build-closure. To avoid this, one
needs to add "dev" outputs to the package.

This commit forces "dev" outputs in case of static builds. That is,
static builds will fail to build without a functioning "dev" output.

This will break packages that require the propagated-build-inputs file
in the main output. An example here are Python packages.

#83667

Note I don't immediately intend to merge this, its just to further the discussion.

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.

@FRidh FRidh requested review from tobim and Ericson2314 March 30, 2020 17:26
@FRidh FRidh mentioned this pull request Mar 30, 2020
Open
Ericson2314
Ericson2314 reviewed Mar 30, 2020
View reviewed changes
pkgs/top-level/static.nix Outdated Show resolved Hide resolved
pkgs/top-level/static.nix Outdated Show resolved Hide resolved
@FRidh
Force "dev" output in case of static builds
7aacf5f 
Static builds propagate all buildInputs resulting in a
`nix-support/propagated-build-inputs` for nearly every package,
retaining references to the entire build-closure. To avoid this, one
needs to add "dev" outputs to the package.

This commit forces "dev" outputs in case of static builds. That is,
static builds will fail to build without a functioning "dev" output.

This will break packages that require the `propagated-build-inputs` file
in the main output. An example here are Python packages.

NixOS#83667
@Ericson2314
Copy link
Member

Yes I think this is the natural and correct solution---if a big pill to swallow---for this. If we only do it when there are propagated deps, I think it will be safer, and something we can eventually do in all of nixpkgs.

This will break packages that require the propagated-build-inputs file
in the main output. An example here are Python packages.

This is interesting. We can in fact put the file in the out output and it will still work. In general, I am not aware of how much "building" happen in the average python derivation vs just downloading some source---maybe it makes sense the same (deduplicated, e.g. with content-addressed derivation outputs or just hard links inside) out would be referred to by different dev depending on what the chosen dependencies are.

@FRidh
Copy link
Member Author

FRidh commented Mar 30, 2020

How to handle cases where different packages propagate different versions of a library, e.g. openssl?

@Ericson2314
Copy link
Member

@FRidh I am not sure that is a new problem: if you depend on an exe that link openssl, you shouldn't be getting that package's dev output anyways. If you really do depend on two libraries that use two different versions, that is already a potential problem---and a fundamental static linking headache all things Nixpkgs aside.

@michaelpj
Copy link
Contributor

Perhaps a stupid suggestion, but: what would happen if we put the nix-support/propagated-build-inputs itself into a separate output? Then if foo depends on bar which depends on baz, a static build of foo could depend on bar.staticLibs which would propagate the appropriate things.

@Ericson2314
Copy link
Member

@michaelpj to be clear, that file does go in dev. I suppose we could have dev and staticLibs but I am not sure why we'd want that, to be honest. It is a good to be more precise on intent of course, but all of this (dev vs out, the propagation mechanism, etc.) is pretty loose on intent---I rather just not use propagated dependencies than some half-way thing.

@FRidh
Copy link
Member Author

FRidh commented Mar 31, 2020

This whole nix-support/propagated-build-inputs is not necessarily needed, because at eval time it is already known what dependencies are needed; the file just reduces unnecessary garbage collection.

@nh2
Copy link
Contributor

nh2 commented Apr 3, 2020

If we do this, I'd like to do a run of static-haskell-nix builds against it before we merge.

@stale

This comment has been minimized.

Sign in to view
@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Sep 30, 2020
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Oct 3, 2020
@stale
Copy link

stale bot commented Jun 7, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 7, 2021
@Ericson2314
Copy link
Member

It might be nice to still do this?

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 7, 2021
@stale
Copy link

stale bot commented Jan 9, 2022

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 9, 2022
@pwaller
Copy link
Contributor

pwaller commented Jul 17, 2022

Ping @FRidh, would be nice to have, if you have any appetite to pick this up.

@wegank wegank marked this pull request as draft March 20, 2024 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ericson2314 Ericson2314 Ericson2314 left review comments

@tobim tobim Awaiting requested review from tobim

Assignees
No one assigned
Labels
2.status: merge conflict 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 2.status: work-in-progress 6.topic: static 10.rebuild-darwin: 0 10.rebuild-linux: 0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@FRidh @Ericson2314 @michaelpj @nh2 @pwaller @ryantm