From 85925b5be6ecd593023f45c12e27d6ae1a716fe1 Mon Sep 17 00:00:00 2001 From: Nathan Froyd Date: Tue, 14 May 2019 05:43:19 +0000 Subject: [PATCH] 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 | 99 ++++++++++++------- .../configure/test_toolchain_configure.py | 12 ++- 2 files changed, 72 insertions(+), 39 deletions(-) diff --git a/build/moz.configure/rust.configure b/build/moz.configure/rust.configure index be3703af94a4..9647cbc40e1f 100644 --- a/build/moz.configure/rust.configure +++ b/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) diff --git a/python/mozbuild/mozbuild/test/configure/test_toolchain_configure.py b/python/mozbuild/mozbuild/test/configure/test_toolchain_configure.py index 934541e01647..997a29626802 100755 --- a/python/mozbuild/mozbuild/test/configure/test_toolchain_configure.py +++ b/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), '')