Fix for bootstrapping on NixOS #39578

canndrew · 2017-02-06T08:33:46Z

NixOS puts Linux's dynamic loader in wierd place. Detect when we're on NixOS and patch the downloaded bootstrap executables appropriately.

rust-highfive · 2017-02-06T08:33:50Z

r? @aturon

(rust_highfive has picked a reviewer for you, use r? to override)

xen0n

LGTM from a Python programmer's perspective, although I can't test this. Take care of the portability and this should be good to merge!

xen0n · 2017-02-06T12:14:08Z

src/bootstrap/bootstrap.py

+
+        default_encoding = sys.getdefaultencoding()
+        try:
+            ostype = subprocess.check_output(['uname', '-s']).strip().decode(default_encoding)


Use platform.system() instead (with appropriate import added to the top). uname is not present on Windows and this gets executed before we know the OS type.

I was aware of platform.system() but I copied what was done here because I figured there must be a reason for it. This shouldn't break on windows because the fix_executable function just returns without doing anything if something goes wrong.

Well, yeah. Didn't see that WindowsError right below. Although IMO the uname calls are only there for consistency with the configure logic.

Could this deduplicate between here and the other location?

xen0n · 2017-02-06T12:15:46Z

src/bootstrap/bootstrap.py

@@ -89,7 +89,6 @@ def verify(path, sha_path, verbose):
               "    expected: {}".format(found, expected))
    return verified

-


Style cleanup is best done in a separate commit.

xen0n · 2017-02-06T12:16:40Z

src/bootstrap/bootstrap.py

+            return
+
+        # At this point we're pretty sure the user is running NixOS
+        print("Info: you seem to be running NixOS. Attempting to patch " + fname)


FWIW we're using all-lowercase messages in rustbuild, and this Info should be lower-cased too.

xen0n · 2017-02-06T12:17:07Z

src/bootstrap/bootstrap.py

+            interpreter = subprocess.check_output(["patchelf", "--print-interpreter", fname])
+            interpreter = interpreter.strip().decode(default_encoding)
+        except subprocess.CalledProcessError as e:
+            print("Warning: failed to call patchelf: %s" % e)


So should this Warning and some others below.

aturon · 2017-02-06T16:36:25Z

r? @brson

alexcrichton · 2017-02-06T16:38:21Z

@canndrew could you describe why this is necessary, and why we should do it as part of the build system? This seems like a lot of brittle logic easily broken for something that I'd naively consider to be a local problem, but I'm sure I'm not aware of all the details!

brson · 2017-02-06T23:24:47Z

I think this is a reasonable workaround, but it's a little weird - if you happen to be running on nix, it changes what it does to the outputs. In other words, one still can't build for NixOS generally, but if you happen to be building on Nix you get a Nix-compatible binary.

canndrew · 2017-02-08T09:24:41Z

@alexcrichton
The stage0 linux rust binaries that the build system downloads are hard-coded to look for the dynamic loader at /lib/ld-linux-$arch.so.2. NixOS is different to other linuxes and keeps its dynamic loader(s) under /nix/store, but it comes with a tool called patchelf for patching precompiled binaries to use a different dynamic loader path. The brittle logic shouldn't be a problem because the fix_executable function will just return without doing anything if it fails.

@brson
Yeah, this isn't really the most correct way to do this on NixOS. Ideally you'd write a Nix derivation which takes the dynamic loader as an input so it can find the path to patch. Just using the dynamic loader for sh is very much a hack. However I want to just be able to cd into a rust directory and build it without screwing around with the nix tools. This will let me do that. And it shouldn't interfere with any derivations people have written if it patches the path immediately after downloading the binary - the derivation can always just patch it again with another path.

alexcrichton · 2017-02-08T18:15:47Z

Thanks for the explanation @canndrew!

canndrew · 2017-02-09T07:00:04Z

It might be a good idea to see what other NixOS users think of this. Maybe @Ericson2314 has an opinion? I'm not sure who else uses NixOS.

Ericson2314 · 2017-02-09T17:41:25Z

Granted, I'm a little out of date on this stuff as I mainly just patch the pre-compiled binaries, and then that stopped working great because the Cargo ones moved. But I thought at least the official nixpkgs rustc package downloads the bootstrap binaries on rustbuild's behalf?

In any event, I think this stuff can go away once the cargo integration with out build systems + distributed cache stuff is figured out, and stdlib-aware Cargo is implemented. Then we can build rustc like any other binary very nicely. So hopefully this would be a temporary thing for less than the year 🤞.

edef1c · 2017-02-11T01:03:46Z

src/bootstrap/bootstrap.py

+        if ostype != "Linux":
+            return
+
+        if not os.path.exists("/nix/store"):


Any NixOS system has /etc/NIXOS (nixos-rebuild / the activation scripts check for its existence and refuse to run without it), so that's probably a better test.

Good point. Thanks.

@edef1c's recommendation was for /etc/NIXOS, and I believe one could have a NixOS installation without /etc/nixos.

Oh, I was wondering why he wrote it in uppercase. I didn't realize that file existed.

for the record, s/he/they/

nagisa · 2017-02-11T11:41:12Z

Instead of patching the rustc binary itself, change bootstrap logic to invoke rustc with ld.so bin/rustc. This can be done inside the rustc wrapper (at src/bootstrap/bin/rustc) for least churn (and similar wrapper could be made for cargo as well).

xen0n · 2017-02-12T03:42:55Z

@nagisa If this is implemented we'd want to change rustup as well, as one can rustup toolchain link the stage0 for whatever reason and expect things to work. Personally I just feel it's adding inconsistency as the places that end up being touched for the ld.so invocations are more than if only the binaries are patched and everything goes as usual.

canndrew · 2017-02-16T04:27:06Z

@nagisa What's the advantage of doing it like that? From my point of view, the precompiled rustc binaries are just broken for NixOS - they point to a file that doesn't exist. So why not just fix the problem directly?

solson · 2017-02-16T04:34:54Z

This is also the standard way to fix binaries on NixOS.

alexcrichton · 2017-02-17T14:55:19Z

@bors: r+

bors · 2017-02-17T14:55:20Z

📌 Commit 5e324bd has been approved by alexcrichton

bors · 2017-02-17T15:20:16Z

⌛ Testing commit 5e324bd with merge 536a900...

Fix for bootstrapping on NixOS NixOS puts Linux's dynamic loader in wierd place. Detect when we're on NixOS and patch the downloaded bootstrap executables appropriately.

bors · 2017-02-17T18:17:41Z

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 536a900 to master...

Fix for bootstrapping on NixOS

64c1ac8

NixOS puts Linux's dynamic loader in wierd place. Detect when we're on NixOS and patch the downloaded bootstrap executables appropriately.

rust-highfive assigned aturon Feb 6, 2017

xen0n suggested changes Feb 6, 2017

View reviewed changes

rust-highfive assigned brson and unassigned aturon Feb 6, 2017

edef1c reviewed Feb 11, 2017

View reviewed changes

canndrew added 2 commits February 11, 2017 13:01

Change test for being on NixOS

6e2631a

Change test for being on nixos again

e0a5b9d

solson mentioned this pull request Feb 13, 2017

Document necessary environment variables for use without rustup rust-lang/miri#106

Closed

Style fixups

5e324bd

bors merged commit 5e324bd into rust-lang:master Feb 17, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix for bootstrapping on NixOS #39578

Fix for bootstrapping on NixOS #39578

canndrew commented Feb 6, 2017

rust-highfive commented Feb 6, 2017

xen0n left a comment

xen0n Feb 6, 2017

canndrew Feb 8, 2017

xen0n Feb 8, 2017

alexcrichton Feb 16, 2017

xen0n Feb 6, 2017

xen0n Feb 6, 2017

alexcrichton Feb 16, 2017

xen0n Feb 6, 2017

aturon commented Feb 6, 2017

alexcrichton commented Feb 6, 2017

brson commented Feb 6, 2017

canndrew commented Feb 8, 2017

alexcrichton commented Feb 8, 2017

canndrew commented Feb 9, 2017

Ericson2314 commented Feb 9, 2017

edef1c Feb 11, 2017

canndrew Feb 11, 2017

8573 Feb 11, 2017 •

edited by eddyb

Loading

canndrew Feb 11, 2017

edef1c Feb 11, 2017

nagisa commented Feb 11, 2017 •

edited

Loading

xen0n commented Feb 12, 2017

canndrew commented Feb 16, 2017

solson commented Feb 16, 2017

alexcrichton commented Feb 17, 2017

bors commented Feb 17, 2017

bors commented Feb 17, 2017

bors commented Feb 17, 2017

		@@ -89,7 +89,6 @@ def verify(path, sha_path, verbose):
		" expected: {}".format(found, expected))
		return verified

Fix for bootstrapping on NixOS #39578

Fix for bootstrapping on NixOS #39578

Conversation

canndrew commented Feb 6, 2017

rust-highfive commented Feb 6, 2017

xen0n left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aturon commented Feb 6, 2017

alexcrichton commented Feb 6, 2017

brson commented Feb 6, 2017

canndrew commented Feb 8, 2017

alexcrichton commented Feb 8, 2017

canndrew commented Feb 9, 2017

Ericson2314 commented Feb 9, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

8573 Feb 11, 2017 • edited by eddyb Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nagisa commented Feb 11, 2017 • edited Loading

xen0n commented Feb 12, 2017

canndrew commented Feb 16, 2017

solson commented Feb 16, 2017

alexcrichton commented Feb 17, 2017

bors commented Feb 17, 2017

bors commented Feb 17, 2017

bors commented Feb 17, 2017

8573 Feb 11, 2017 •

edited by eddyb

Loading

nagisa commented Feb 11, 2017 •

edited

Loading