1415922 - Stylo packages not installed during mach bootstrap if .hgrc file has alias for log set to log -G

Status: RESOLVED FIXED

(Firefox Build System :: General, defect, P5)

Description

[Michael Froman [:mjf]]

There are a couple things here:
1) If .hgrc file aliases log to log -G, the stylo packages are not automatically installed/upgraded by ./mach boostrap.  In my case, I see the following message, even when I'm running ./mach bootstrap from a valid checkout of mozilla-central:
Installing Stylo packages requires a checkout of mozilla-central. Once you
have such a checkout, please re-run `./mach bootstrap` from the checkout
directory.

If I remove the alias for log in the .hgrc file, bootstrap does the install/upgrade.  In this case it was a download and install of clang that was needed.

2) The message above "Installing Stylo packages..." does not make it completely clear that this is really an error or exception message, as in "Bootstrap has not detected a checkout of mozilla-central."

Comment 1

[Michael Froman [:mjf]]

3) I'm not sure if this is intentional or not: With no alias for log, everytime I run ./mach bootstrap, I see:
 0:04.83 Downloading clang.tar.xz
 0:04.83 Downloading to temporary location /home/mfroman/.mozbuild/toolchains/b4256ce16c9703b6-clang.tar.xz
 0:04.83 Downloaded artifact to /home/mfroman/.mozbuild/toolchains/b4256ce16c9703b6-clang.tar.xz
 0:05.34 rm tree: /home/mfroman/.mozbuild/clang
 0:05.56 untarring "/home/mfroman/.mozbuild/clang.tar.xz"

Should bootstrap download clang every time even if it downloaded it within the last minute?

4) After trying things a few time, I realized that _with_ the alias set to 'log -G', each time I run ./mach boostrap it offers:
If you would like to clone the mozilla-unified Mercurial repository, please
enter the destination path below.

Without the alias, it does not offer to clone the Mercurial repo.  Again, I think a message about bootstrap not detecting a checkout of mozilla-central would be helpful.

Comment 2

[Michael Froman [:mjf]]

Attachment: Bug1415922.patch

This resets any aliases to the log command to the default when looking for rev 0 to verify the hg repo.  I believe something similar may be needed in python/mozversioncontrol/mozversioncontrol/__init__.py for the head_ref call.

Comment 3

[Chris Peterson [:cpeterson]]

Michael, is your patch ready for review? Or are you still working on it?

Comment 4

[Michael Froman [:mjf]]

Chris, providing you have no concerns about other places that aliases for the log command might cause issues, my patch is ready for review insofar as it fixes issues 1 and 4 above.

I will leave it up to you to decide whether issues 2 and 3 should be addressed (possibly in other bugs).

Comment 5

[Gregory Szorc [:gps]]

Comment on attachment 8926958 [details] [diff] [review]
Bug1415922.patch

Review of attachment 8926958 [details] [diff] [review]:
-----------------------------------------------------------------

::: python/mozboot/mozboot/bootstrap.py
@@ +520,4 @@
>          if hg and os.path.exists(hg_dir):
>              # Verify the hg repo is a Firefox repo by looking at rev 0.
>              try:
> +                node = check_output([hg, '--config', 'alias.log=log', 'log', '-r', '0', '--template', '{node}'], cwd=path)

We should set the HGPLAIN=1 environment variable to disable loading of aliases.

Generally speaking, any scripting usage of Mercurial should set HGPLAIN=1 to minimize variation at run-time. See `hg help scripting`.

Comment 6

[Chris Peterson [:cpeterson]]

(In reply to Michael Froman [:mjf] from comment #4)
> Chris, providing you have no concerns about other places that aliases for
> the log command might cause issues, my patch is ready for review insofar as
> it fixes issues 1 and 4 above.

I wouldn't be reviewing your patch. Your patch didn't have the r? flag set, so I just wanted to make sure your patch didn't get overlooked if was ready.

After you implement Greg's HGPLAIN=1 suggestion, we can find a reviewer.

Comment 7

[Michael Froman [:mjf]]

(In reply to Chris Peterson [:cpeterson] from comment #6)
> (In reply to Michael Froman [:mjf] from comment #4)
> > Chris, providing you have no concerns about other places that aliases for
> > the log command might cause issues, my patch is ready for review insofar as
> > it fixes issues 1 and 4 above.
> 
> I wouldn't be reviewing your patch. Your patch didn't have the r? flag set,
> so I just wanted to make sure your patch didn't get overlooked if was ready.
> 
> After you implement Greg's HGPLAIN=1 suggestion, we can find a reviewer.

I was really hoping that a true build peer would take over with the offered fix since I'm doing my normal Moz WebRTC work, and I figured that there was a more general solution that was needed over the entire bootstrap process.  That said, I'll take another stab at it, but after that I need to get back to what I'm supposed to be working on! :-)

Comment 8

[Nathan Froyd [:froydnj]]

(In reply to Michael Froman [:mjf] from comment #7)
> I was really hoping that a true build peer would take over with the offered
> fix since I'm doing my normal Moz WebRTC work, and I figured that there was
> a more general solution that was needed over the entire bootstrap process. 

I can poke at this.

Comment 9

[Michael Froman [:mjf]]

(In reply to Nathan Froyd [:froydnj] from comment #8)
> (In reply to Michael Froman [:mjf] from comment #7)
> > I was really hoping that a true build peer would take over with the offered
> > fix since I'm doing my normal Moz WebRTC work, and I figured that there was
> > a more general solution that was needed over the entire bootstrap process. 
> 
> I can poke at this.
Thank you - I just was trying to add the env calls (like _hgplain_env), and all I managed to do was make bootstrap crash in several ways. :-)  Please let me know if I can help with testing or anything else.

Comment 10

[Michael Froman [:mjf]]

Attachment: ensure bootstrap's current_firefox_checkout using HGPLAIN=1

This avoids issues with hg log aliases (specifically hg log -G) causing hg repo detection to fail.

Comment 11

[Michael Froman [:mjf]]

Chris, I had a few spare minutes so I took another swing at this.  Please let me know if I need to do anything different for the review request!

Comment 12

[Chris Peterson [:cpeterson]]

Thanks! The review request looks good. A core-build-config reviewer will review the code change.

Are there other hg calls that should use HGPLAIN=1? You mentioned python/mozversioncontrol/mozversioncontrol/__init__.py's head_ref() in comment 2 above. That script looks like it currently sets HGPLAIN=1 (in class HgRepository), so that call is OK.

Comment 13

[Michael Froman [:mjf]]

(In reply to Chris Peterson [:cpeterson] from comment #12)
> Are there other hg calls that should use HGPLAIN=1?
I'm not sure.  This is the big one, since it has caused me pain for a while now. ;-)  If I stumble onto anything in the future, I'll open a new bug.

Comment 14

[Chris Peterson [:cpeterson]]

(In reply to Michael Froman [:mjf] from comment #13)
> (In reply to Chris Peterson [:cpeterson] from comment #12)
> > Are there other hg calls that should use HGPLAIN=1?
> I'm not sure.  This is the big one, since it has caused me pain for a while
> now. ;-)  If I stumble onto anything in the future, I'll open a new bug.

I was just wondering out loud. I didn't mean to suggest you to find and fix them all! :)

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3b3d07447b82
"Stylo packages not installed during mach bootstrap if .hgrc file has alias for log set to log -G" [r=gps]

Comment 16

[Tiberius Oros[:tiberius_oros]]

https://hg.mozilla.org/mozilla-central/rev/3b3d07447b82

Comment 17

[alec.muffett]

Hi! I believe this bug has broken installation on OSX, at least for me?

I'm following the installation instructions at 

https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Mac_OS_X_Prerequisites

...and am seeing the attached, on 10.13.2 with fully up-to-date Homebrew and nothing particularly weird.

    -alec




00:12:58 hoover:firefox $ python bootstrap.py

Note on Artifact Mode:

Firefox for Desktop and Android supports a fast build mode called
artifact mode. Artifact mode downloads pre-built C++ components rather
than building them locally, trading bandwidth for time.

Artifact builds will be useful to many developers who are not working
with compiled code. If you want to work on look-and-feel of Firefox,
you want "Firefox for Desktop Artifact Mode".

Similarly, if you want to work on the look-and-feel of Firefox for Android,
you want "Firefox for Android Artifact Mode".

To work on the Gecko technology platform, you would need to opt to full,
non-artifact mode. Gecko is Mozilla's web rendering engine, similar to Edge,
Blink, and WebKit. Gecko is implemented in C++ and JavaScript. If you
want to work on web rendering, you want "Firefox for Desktop", or
"Firefox for Android".

If you don't know what you want, start with just Artifact Mode of the desired
platform. Your builds will be much shorter than if you build Gecko as well.
But don't worry! You can always switch configurations later.

You can learn more about Artifact mode builds at
https://developer.mozilla.org/en-US/docs/Artifact_builds.

Please choose the version of Firefox you want to build:
1. Firefox for Desktop Artifact Mode
2. Firefox for Desktop
3. Firefox for Android Artifact Mode
4. Firefox for Android
Your choice: 2

Looks like you have Homebrew installed. We will install all required packages via Homebrew.

Your version of Mercurial (4.4.2) is sufficiently modern.
Your version of Python (2.7.14) is new enough.
Your version of Rust (1.23.0) is new enough.
Rust supports x86_64-apple-darwin targets.
Traceback (most recent call last):
  File "bootstrap.py", line 172, in <module>
    sys.exit(main(sys.argv))
  File "bootstrap.py", line 163, in main
    dasboot.bootstrap()
  File "/tmp/tmpVoju2l/mozboot/bootstrap.py", line 324, in bootstrap

AttributeError: 'OSXBootstrapper' object has no attribute '_hgplain_env'

Comment 18

[u610118]

Hi, I believe this has also caused me to fail my installation on Ubuntu 16.04:

Traceback (most recent call last):
  File "bootstrap.py", line 172, in <module>
    sys.exit(main(sys.argv))
  File "bootstrap.py", line 163, in main
    dasboot.bootstrap()
  File "/tmp/tmpX5QM0f/mozboot/bootstrap.py", line 324, in bootstrap

AttributeError: 'DebianBootstrapper' object has no attribute '_hgplain_env'

Comment 19

[Nathan Froyd [:froydnj]]

Errors with `_hgplain_env` should be fixed by bug 1431226, which landed on mozilla-central recently.  If you're still having problems after double-checking that your tree includes that fix, please open a new bug for your specific problems.  Thanks!