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

bootstrap: on NixOS, also patchelf libexec/*, just like bin/*. #104938

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions src/bootstrap/bootstrap.py
Original file line number Diff line number Diff line change
Expand Up @@ -437,10 +437,19 @@ def download_toolchain(self):
filename = "cargo-{}-{}{}".format(rustc_channel, self.build,
tarball_suffix)
self._download_component_helper(filename, "cargo", tarball_suffix)
self.fix_bin_or_dylib("{}/bin/cargo".format(bin_root))

self.fix_bin_or_dylib("{}/bin/rustc".format(bin_root))
self.fix_bin_or_dylib("{}/bin/rustdoc".format(bin_root))
for bin_dir_name in ["bin", "libexec"]:
bin_dir = "{}/{}".format(bin_root, bin_dir_name)
for bin in os.listdir(bin_dir):
bin_file = os.path.join(bin_dir, bin)

# Skip script files, only actual executables need patching.
with open(bin_file, "rb", buffering=0) as f:
if f.read(2) == b"#!":
continue

self.fix_bin_or_dylib(bin_file)
Copy link
Member

Choose a reason for hiding this comment

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

I mildly remember seeing this exact pattern before. Can’t tell if it was here, or in nixpkgs. Either way something like this seems fine to me, despite the apparent brittleness in the way patchable binaries are being detected. It would seem to me that an ideal approach would filter all ELF-looking files within . with the execute permissions and try to patch those, since that’s pretty much the intent behind this code (and would transparently cover both DSOs and executables alike…)

Copy link
Member Author

Choose a reason for hiding this comment

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

To be clear, the shell filtering just removes patchelf printing a warning, it's otherwise a noop on non-ELF files, it's just an aesthetic fix for the output, I can remove the filtering entirely if that seems better.

(Not filtering would also mean easier to port the logic to Rust - which @jyn514 told me about, I had no idea this got duplicated)


lib_dir = "{}/lib".format(bin_root)
for lib in os.listdir(lib_dir):
if lib.endswith(".so"):
Expand Down