Bug 1547196 - remove rustup wrapper from `rustc` as well as `cargo`; r=glandium

Having `rustc` be `rustup`'s wrapper for `rustc` means that we may
silently honor `rustup`'s override mechanisms.  We noticed this first on
OS X, where we use the "real" `cargo` but `rustup`'s `rustc` wrapper,
and problems ensued when `cargo` thought it was using one version of
`rustc`, but actually wound up using something different.

It seems better to avoid silently interposing `rustup`'s toolchain
override mechanisms everywhere, rather than having to special-case OS
X.  So let's factor out a general mechanism for removing the wrappers
`rustup` provides and use that for both `rustc` and `cargo`.  The tests
need adjusting because we weren't triggering the unwrapping cases
before; we don't yet test the case where we really do need to unwrap.
That test can be left for a future patch.

Differential Revision: https://phabricator.services.mozilla.com/D29531

--HG--
extra : moz-landing-system : lando

build/moz.configure/rust.configure

@@ -10,56 +10,81 @@
 js_option(env='RUSTC', nargs=1, help='Path to the rust compiler')
 js_option(env='CARGO', nargs=1, help='Path to the Cargo package manager')
 
-rustc = check_prog('RUSTC', ['rustc'], paths=toolchain_search_path,
-                   input='RUSTC', allow_missing=True)
+rustc = check_prog('_RUSTC', ['rustc'], what='rustc',
+                   paths=toolchain_search_path, input='RUSTC',
+                   allow_missing=True)
 cargo = check_prog('_CARGO', ['cargo'], what='cargo',
                    paths=toolchain_search_path, input='CARGO',
                    allow_missing=True)
 
 
-@depends(cargo, host)
-@imports('subprocess')
-@imports(_from='__builtin__', _import='open')
-@imports('os')
-def cargo(cargo, host):
-    # The cargo executable can be either a rustup wrapper, or a real,
-    # plain, cargo. In the former case, on OSX, rustup sets
-    # DYLD_LIBRARY_PATH (at least until
-    # https://github.com/rust-lang/rustup.rs/pull/1752 is merged and shipped)
-    # and that can wreck havoc (see bug 1536486).
+@template
+def unwrap_rustup(prog, name):
+    # rustc and cargo can either be rustup wrappers, or they can be the actual,
+    # plain executables. For cargo, on OSX, rustup sets DYLD_LIBRARY_PATH (at
+    # least until https://github.com/rust-lang/rustup.rs/pull/1752 is merged
+    # and shipped) and that can wreak havoc (see bug 1536486). Similarly, for
+    # rustc, rustup silently honors toolchain overrides set by vendored crates
+    # (see bug 1547196).
     #
-    # So if we're in that situation, find the corresponding real plain
-    # cargo.
+    # In either case, we need to find the plain executables.
     #
-    # To achieve that, try to run `cargo +stable`. When it's the rustup
-    # wrapper, it either prints cargo's help and exits with status 0, or print
+    # To achieve that, try to run `PROG +stable`. When the rustup wrapper is in
+    # use, it either prints PROG's help and exits with status 0, or prints
     # an error message (error: toolchain 'stable' is not installed) and exits
-    # with status 1. When it's plain cargo, it exits with a different error
-    # message (error: no such subcommand: `+stable`), and exits with status
-    # 101.
-    if host.os == 'OSX':
-        with open(os.devnull, 'wb') as devnull:
-            retcode = subprocess.call(
-                [cargo, '+stable'], stdout=devnull, stderr=devnull)
-        if retcode != 101:
-            # We now proceed to find the real cargo. We're not sure `rustup`
-            # is in $PATH, but we know the cargo executable location, and that
-            # it /is/ rustup, so we can pass it `rustup` as argv[0], which
-            # will make it act as rustup.
-            # Note we could avoid the `cargo +stable` call above, but there's
-            # the possibility that there's a `cargo-which` command that would
-            # not fail with running `cargo which cargo` with a real cargo.
-            out = check_cmd_output('rustup', 'which', 'cargo',
-                                   executable=cargo).rstrip()
+    # with status 1. In the cargo case, when plain cargo is in use, it exits
+    # with a different error message (e.g. "error: no such subcommand:
+    # `+stable`"), and exits with status 101.
+    #
+    # Unfortunately, in the rustc case, when plain rustc is in use,
+    # `rustc +stable` will exit with status 1, complaining about a missing
+    # "+stable" file. We'll examine the error output to try and distinguish
+    # between failing rustup and failing rustc.
+    @depends(prog, dependable(name))
+    @imports('subprocess')
+    @imports(_from='__builtin__', _import='open')
+    @imports('os')
+    def unwrap(prog, name):
+        def from_rustup_which():
+            out = check_cmd_output('rustup', 'which', name,
+                                   executable=prog).rstrip()
             # If for some reason the above failed to return something, keep the
-            # cargo we found originally.
+            # PROG we found originally.
             if out:
-                cargo = out
-                log.info('Actually using %s', cargo)
-    return cargo
+                log.info('Actually using \'%s\'', out)
+                return out
+
+            log.info('No `rustup which` output, using \'%s\'', prog)
+            return prog
+
+        (retcode, stdout, stderr) = get_cmd_output(prog, '+stable')
+
+        if name == 'cargo' and retcode != 101:
+            prog = from_rustup_which()
+        elif name == 'rustc':
+            if retcode == 0:
+                prog = from_rustup_which()
+            elif "+stable" in stderr:
+                # PROG looks like plain `rustc`.
+                pass
+            else:
+                # Assume PROG looks like `rustup`. This case is a little weird,
+                # insofar as the user doesn't have the "stable" toolchain
+                # installed, but go ahead and unwrap anyway: the user might
+                # have only certain versions, beta, or nightly installed, and
+                # we'll catch invalid versions later.
+                prog = from_rustup_which()
+
+        return prog
+
+    return unwrap
+
+rustc = unwrap_rustup(rustc, 'rustc')
+cargo = unwrap_rustup(cargo, 'cargo')
 
 
 set_config('CARGO', cargo)
+set_config('RUSTC', rustc)
 
 
 @depends_if(rustc)

python/mozbuild/mozbuild/test/configure/test_toolchain_configure.py

@@ -1388,8 +1388,11 @@ class OpenBSDToolchainTest(BaseToolchainTest):
 
 
 @memoize
-def gen_invoke_cargo(version):
+def gen_invoke_cargo(version, rustup_wrapper=False):
     def invoke_cargo(stdin, args):
+        args = tuple(args)
+        if not rustup_wrapper and args == ('+stable',):
+            return (101, '', 'we are the real thing')
         if args == ('--version', '--verbose'):
             return 0, 'cargo %s\nrelease: %s' % (version, version), ''
         raise NotImplementedError('unsupported arguments')
@@ -1397,8 +1400,13 @@ def gen_invoke_cargo(version):
 
 
 @memoize
-def gen_invoke_rustc(version):
+def gen_invoke_rustc(version, rustup_wrapper=False):
     def invoke_rustc(stdin, args):
+        args = tuple(args)
+        # TODO: we don't have enough machinery set up to test the `rustup which`
+        # fallback yet.
+        if not rustup_wrapper and args == ('+stable',):
+            return (1, '', 'error: couldn\'t read +stable: No such file or directory')
         if args == ('--version', '--verbose'):
             return (0, 'rustc %s\nrelease: %s\nhost: x86_64-unknown-linux-gnu'
                        % (version, version), '')