Bug 1547196 - remove rustup wrapper from `rustc` as well as `cargo`; r=glandium
authorNathan Froyd <froydnj@mozilla.com>
Tue, 14 May 2019 05:43:19 +0000
changeset 532623 c9f1a1db8fa2190a88c09eb624b321a07da6c3e1
parent 532622 c8fc7c4dc43c27a9c094b38de983425c1a06f43c
child 532624 dc20a50efcf1d6343019ab8cabe882698f2823fd
push id11270
push userrgurzau@mozilla.com
push dateWed, 15 May 2019 15:07:19 +0000
treeherdermozilla-beta@571bc76da583 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersglandium
bugs1547196
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
build/moz.configure/rust.configure
python/mozbuild/mozbuild/test/configure/test_toolchain_configure.py
--- a/build/moz.configure/rust.configure
+++ b/build/moz.configure/rust.configure
@@ -5,66 +5,91 @@
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 
 # Rust is required by `rust_compiler` below. We allow_missing here
 # to propagate failures to the better error message there.
 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)
 @checking('rustc version', lambda info: info.version)
 def rustc_info(rustc):
     out = check_cmd_output(rustc, '--version', '--verbose').splitlines()
     info = dict((s.strip() for s in line.split(':', 1)) for line in out[1:])
     return namespace(
--- a/python/mozbuild/mozbuild/test/configure/test_toolchain_configure.py
+++ b/python/mozbuild/mozbuild/test/configure/test_toolchain_configure.py
@@ -1383,27 +1383,35 @@ class OpenBSDToolchainTest(BaseToolchain
     def test_gcc(self):
         self.do_toolchain_test(self.PATHS, {
             'c_compiler': self.DEFAULT_GCC_RESULT,
             'cxx_compiler': self.DEFAULT_GXX_RESULT,
         })
 
 
 @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')
     return invoke_cargo
 
 
 @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), '')
         if args == ('--print', 'target-list'):
             # Raw list returned by rustc version 1.32, + ios, which somehow
             # don't appear in the default list.
             # https://github.com/rust-lang/rust/issues/36156
             rust_targets = [