Closed Bug 1415922 Opened 2 years ago Closed 2 years ago

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

Categories

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

58 Branch
defect

Tracking

(firefox57 wontfix, firefox58 wontfix, firefox59 fixed)

RESOLVED FIXED
mozilla59
Tracking Status
firefox57 --- wontfix
firefox58 --- wontfix
firefox59 --- fixed

People

(Reporter: mjf, Assigned: mjf)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

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."
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.
Attached patch Bug1415922.patch (obsolete) — Splinter Review
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.
Michael, is your patch ready for review? Or are you still working on it?
Assignee: nobody → mfroman
Priority: -- → P5
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).
Flags: needinfo?(cpeterson)
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`.
(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.
Flags: needinfo?(cpeterson)
(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! :-)
(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.
(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.
This avoids issues with hg log aliases (specifically hg log -G) causing hg repo detection to fail.
Attachment #8926958 - Attachment is obsolete: true
Attachment #8941849 - Flags: review?(core-build-config-reviews)
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!
Flags: needinfo?(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.
Flags: needinfo?(cpeterson)
(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.
(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! :)
Attachment #8941849 - Flags: review?(core-build-config-reviews) → review?(gps)
Attachment #8941849 - Flags: review?(gps) → review+
Keywords: checkin-needed
Pushed by csabou@mozilla.com:
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]
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/3b3d07447b82
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
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'
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'
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!
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.