Closed Bug 936555 Opened 11 years ago Closed 10 years ago

mach build-backend is unhappy when MOZCONFIG was originally specified as an environment variable

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla30

People

(Reporter: froydnj, Unassigned)

References

Details

Attachments

(1 file, 3 obsolete files)

I ran $(topsrcdir)/configure from a build directory with a MOZCONFIG environment variable.  I then ran into some build issues, made changes, make complained at me, and so I ran |mach build-backend|:

[froydnj@cerebro cross-osx]$ mach build-backend
Error running mach:

    ['build-backend']

The error occurred in mach itself. This is likely a bug in mach itself or a
fundamental problem with a loaded module.

Please consider filing a bug against mach by going to the URL:

    https://bugzilla.mozilla.org/enter_bug.cgi?product=Core&component=mach


If filing a bug, please include the full output of mach, including this error
message.

The details of the failure are as follows:

MozconfigLoadException: Evaluation of your mozconfig exited with an error. This could be triggered
by a command inside your mozconfig failing. Please change your mozconfig
to not error and/or to catch errors in executed commands.

  File "/home/froydnj/src/mozilla-central-official/python/mach/mach/main.py", line 265, in run
    return self._run(argv)
  File "/home/froydnj/src/mozilla-central-official/python/mach/mach/main.py", line 351, in _run
    instance = cls(context)
  File "/home/froydnj/src/mozilla-central-official/python/mozbuild/mozbuild/base.py", line 528, in __init__
    dummy = MozbuildObject.from_environment(cwd=context.cwd)
  File "/home/froydnj/src/mozilla-central-official/python/mozbuild/mozbuild/base.py", line 165, in from_environment
    config = loader.read_mozconfig(mozconfig)
  File "/home/froydnj/src/mozilla-central-official/python/mozbuild/mozbuild/mozconfig.py", line 196, in read_mozconfig
    raise MozconfigLoadException(path, MOZCONFIG_BAD_EXIT_CODE, lines)

Adding more informative error messages showed me that mach was trying to load $(topsrcdir)/.mozconfig, which doesn't exist.

This is even less exciting than modifying individual Makefile.ins and having them trigger full-tree rebuilds.
Looks like mozconfig was set to $(topsrcdir)/.mozconfig in mozinfo.json, which is bogus.
Ah, the handling of MOZCONFIG here:

http://mxr.mozilla.org/mozilla-central/source/python/mozbuild/mozbuild/mozinfo.py#30

is broken; we should be merging MOZCONFIG with the current directory, not topsrcdir.
mozbuild.mozinfo should probably be using mozbuild.mozconfig.MozconfigLoader.
Component: mach → Build Config
This seems to DTRT for me.
Attachment #829476 - Flags: review?(gps)
Comment on attachment 829476 [details] [diff] [review]
make mozinfo use MozconfigLoader to locate the mozconfig

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

r+ conditional on change to import directive.

::: python/mozbuild/mozbuild/mozinfo.py
@@ +7,5 @@
>  
>  import os
>  import re
>  import json
> +import mozconfig

Use relative imports:

import .mozconfig as mozconfig

(What you wrote won't work in Python 3 because all imports are absolute. Python would search for the "mozconfig" package, not the "mozbuild.mozconfig" module.
Attachment #829476 - Flags: review?(gps) → review+
So test failures.

Getting the environment recognized is pretty trivial.  But this patch causes more
test failures of the form:

TEST-UNEXPECTED-FAIL | /home/froydnj/src/mozilla-central-official/python/mozbuild/mozbuild/test/test_mozinfo.py | line 242, test_basic: MOZCONFIG environment variable refers to a path that does not exist: foo
TEST-PASS | /home/froydnj/src/mozilla-central-official/python/mozbuild/mozbuild/test/test_mozinfo.py | test_fileobj
ERROR: test_basic (__main__.TestWriteMozinfo)
Traceback (most recent call last):
  File "/home/froydnj/src/mozilla-central-official/python/mozbuild/mozbuild/test/test_mozinfo.py", line 242, in test_basic
    write_mozinfo(self.f, c, {'MOZCONFIG': 'foo'})
  File "/home/froydnj/src/mozilla-central-official/python/mozbuild/mozbuild/mozinfo.py", line 95, in write_mozinfo
    build_conf = build_dict(config, env)
  File "/home/froydnj/src/mozilla-central-official/python/mozbuild/mozbuild/mozinfo.py", line 30, in build_dict
    the_mozconfig = mozconfig.MozconfigLoader(config.topsrcdir).find_mozconfig(env)
  File "/home/froydnj/src/mozilla-central-official/python/mozbuild/mozbuild/mozconfig.py", line 102, in find_mozconfig
    'does not exist: ' + env_path)
MozconfigFindException: MOZCONFIG environment variable refers to a path that does not exist: foo

What's the preferred way to handle this?  Screwing around with pathnames here seems...
unfortunate.
Attachment #830333 - Flags: feedback?(gps)
Comment on attachment 830333 [details] [diff] [review]
thread mozinfo.build_dict's env through to MozconfigLoader.find_mozconfig

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

We've historically had problems with testing environment detection in release automation. See e.g. bug 853954. This patch /might/ make things nicer.

It sounds like you'll need to create a temp directory in the test. See e.g. test_base.py.
Attachment #830333 - Flags: feedback?(gps) → feedback+
OK, so we have some semblance of a proper test now that passes locally.  I think it
might even pass on Windows, but I'm going to do a try run for that.

The one slightly sketchy thing about this is that the test behavior is changed:
before, we had a relative MOZCONFIG path and tested that the returned mozconfig
was absolute.  Now we have an absolute MOZCONFIG path; a relative path fails because
mozinfo checks for the existence of MOZCONFIG relative to the current working
directory.

I have taken the path of least resistance.  Comments as to whether this is desirable
or not welcome.
Attachment #829476 - Attachment is obsolete: true
Attachment #830333 - Attachment is obsolete: true
Attachment #830853 - Flags: review?(gps)
Comment on attachment 830853 [details] [diff] [review]
make mozinfo use MozconfigLoader to locate the mozconfig

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

I'm not concerned about the changed test behavior.

Please be sure this clears try before landing.
Attachment #830853 - Flags: review?(gps) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/19895f599868

Green try on three major platforms: https://tbpl.mozilla.org/?tree=Try&rev=8a9187440065
Flags: in-testsuite- → in-testsuite+
Re-pushed given a totally green try build:

https://tbpl.mozilla.org/?tree=Try&rev=cd3ba3891ccc

Took philor's advice and touched the CLOBBER file:

https://hg.mozilla.org/integration/mozilla-inbound/rev/f9d83ec11ca9
Despite not reproducing on try, the backout-causing bustage did reproduce on my
Windows laptop.  Apparently the version of Python on try will DTRT with
mixed-separator paths like "/tmp\\blahblah", whereas other versions will not.

The fix I've adopted is to just use tempfile.tempdir as the directory for our
temporary mozconfig.  dir= is only ever used in-tree with NamedTemporaryFile in
this patch, so presumably that's why we're only seeing this come up here.

This version tests clean on my Windows box and Linux box; ideally it will go
through on Try as well and we can lay this bug to rest.
Attachment #830853 - Attachment is obsolete: true
Attachment #8370213 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/0234c1bf4ea0
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
I tend to set MOZCONFIG to the name of a file in my srcdir:

  export MOZCONFIG=.mozconfig-foo
  ./mach build

Or even:

  MOZCONFIG=.mozconfig-foo ./mach build

After pulling from upstream this morning, builds on my Mac are failing with:

 0:04.01 creating ./config.status
 0:04.14 Traceback (most recent call last):
 0:04.14   File "./config.status", line 902, in <module>
 0:04.14     config_status(**args)
 0:04.14   File "/Users/myk/Mozilla/gecko-dev/python/mozbuild/mozbuild/config_status.py", line 81, in config_status
 0:04.14     write_mozinfo(os.path.join(topobjdir, 'mozinfo.json'), env, os.environ)
 0:04.14   File "/Users/myk/Mozilla/gecko-dev/python/mozbuild/mozbuild/mozinfo.py", line 99, in write_mozinfo
 0:04.14     build_conf = build_dict(config, env)
 0:04.14   File "/Users/myk/Mozilla/gecko-dev/python/mozbuild/mozbuild/mozinfo.py", line 30, in build_dict
 0:04.14     the_mozconfig = mozconfig.MozconfigLoader(config.topsrcdir).find_mozconfig(env)
 0:04.14   File "/Users/myk/Mozilla/gecko-dev/python/mozbuild/mozbuild/mozconfig.py", line 102, in find_mozconfig
 0:04.14     'does not exist: ' + env_path)
 0:04.14 mozbuild.mozconfig.MozconfigFindException: MOZCONFIG environment variable refers to a path that does not exist: .mozconfig-droid-opt

Perhaps that's a regression from this fix?
Blocks: 969085
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.