Closed
Bug 1277087
Opened 8 years ago
Closed 8 years ago
in-tree mach should be imported by default
Categories
(Firefox Build System :: Mach Core, enhancement)
Firefox Build System
Mach Core
Tracking
(firefox49 fixed)
RESOLVED
FIXED
mozilla49
Tracking | Status | |
---|---|---|
firefox49 | --- | fixed |
People
(Reporter: rhelmer, Assigned: glandium)
References
Details
Attachments
(2 files)
In https://dxr.mozilla.org/mozilla-central/source/build/mach_bootstrap.py?from=mach_bootstrap.py#238-240 `mach.main` is only imported from the in-tree location if a plain `import mach.main` fails. I ran into this because I happened to have an older mach installed in `site-packages` (by accident, meant for it to be in a virtualenv), and Glandium says this caused ESR build failures recently too. mach should just import from the in-tree location.
Assignee | ||
Comment 1•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/56742/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/56742/
Attachment #8758484 -
Flags: review?(gps)
Comment 2•8 years ago
|
||
Comment on attachment 8758484 [details] MozReview Request: Bug 1277087 - Always add in-tree search paths when bootstrapping mach. r=gps https://reviewboard.mozilla.org/r/56742/#review53462 For posterity and as I typed in IRC earlier, this code stems from the initial check-in of mach. Back then, mach was mozilla-central specific. Later, we added support for mach being a generic Python package. At that time we should have removed this code and always set the local path on sys.path first. But we didn't. This change is obviously correct given that mach can be present on sys.path.
Attachment #8758484 -
Flags: review?(gps) → review+
Pushed by mh@glandium.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/b541ce3cbff5 Always add in-tree search paths when bootstrapping mach. r=gps
Comment 4•8 years ago
|
||
seems this caused issues like https://treeherder.mozilla.org/logviewer.html#?job_id=29164386&repo=mozilla-inbound and was backed out
Flags: needinfo?(mh+mozilla)
Backout by cbook@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/b6fe7fa88db0 Backed out changeset b541ce3cbff5 for ab build failures
Assignee | ||
Comment 6•8 years ago
|
||
So the funny thing is that this makes the buildconfig module in the tree get picked instead of the one in the virtualenv, which makes config.status not found. This effectively breaks things in subtle ways... I don't remember if we have any use of the buildconfig module from the source tree. A way out might be to move the file in a directory that is not in virtualenv_packages.txt.
Assignee | ||
Comment 7•8 years ago
|
||
So far, we relied on the module being copied over in the virtualenv, and the module itself would try to find config.status in parent directories of its own location. Unfortunately, this falls short when the source tree's build/ directory appears early in the sys.path. With this change, we don't copy the module to the virtualenv anymore, and try to find config.status in parent directories of the python executable, which, when running from the virtualenv, will be equivalent to the current behavior. Review commit: https://reviewboard.mozilla.org/r/56866/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/56866/
Attachment #8758484 -
Attachment description: MozReview Request: Bug 1277087 - Always add in-tree search paths when bootstrapping mach. r?gps → MozReview Request: Bug 1277087 - Always add in-tree search paths when bootstrapping mach. r=gps
Assignee | ||
Comment 8•8 years ago
|
||
Comment on attachment 8758484 [details] MozReview Request: Bug 1277087 - Always add in-tree search paths when bootstrapping mach. r=gps Review request updated; see interdiff: https://reviewboard.mozilla.org/r/56742/diff/1-2/
Assignee | ||
Updated•8 years ago
|
Attachment #8758612 -
Flags: review?(gps)
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → mh+mozilla
Flags: needinfo?(mh+mozilla)
Comment 9•8 years ago
|
||
Comment on attachment 8758612 [details] MozReview Request: Bug 1277087 - Change how the buildconfig module searches for config.status https://reviewboard.mozilla.org/r/56866/#review53638 I never liked how we treated buildstatus. Be warned, there is some wonky behavior with regards to this module. I wouldn't be surprised if this broken random things. But don't let my FUD prevent you from landing.
Attachment #8758612 -
Flags: review?(gps) → review+
Comment 10•8 years ago
|
||
Pushed by mh@glandium.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/b6db2e11244e Change how the buildconfig module searches for config.status. r=gps https://hg.mozilla.org/integration/mozilla-inbound/rev/ba695fcbedc8 Always add in-tree search paths when bootstrapping mach. r=gps
Comment 11•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b6db2e11244e https://hg.mozilla.org/mozilla-central/rev/ba695fcbedc8
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox49:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Updated•6 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•