rustup installs rustc+cargo to ~/.cargo/bin by default, so configure should look there to avoid the annoying case where someone installed rust+cargo (possibly via `mach bootstrap`) but forgot to add ~/.cargo/bin to their PATH. Presumably most people that are doing actual Rust development will have done so, but people that just want to build Firefox may not, so we should make the build work for the common case.

Comment on attachment 8904720 [details] bug 1396993 - Look in ~/.cargo/bin for rustc+cargo even they're not in PATH. https://reviewboard.mozilla.org/r/176512/#review181578 ::: build/moz.configure/rust.configure:13 (Diff revision 1) > +def rustup_bin_path(): > + return [expanduser('~/.cargo/bin')] > + > # Rust is required by `rust_compiler` below. We allow_missing here > # to propagate failures to the better error message there. > -rustc = check_prog('RUSTC', ['rustc'], allow_missing=True) > +rustc = check_prog('RUSTC', ['rustc'], allow_missing=True, paths=rustup_bin_path) This will only look in ~/.cargo/bin and nowhere else.

Comment on attachment 8904720 [details] bug 1396993 - Look in ~/.cargo/bin for rustc+cargo even they're not in PATH. https://reviewboard.mozilla.org/r/176512/#review181590 ::: build/moz.configure/rust.configure:9 (Diff revision 1) > # License, v. 2.0. If a copy of the MPL was not distributed with this > # file, You can obtain one at http://mozilla.org/MPL/2.0/. > > +@imports(_from='os.path', _import='expanduser') > +def rustup_bin_path(): > + return [expanduser('~/.cargo/bin')] Does this do the correct thing in the msys environment we build under on windows? Also, it looks like `rustup` checks both `CARGO_HOME` and `RUSTUP_HOME` in the environment. Handling those doesn't need to block this patch though. ::: build/moz.configure/rust.configure:14 (Diff revision 1) > + return [expanduser('~/.cargo/bin')] > + > # Rust is required by `rust_compiler` below. We allow_missing here > # to propagate failures to the better error message there. > -rustc = check_prog('RUSTC', ['rustc'], allow_missing=True) > -cargo = check_prog('CARGO', ['cargo'], allow_missing=True) > +rustc = check_prog('RUSTC', ['rustc'], allow_missing=True, paths=rustup_bin_path) > +cargo = check_prog('CARGO', ['cargo'], allow_missing=True, paths=rustup_bin_path) It looks like you'd have to duplicate the path lookup logic in find_program to add `~/.cargo/bin` to the end of the default PATH. I'd suggest either abstracting that into a separate function so you can call it here, and in `find_program`, or just check for failure here and call again with `rustup_bin_path`. Note that we want the check `~/.cargo/bin` *after* everything in PATH so customizations work as expected.

See toolchain_search_path in toolchain.configure.

The way we did it for the llvm-config that gets installed via `mach bootstrap` is to search for: ['llvm-config', ...other variant spellings..., '/home/path/to/.mozbuild/clang/bin'] and that way you don't need to muck around with passing a special path in, since non-absolute filenames get searched along $PATH and absolute filenames just get checked. Whether that's the Right Way to do things...

(In reply to Nathan Froyd [:froydnj] from comment #5) > The way we did it for the llvm-config that gets installed via `mach > bootstrap` is to search for: > > ['llvm-config', ...other variant spellings..., > '/home/path/to/.mozbuild/clang/bin'] Thanks! That seems very sensible so I copied that approach.

Comment on attachment 8904720 [details] bug 1396993 - Look in ~/.cargo/bin for rustc+cargo even they're not in PATH. https://reviewboard.mozilla.org/r/176512/#review181932 Thanks for fixing. Looks good to me except for the naming issue. ::: build/moz.configure/rust.configure:8 (Diff revision 2) > # This Source Code Form is subject to the terms of the Mozilla Public > # License, v. 2.0. If a copy of the MPL was not distributed with this > # file, You can obtain one at http://mozilla.org/MPL/2.0/. > > +@imports(_from='os.path', _import='expanduser') > +def rustup_path(what): Calling this `rustup_path` makes me think it just wraps the command name, not that it tries both. Maybe call this `append_rustup_path` or `add_rustup_path` instead?

Comment on attachment 8904720 [details] bug 1396993 - Look in ~/.cargo/bin for rustc+cargo even they're not in PATH. https://reviewboard.mozilla.org/r/176512/#review182008

https://hg.mozilla.org/integration/mozilla-inbound/rev/4a93894dd13faa7acafc2c175e6e1ddea32c2599 bug 1396993 - Look in ~/.cargo/bin for rustc+cargo even they're not in PATH. r=rillian

Comment on attachment 8904720 [details] bug 1396993 - Look in ~/.cargo/bin for rustc+cargo even they're not in PATH. https://reviewboard.mozilla.org/r/176512/#review181590 > Does this do the correct thing in the msys environment we build under on windows? > > Also, it looks like `rustup` checks both `CARGO_HOME` and `RUSTUP_HOME` in the environment. Handling those doesn't need to block this patch though. For the record: ``` >>> os.path.expanduser('~/.cargo/bin') 'c:/Users/ted/.cargo/bin' $ which rustc /c/Users/ted/.cargo/bin/rustc.exe ```

Comment on attachment 8904720 [details] bug 1396993 - Look in ~/.cargo/bin for rustc+cargo even they're not in PATH. https://reviewboard.mozilla.org/r/176512/#review181932 > Calling this `rustup_path` makes me think it just wraps the command name, not that it tries both. Maybe call this `append_rustup_path` or `add_rustup_path` instead? I renamed this to `add_rustup_path`.

https://hg.mozilla.org/integration/mozilla-inbound/rev/f5e947cf3133d68f61e4b931a8516f458c8f0645 bug 1396993 - fix flake8 lint. r=bustage CLOSED TREE

https://hg.mozilla.org/mozilla-central/rev/4a93894dd13f https://hg.mozilla.org/mozilla-central/rev/f5e947cf3133

https://hg.mozilla.org/mozilla-central/rev/4a93894dd13f https://hg.mozilla.org/mozilla-central/rev/f5e947cf3133