This has a number of benefits.

Using this command is more robust than our current method, and brings several benefits, such as smart caching of the downloaded toolchain. We change the clang package downloaded for Windows with this change, but bindgen has been updated to work well with LLVM 5.0, so there should be no problems. I think this should work, though we may need to include a specific Python in the command invocation for Windows.

Comment on attachment 8867387 [details] [diff] [review] use `mach artifact toolchain' to download clang Stylo packages Review of attachment 8867387 [details] [diff] [review]: ----------------------------------------------------------------- ::: python/mozboot/mozboot/base.py @@ +278,4 @@ > > + mach_binary = os.path.join(topsrcdir, 'mach') > + if not os.path.exists(mach_binary): > + raise ValueError("mach not found at %s" % mach_binary) This means the stylo download step will fail if bootstrap.py is running out outside the context of a firefox source tree (like in an initial build environment bootstrap) right? I suppose that's ok, but it might be nice to skip the stylo prompt if mach and the tooltool manifests aren't present. You could also fall back to pulling the manifests over the network `json.loads()` them to get the hashes. That's code duplication, but better preserves the 'bootstrap' sense of the mozboot module.

(In reply to Ralph Giles (:rillian) | needinfo me from comment #2) > ::: python/mozboot/mozboot/base.py > @@ +278,4 @@ > > > > + mach_binary = os.path.join(topsrcdir, 'mach') > > + if not os.path.exists(mach_binary): > > + raise ValueError("mach not found at %s" % mach_binary) > > This means the stylo download step will fail if bootstrap.py is running out > outside the context of a firefox source tree (like in an initial build > environment bootstrap) right? Uh. We have a mode like that? I had heard rumors, but never understood how that was supposed to work. You apparently know, though--can you explain? > You could also fall back to pulling the manifests over the network > `json.loads()` them to get the hashes. That's code duplication, but better > preserves the 'bootstrap' sense of the mozboot module. Then I have to keep all the tooltool code...not sure if it's worth it or not.

bootstrap also clones m-c if it's not run from it, so, just defer to after m-c is cloned?

(In reply to Nathan Froyd [:froydnj] from comment #3) > Uh. We have a mode like that? I had heard rumors, but never understood how > that was supposed to work. You apparently know, though--can you explain? The general idea is that one can start with a system with only a python installation, download python/mozboot/bin/bootstrap.py from the repository web interface, and execute it. It will pull the rest of the module and then use it to start installing dependencies. I can imagine it's handy if one isn't used to using version control to check out the source first, but I don't know how common that situation is. Anyway, this is why mozboot doesn't use any of the common infrastructure available to other python modules in Firefox. See https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_build/Linux_and_MacOS_build_preparation or https://github.com/mozilla/gecko-dev/blob/master/python/mozboot/README.rst for example instructions for people to do this. > Then I have to keep all the tooltool code...not sure if it's worth it or not. I don't either. Of course you could move the tooltool wrapper into mozboot and have mach call into it. (In reply to Mike Hommey [:glandium] from comment #4) > bootstrap also clones m-c if it's not run from it, so, just defer to after > m-c is cloned? This is in https://dxr.mozilla.org/mozilla-central/source/python/mozboot/mozboot/bootstrap.py#398 btw. I'd forgotten about this, because I always follow the git prompts and this only happens with hg. At least we're writing an expert system in python instead of sh+m4. :)

And we also want to support Git clones in the one-line bootstrapper. It just hasn't been implemented yet. Also, in the one-line bootstrap mode, we could download extra files (such as tooltool manifests) if we wanted to. Code for that lives at https://dxr.mozilla.org/mozilla-central/source/python/mozboot/bin/bootstrap.py#107. The considerations for adding new files to download are: * Latency. More individual files, more round trips, slower bootstrap execution. * Defining paths to files. You'll have to give the bootstrapper paths to certain files, since they may be in a relative source directory under version control or in a temp directory.

(In reply to Gregory Szorc [:gps] from comment #6) > And we also want to support Git clones in the one-line bootstrapper. It just > hasn't been implemented yet. It also seems a bit weird that we prompt for the clone target location. I suspect this is a really a limitation of mozboot's prompting, but "what do you want to call the checkout" seems a trivial question if the point is to automate a lot of complicated steps. I'll file a bug. > * Latency. More individual files, more round trips, slower bootstrap > execution. In theory we could address this by promise-ifying the downloads. The manifests we'd need to make this bug work in one-line mode are small, so the latency will be easy to hide behind the larger network and disk-walking tasks we do between startup and getting an answer to the stylo question.

(In reply to Ralph Giles (:rillian) | needinfo me from comment #5) > (In reply to Nathan Froyd [:froydnj] from comment #3) > > > Uh. We have a mode like that? I had heard rumors, but never understood how > > that was supposed to work. You apparently know, though--can you explain? > > The general idea is that one can start with a system with only a python > installation, download python/mozboot/bin/bootstrap.py from the repository > web interface, and execute it. It will pull the rest of the module and then > use it to start installing dependencies. Ah, OK, thanks for the explanation. I've spent some time wrapping my head around how all of this works. > > Then I have to keep all the tooltool code...not sure if it's worth it or not. > > I don't either. Of course you could move the tooltool wrapper into mozboot > and have mach call into it. I looked at doing this: `mach artifact toolchain` is pretty large on its own and pulls in nine kinds of dependencies from outside (taskcluster stuff, various bits of mozbuild, etc.), and trying to untangle that so we can put it in mozboot doesn't seem like a good idea. Untangling it so a minimum skeleton of functionality can be used from mozboot (or cut-and-pasted, or whatever) also doesn't feel like the right thing. No point in turning mozboot into a pile of utility code that people have to import just so the bootstrapper can be nice. (Maybe the solution is pulling more python directories from hgweb in the bootstrapper?) I'm kind of inclining towards fixing the problems the current code has (e.g. Windows paths), and leaving `mach artifact toolchain` as future work at this point. > (In reply to Mike Hommey [:glandium] from comment #4) > > bootstrap also clones m-c if it's not run from it, so, just defer to after > > m-c is cloned? I am not super-excited about doing this, because in my mind it breaks one of the core ideas of bootstrap: run it once, and you have everything, you're ready to go. There's a bit of code in there that asks you to start a new shell if a suitable version of Rust can't be found, but that doesn't feel like a violation of the core principle. Especially with bootstrap asking you to run through most of the questions again, I'd rather not ask people to run things multiple times. WDYT? Do you think we should just make the current code more viable, or do you think we should try to polish it up?

I wasn't suggesting people to run mach bootstrap twice, but for bootstrap to do stuff after it cloned m-c.

I think you should just (a) not prompt to install llvm if bootstrap is running stand-alone, meaning such users will have to run bootstrap again once they have a checkout once stylo is required, or (b) delay the prompt until a checkout is available and call mach from there, as glandium suggested. Those are both ways to get this viable enough to land.

These patches haven't been tested, and I'm not quite done with all the modifications needed, but I thought I might as well post them for potential feedback while I'm writing later patches in the series. Now that we're installing Stylo packages through `mach`, we need to have `mach` available, which is only true after we've checked for a clone of the tree. We move the block performing the installation in this commit and then fixup resulting problems in future commits. Thanks to glandium for the clarification on what to do here.

If we're running bootstrap from inside a mozilla-central checkout, we'll have all the necessary files that we need to install tooltool packages via `mach artifact toolchain`. If we're running bootstrap from a downloaded bootstrap.py, however, it's possible that we failed to clone the repository. We need to account for that situation and provide instructions on how to deal with it. A consequence of these two mechanisms for bootstrap is that we'll have to make sure we're invoking `mach artifact toolchain` correctly in both cases, which we'll handle in subsequent commits. I think doing things this way is kind of fallout from not loudly failing if we're not able to clone. WDYT about just sys.exit(1)'ing if we fail to clone the tree in any way, with a message about re-running `mach bootstrap`, so people can try again? Do you think that's any better than this approach?

(In reply to Nathan Froyd [:froydnj] from comment #12) > I think doing things this way is kind of fallout from not loudly failing if > we're not able to clone. WDYT about just sys.exit(1)'ing if we fail to clone > the tree in any way, with a message about re-running `mach bootstrap`, so > people can try again? Do you think that's any better than this approach? That sounds like something we could do after cloning from git is supported. It might be good enough for these purposes to hack that to just pull https://github.com/mozilla/gecko-dev/ which is easy to add, but doesn't support contributing changes back.

Comment on attachment 8871378 [details] [diff] [review] part 3 - check for a clone before installing Stylo packages Review of attachment 8871378 [details] [diff] [review]: ----------------------------------------------------------------- Works for me.

Comment on attachment 8871376 [details] [diff] [review] part 2 - move Stylo package installation Review of attachment 8871376 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for the separate patches for review. I think you should squash this together with part 3 before landing though, since it has the same problems as the previous patch as it stands.

We'll need this information to be able to locate mach later, since we can't assume that mach is just several directories above the script we're currently running.

This change ensures that when we're running bootstrap in standalone mode, we find the correct mach to invoke. Still need to check on windows whether we need to invoke mach through Python or not. Are we always running this stuff from a shell that can invoke Python scripts directly?

(In reply to Ralph Giles (:rillian) | needinfo me from comment #15) > Thanks for the separate patches for review. I think you should squash this > together with part 3 before landing though, since it has the same problems > as the previous patch as it stands. Technically everything after part 1 is fixing brokenness that part 1 introduced, sooo... :)

This is a fixup to the original patch to use `mach artifact toolchain`, discovered by testing this patch series on Windows. I'll fold it in before committing, but it's easier to review separately.

Comment on attachment 8871825 [details] [diff] [review] part 1a - use sys.executable to invoke mach Review of attachment 8871825 [details] [diff] [review]: ----------------------------------------------------------------- ::: python/mozboot/mozboot/base.py @@ +278,5 @@ > mach_binary = os.path.join(checkout_root, 'mach') > if not os.path.exists(mach_binary): > raise ValueError("mach not found at %s" % mach_binary) > > + # XXX not much we can do here on Windows You don't say what the error was here. Is sys.executable not a path under cygwin?

(In reply to Ralph Giles (:rillian) | needinfo me from comment #20) > ::: python/mozboot/mozboot/base.py > @@ +278,5 @@ > > mach_binary = os.path.join(checkout_root, 'mach') > > if not os.path.exists(mach_binary): > > raise ValueError("mach not found at %s" % mach_binary) > > > > + # XXX not much we can do here on Windows > > You don't say what the error was here. Is sys.executable not a path under > cygwin? The poorly expressed intent of this comment was that sys.executable can actually be something nonsensical, like None, in which case there's not much we can do for attempting to execute a Python script. And trying to pass: [None, 'mach', 'artifact', ...] to execv or similar syscall is not going to end very well. I have not dug around in the Python source to how common a None sys.executable might be, whether it occurs only on weird operating systems or similar.

Pushed by nfroyd@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/25c41cd44a05 part 1 - use `mach artifact toolchain' to download clang Stylo packages; r=rillian https://hg.mozilla.org/integration/mozilla-inbound/rev/c37162881c6e part 2 - move Stylo package installation; r=rillian https://hg.mozilla.org/integration/mozilla-inbound/rev/78a7d1f7f4d7 part 3 - check for a clone before installing Stylo packages; r=rillian https://hg.mozilla.org/integration/mozilla-inbound/rev/93ba2fb5fe5d part 4 - compute the firefox checkout root; r=rillian https://hg.mozilla.org/integration/mozilla-inbound/rev/94a6ef48ce1a part 5 - pass the checkout_root down to stylo package installation; r=rillian

(In reply to Nathan Froyd [:froydnj] from comment #21) > I have not dug around in the Python source to how common a None sys.executable might be, > whether it occurs only on weird operating systems or similar. Ok. I saw that in the docs, and figured that was the idea; the comment just made me think you were handling a specific case. yay landing!

https://hg.mozilla.org/mozilla-central/rev/25c41cd44a05 https://hg.mozilla.org/mozilla-central/rev/c37162881c6e https://hg.mozilla.org/mozilla-central/rev/78a7d1f7f4d7 https://hg.mozilla.org/mozilla-central/rev/93ba2fb5fe5d https://hg.mozilla.org/mozilla-central/rev/94a6ef48ce1a