Closed Bug 1282256 Opened 4 years ago Closed 4 years ago

Frequent windows mozbuild.mozconfig.MozconfigLoadException: Evaluation of your mozconfig exited with an error. This could be triggered

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(firefox50 fixed, firefox51 fixed)

RESOLVED FIXED
mozilla51
Tracking Status
firefox50 --- fixed
firefox51 --- fixed

People

(Reporter: intermittent-bug-filer, Assigned: glandium)

References

Details

(Keywords: intermittent-failure)

Attachments

(4 files)

Filed by: philringnalda@gmail.com

https://treeherder.mozilla.org/logviewer.html#?job_id=10101839&repo=fx-team

http://archive.mozilla.org/pub/firefox/tinderbox-builds/fx-team-win64/1466891003/fx-team-win64-bm94-build1-build8.txt.gz

By which it means:

15:26:22     INFO -  c:\builds\moz2_slave\fx-team-w64-000000000000000000\build\src\python\mozbuild\mozbuild\mozconfig.py:248: UnicodeWarning: Unicode equal comparison failed to convert both arguments to Unicode - interpreting them as being unequal
15:26:22     INFO -    index = lines.index('------END_BEFORE_SOURCE')
15:26:22     INFO -  Traceback (most recent call last):
15:26:22     INFO -    File "c:/builds/moz2_slave/fx-team-w64-000000000000000000/build/src/config/expandlibs_gen.py", line 11, in <module>
15:26:22     INFO -      import expandlibs_config as conf
15:26:22     INFO -    File "c:\builds\moz2_slave\fx-team-w64-000000000000000000\build\src\build\mach_bootstrap.py", line 380, in __call__
15:26:22     INFO -      module = self._original_import(name, globals, locals, fromlist, level)
15:26:22     INFO -    File "c:\builds\moz2_slave\fx-team-w64-000000000000000000\build\src\config\expandlibs_config.py", line 5, in <module>
15:26:22     INFO -      from buildconfig import substs
15:26:22     INFO -    File "c:\builds\moz2_slave\fx-team-w64-000000000000000000\build\src\build\mach_bootstrap.py", line 380, in __call__
15:26:22     INFO -      module = self._original_import(name, globals, locals, fromlist, level)
15:26:22     INFO -    File "c:\builds\moz2_slave\fx-team-w64-000000000000000000\build\src\build\buildconfig.py", line 9, in <module>
15:26:22     INFO -      config = MozbuildObject.from_environment()
15:26:22     INFO -    File "c:\builds\moz2_slave\fx-team-w64-000000000000000000\build\src\python\mozbuild\mozbuild\base.py", line 169, in from_environment
15:26:22     INFO -      config = loader.read_mozconfig(mozconfig, moz_build_app=current_project)
15:26:22     INFO -    File "c:\builds\moz2_slave\fx-team-w64-000000000000000000\build\src\python\mozbuild\mozbuild\mozconfig.py", line 253, in read_mozconfig
15:26:22     INFO -      raise MozconfigLoadException(path, MOZCONFIG_BAD_EXIT_CODE, lines)
Intermittent?!
(In reply to Gregory Szorc [:gps] from comment #1)
> Intermittent?!

well more frequent
Blocks: 1278415
Assignee: nobody → mh+mozilla
Blocks: 1291589
We've been reading the mozconfig in MozbuildObject.from_environment to
check whether the mozconfig topobjdir matches the detected topobjdir.

Since bug 1278415, everything using the buildconfig python module now
calls MozbuildObject.from_environment, which reads the mozconfig. A lot
of things to that during the build. But none of them actually need the
data from the mozconfig, and the topobjdir match test has been breaking
things randomly on multiple occasions.

The topobjdir match test, however, really only needs to happen once:
when a mach command starts. So we can move the test to MachCommandBase,
where it belongs, and anything actively using MozbuildObject.mozconfig
will have the mozconfig read, but everything else won't.

On a Linux64 opt build, this brings down the number of times the
mozconfig is read during `mach build` from 979 to 9.

Review commit: https://reviewboard.mozilla.org/r/69168/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/69168/
Attachment #8777651 - Flags: review?(gps)
Let's pile up some clean up as well.
Back when it was added, it was used, but it is not anymore, outside
test_base.py.

Review commit: https://reviewboard.mozilla.org/r/69174/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/69174/
Attachment #8777654 - Flags: review?(gps)
Attachment #8777655 - Flags: review?(gps)
Attachment #8777656 - Flags: review?(gps)
The only use that didn't have an existing instance was just removed.

Review commit: https://reviewboard.mozilla.org/r/69176/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/69176/
The last use that didn't have an existing instance was just removed.

Review commit: https://reviewboard.mozilla.org/r/69178/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/69178/
(In reply to Mike Hommey [:glandium] from comment #8)
> On a Linux64 opt build, this brings down the number of times the
> mozconfig is read during `mach build` from 979 to 9.

The remaining 9 come from:
- 5 times `mach environment --format=client.mk`
- twice `mach environment --format=json -o $topobjdir/.mozconfig.json`
- twice `configure.py`

That's suboptimal, but that can be left for a followup. Or we could declare that we don't care about the first 2 items, because they come from client.mk and we'll eventually get rid of it. As for configure.py, it reads it twice because once before checking PYTHON, and once after restarting in the virtualenv.
No longer blocks: 1291589
Duplicate of this bug: 1291589
Comment on attachment 8777651 [details]
Bug 1282256 - Avoid loading mozconfig in MozbuildObject.from_environment.

https://reviewboard.mozilla.org/r/69168/#review66808

I didn't realize we were evaluating the mozconfig so often during a build. That was horrible.
Attachment #8777651 - Flags: review?(gps) → review+
Comment on attachment 8777654 [details]
Bug 1282256 - Remove MozbuildObject._config_guess.

https://reviewboard.mozilla.org/r/69174/#review66810
Attachment #8777654 - Flags: review?(gps) → review+
Comment on attachment 8777655 [details]
Bug 1282256 - Make MozbuildObject.resolve_mozconfig_topobjdir an instance method.

https://reviewboard.mozilla.org/r/69176/#review66812
Attachment #8777655 - Flags: review?(gps) → review+
Comment on attachment 8777656 [details]
Bug 1282256 - Make MozbuildObject.resolve_config_guess an instance method.

https://reviewboard.mozilla.org/r/69178/#review66814
Attachment #8777656 - Flags: review?(gps) → review+
Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/autoland/rev/9774cf27be69
Avoid loading mozconfig in MozbuildObject.from_environment. r=gps
https://hg.mozilla.org/integration/autoland/rev/fbeb48318c1e
Remove MozbuildObject._config_guess. r=gps
https://hg.mozilla.org/integration/autoland/rev/58bdfaad94aa
Make MozbuildObject.resolve_mozconfig_topobjdir an instance method. r=gps
https://hg.mozilla.org/integration/autoland/rev/61a4dd746681
Make MozbuildObject.resolve_config_guess an instance method. r=gps
Please request Aurora approval on this when you get a chance.
Flags: needinfo?(mh+mozilla)
Does it happen enough on aurora to take the risk? For one, there's at least one followup bug this caused. I'm also not entirely sure how independent it is from other changes to the build system. So I'd rather not try to uplift if it's not too much of a problem on aurora.
Flags: needinfo?(mh+mozilla) → needinfo?(ryanvm)
It happens on Aurora as well, but not at a high enough frequency to care if there's regression possibilities. On the other hand, it could at least in theory be a bigger problem when 50 goes to Beta if a broken build delays a go to build.
Flags: needinfo?(ryanvm)
(In reply to Mike Hommey [:glandium] from comment #23)
> Does it happen enough on aurora to take the risk? For one, there's at least
> one followup bug this caused.

I might have misremembered here, because I couldn't find a trace of such a followup when looking at the history following this bug. However *this* bug is *also* a fixup for previous bugs, some of which are in 50, while this one is in 51 only. I'm suspecting this is the cause for bug 1303926 (and try confirms that uplifting this to beta fixes that bug)
(In reply to Mike Hommey [:glandium] from comment #25)
> (In reply to Mike Hommey [:glandium] from comment #23)
> > Does it happen enough on aurora to take the risk? For one, there's at least
> > one followup bug this caused.
> 
> I might have misremembered here, because I couldn't find a trace of such a
> followup when looking at the history following this bug. However *this* bug
> is *also* a fixup for previous bugs, some of which are in 50, while this one
> is in 51 only. I'm suspecting this is the cause for bug 1303926 (and try
> confirms that uplifting this to beta fixes that bug)

mike, sylvestre, can we have approval for beta for this changes to fix bug 1303926 ?
Flags: needinfo?(sledru)
Flags: needinfo?(mh+mozilla)
s/approval/ approval or the approval request
Approved :)
Flags: needinfo?(sledru)
Duplicate of this bug: 1303926
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.