Stylo packages (including cbindgen) and node are not installed if --no-interactive is passed to bootstrap

VERIFIED FIXED in Firefox 63

Status

defect
VERIFIED FIXED
11 months ago
10 days ago

People

(Reporter: pdknsk, Assigned: emilio)

Tracking

Trunk
mozilla63

Firefox Tracking Flags

(firefox63 fixed)

Details

Attachments

(1 attachment)

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/66.0.3359.139 Safari/537.36

Steps to reproduce:

This is a recent failure.

$ ./mach bootstrap --no-interactive --application-choice browser
...
Could not find a Rust compiler.
Will try to install Rust.
Downloading rustup-init... Ok
Running rustup-init...
info: syncing channel updates for 'stable-x86_64-unknown-linux-gnu'
info: latest update on 2018-08-02, rust version 1.28.0 (9634041f0 2018-07-30)
info: downloading component 'rustc'
info: downloading component 'rust-std'
info: downloading component 'cargo'
info: downloading component 'rust-docs'
info: installing component 'rustc'
info: installing component 'rust-std'
info: installing component 'cargo'
info: installing component 'rust-docs'
info: default toolchain set to 'stable'
...
$ ./mach build
...
 0:13.66 checking for rustc... /root/.cargo/bin/rustc
 0:13.66 checking for cargo... /root/.cargo/bin/cargo
 0:13.71 checking rustc version... 1.28.0
 0:13.73 checking cargo version... 1.28.0
 0:14.06 checking for rustdoc... /root/.cargo/bin/rustdoc
 0:14.06 checking for cbindgen... not found
 0:14.06 DEBUG: cbindgen: Trying cbindgen
 0:14.06 DEBUG: cbindgen: Trying /root/.cargo/bin/cbindgen
 0:14.06 ERROR: Cannot find cbindgen
Which OS is this?
Ubuntu 16.04
Huh, wat. For whatever reason we only install this kind of packages in interactive mode:

  https://searchfox.org/mozilla-central/rev/ef8b3886cb173d5534b954b6fb7eb2d94a9473d0/python/mozboot/mozboot/bootstrap.py#319

Looks like this comes from bug 1314355, and I couldn't find the reason for that. Nathan, do you know why that condition?
Flags: needinfo?(nfroyd)
Status: UNCONFIRMED → NEW
Component: Untriaged → Bootstrap Configuration
Ever confirmed: true
Product: Firefox → Firefox Build System
Summary: ERROR: cannot find cbindgen → Stylo packages (including cbindgen) and node are not installed if --no-interactive is passed
Summary: Stylo packages (including cbindgen) and node are not installed if --no-interactive is passed → Stylo packages (including cbindgen) and node are not installed if --no-interactive is passed to bootstrap
I don't know why that condition is there, presumably removing it locally will fix that. Alternatively, manually installing cbindgen via `cargo install cbindgen` should work as well.
That probably explains why the recent update to a new node.js requirement also made the build fail, until I installed it manually.
Manually installing works for now.
It's probably obvious, but not running in non-interactive mode is not an option here, so this needs to be fixed in some way eventually.
(In reply to Emilio Cobos Álvarez (:emilio) from comment #4)
> I don't know why that condition is there, presumably removing it locally
> will fix that. Alternatively, manually installing cbindgen via `cargo
> install cbindgen` should work as well.

I think the rationale here was that we were creating bits in the user's home directory, and we considered it somewhat rude to blindly install packages into their home directory--especially when Stylo wasn't required, which is the vintage of this particular code.

That being said, we install rustup (which is at least as intrusive) in non-interactive mode, and Android bootstrap has long created bits in the user's home directory without bothering to ask whether that's a good idea or not.  The --no-system-changes flag, which I guess is The Future(tm), also seems to be an explicit invitation to create whatever you like in the user's private space.

I think the final call on removing the --no-interactive check for private LLVM/Clang/Node is up to gps, who owns (?) bootstrap and recently reviewed the Node changes, which were also placed under the --no-interactive flag.
Flags: needinfo?(nfroyd) → needinfo?(gps)
If it's decided to no remove the check, I'd like to ask for a flag like --system-changes to allow it.
--no-interactive's behavior is not ideal and can be explained by "historical reasons."

The important flag to honor is --no-system-changes, which shouldn't allow touching things outside of HOME when present.

I think simple toolchain artifact downloading can be performed when --no-interactive is present.

Rust installation is a bit harder, as installing rustup involves touching your shell init scripts and PATH. There should be a prompt for that. But if --no-interactive is given, the user doesn't want to see prompts. So IMO the presence of the flag can be interpreted as consent.

Where we get into problems is where `mach bootstrap --no-interactive` wants to do something new in the future. If we tell people to run `mach bootstrap --no-interactive` periodically, we get into problems where a future invocation does something new that the user doesn't want.

FWIW, I think `mach bootstrap` needs some significant love and a redesign to reflect the new reality of its ever-growing set of features. But it isn't at the top of the prioritization list. So we have to make due with the existing ball of wax :/
Flags: needinfo?(gps)
The state directory is in $HOME by default, so should be fine to just create it
if we get --no-interactive I think.
Comment on attachment 9002604 [details]
Create state dir and install node / stylo stuff in bootstrap's non-interactive mode.

Ted Mielczarek [:ted] [:ted.mielczarek] has approved the revision.
Attachment #9002604 - Flags: review+
Pushed by emilio@crisal.io:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4b7383b27f89
Create state dir and install node / stylo stuff in bootstrap's non-interactive mode. r=ted
https://hg.mozilla.org/mozilla-central/rev/4b7383b27f89
Status: NEW → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Verified.
Assignee: nobody → emilio
Status: RESOLVED → VERIFIED
Duplicate of this bug: 1465565
You need to log in before you can comment on or make changes to this bug.