bootstrap.py: patch RPATH on NixOS to handle the new zlib dependency. #74441
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rust-highfive
added
the
S-waiting-on-review
This was referenced Jul 17, 2020
Merged
davidtwco
approved these changes
Jul 17, 2020
nagisa
reviewed
Jul 17, 2020
| try: | ||
| subprocess.check_output([ | ||
| "nix-build", "<nixpkgs>", | ||
| "-A", dep, |
There was a problem hiding this comment.
Would it make sense to try and build all of the deps with a single command?
There was a problem hiding this comment.
Not sure how to do that and keep the symlinks.
There was a problem hiding this comment.
The symlinks won’t be named as well as they are right now, but:
$ nix-build '<nixpkgs>' -A zlib -A stdenv.cc.bintools -o foo/.nixdeps
$ ls -alh foo/
total 0
drwxr-xr-x 2 nagisa users 80 Jul 17 17:43 .
drwxrwxrwt 21 root root 560 Jul 17 17:43 ..
lrwxrwxrwx 1 nagisa users 55 Jul 17 17:43 .nixdeps -> /nix/store/ml4ipdnvc7pr07dr6i35831f7ffxny0k-zlib-1.2.11
lrwxrwxrwx 1 nagisa users 67 Jul 17 17:43 .nixdeps-2 -> /nix/store/wy6v1s2y8rvxcy98l2yvxqj280cq9wgc-binutils-wrapper-2.31.1
nagisa
reviewed
Jul 17, 2020
| if fname.endswith(".so"): | ||
| # Dynamic library, patch RPATH to point to system dependencies. | ||
| dylib_deps = ["zlib"] | ||
| rpath_entries = [ |
There was a problem hiding this comment.
Hm… shouldn't we just append the rpath?
There was a problem hiding this comment.
That would require a command to read it, which... I can do it if you want, it's just uglier (and I've gotten used to hardcoding the $ORIGIN/../lib part in my patchelf commands).
There was a problem hiding this comment.
yeah, it is just not clear to me that $ORIGIN/../lib is sufficient and that there aren’t libraries/binaries that implicitly gain other rpath entries as part of their compilation process. I’m fine with it if it is, I guess.
There was a problem hiding this comment.
For the record, NixOS rustup also hardcodes $ORIGIN/../lib:, so I think we're fine.
|
@bors r+ with or without comments addressed. |
|
📌 Commit b5076fb has been approved by |
bors
added
S-waiting-on-bors
Manishearth
added a commit
to Manishearth/rust
that referenced
this pull request
Jul 18, 2020
bootstrap.py: patch RPATH on NixOS to handle the new zlib dependency. This is a stop-gap until rust-lang#74420 is resolved (assuming we'll patch beta to statically link zlib). However, I've been meaning to rewrite the NixOS support we have in `bootstrap.py` for a while now, and had to in order to cleanly add zlib as a dependency (the second commit is a relatively small delta in functionality, compared to the first). Previously, we would extract the `ld-linux.so` path from the output of `ldd /run/current-system/sw/bin/sh`, which assumes a lot. On top of that we didn't use any symlinks, which meant if the user ran GC (`nix-collect-garbage`), e.g. after updating their system, their `stage0` binaries would suddenly be broken (i.e. referring to files that no longer exist). We were also using `patchelf` directly, assuming it can be found in `$PATH` (which is not necessarily true). My new approach relies on using `nix-build` to get the following "derivations" (packages, more or less): * `stdenv.cc.bintools`, which has a `nix-support/dynamic-linker` file containing the path to `ld-linux.so` * reading this file is [the canonical way to run `patchelf --set-interpreter`](https://github.com/NixOS/nixpkgs/search?l=Nix&q=%22--set-interpreter+%24%28cat+%24NIX_CC%2Fnix-support%2Fdynamic-linker%29%22) * `patchelf` (so that the user doesn't need to have it installed) * `zlib`, for the `libz.so` dependency of `libLLVM-*.so` (until rust-lang#74420 is resolved, presumably) This is closer to how software is built on Nix, but I've tried to keep it as simple as possible (and not add e.g. a `stage0.nix` file). Symlinks to each of those dependencies are kept in `stage0/.nix-deps`, which prevents GC from invalidating `stage0` binaries. r? @nagisa cc @Mark-Simulacrum @oli-obk @davidtwco
Manishearth
added a commit
to Manishearth/rust
that referenced
this pull request
Jul 18, 2020
bootstrap.py: patch RPATH on NixOS to handle the new zlib dependency. This is a stop-gap until rust-lang#74420 is resolved (assuming we'll patch beta to statically link zlib). However, I've been meaning to rewrite the NixOS support we have in `bootstrap.py` for a while now, and had to in order to cleanly add zlib as a dependency (the second commit is a relatively small delta in functionality, compared to the first). Previously, we would extract the `ld-linux.so` path from the output of `ldd /run/current-system/sw/bin/sh`, which assumes a lot. On top of that we didn't use any symlinks, which meant if the user ran GC (`nix-collect-garbage`), e.g. after updating their system, their `stage0` binaries would suddenly be broken (i.e. referring to files that no longer exist). We were also using `patchelf` directly, assuming it can be found in `$PATH` (which is not necessarily true). My new approach relies on using `nix-build` to get the following "derivations" (packages, more or less): * `stdenv.cc.bintools`, which has a `nix-support/dynamic-linker` file containing the path to `ld-linux.so` * reading this file is [the canonical way to run `patchelf --set-interpreter`](https://github.com/NixOS/nixpkgs/search?l=Nix&q=%22--set-interpreter+%24%28cat+%24NIX_CC%2Fnix-support%2Fdynamic-linker%29%22) * `patchelf` (so that the user doesn't need to have it installed) * `zlib`, for the `libz.so` dependency of `libLLVM-*.so` (until rust-lang#74420 is resolved, presumably) This is closer to how software is built on Nix, but I've tried to keep it as simple as possible (and not add e.g. a `stage0.nix` file). Symlinks to each of those dependencies are kept in `stage0/.nix-deps`, which prevents GC from invalidating `stage0` binaries. r? @nagisa cc @Mark-Simulacrum @oli-obk @davidtwco
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jul 18, 2020
…arth Rollup of 11 pull requests Successful merges: - rust-lang#72414 ( Add lazy initialization primitives to std) - rust-lang#74069 (Compare tagged/niche-filling layout and pick the best one) - rust-lang#74418 (ci: Set `shell: bash` as a default, remove duplicates) - rust-lang#74441 (bootstrap.py: patch RPATH on NixOS to handle the new zlib dependency.) - rust-lang#74444 (Add regression test for rust-lang#69414) - rust-lang#74448 (improper_ctypes_definitions: allow `Box`) - rust-lang#74449 (Test codegen of compare_exchange operations) - rust-lang#74450 (Fix `Safety` docs for `from_raw_parts_mut`) - rust-lang#74453 (Use intra-doc links in `str` and `BTreeSet`) - rust-lang#74457 (rustbuild: drop tool::should_install) - rust-lang#74464 (Use pathdiff crate) Failed merges: r? @ghost
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a stop-gap until #74420 is resolved (assuming we'll patch beta to statically link zlib).
However, I've been meaning to rewrite the NixOS support we have in
bootstrap.pyfor a while now, and had to in order to cleanly add zlib as a dependency (the second commit is a relatively small delta in functionality, compared to the first).Previously, we would extract the
ld-linux.sopath from the output ofldd /run/current-system/sw/bin/sh, which assumes a lot. On top of that we didn't use any symlinks, which meant if the user ran GC (nix-collect-garbage), e.g. after updating their system, theirstage0binaries would suddenly be broken (i.e. referring to files that no longer exist).We were also using
patchelfdirectly, assuming it can be found in$PATH(which is not necessarily true).My new approach relies on using
nix-buildto get the following "derivations" (packages, more or less):stdenv.cc.bintools, which has anix-support/dynamic-linkerfile containing the path told-linux.sopatchelf --set-interpreterpatchelf(so that the user doesn't need to have it installed)zlib, for thelibz.sodependency oflibLLVM-*.so(until rustc is now dynamically linked to zlib #74420 is resolved, presumably)This is closer to how software is built on Nix, but I've tried to keep it as simple as possible (and not add e.g. a
stage0.nixfile).Symlinks to each of those dependencies are kept in
stage0/.nix-deps, which prevents GC from invalidatingstage0binaries.r? @nagisa cc @Mark-Simulacrum @oli-obk @davidtwco