Closed Bug 1277087 Opened 4 years ago Closed 4 years ago

in-tree mach should be imported by default

Categories

(Firefox Build System :: Mach Core, enhancement)

enhancement
Not set

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.
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
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
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.
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
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/
Attachment #8758612 - Flags: review?(gps)
Assignee: nobody → mh+mozilla
Flags: needinfo?(mh+mozilla)
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+
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
https://hg.mozilla.org/mozilla-central/rev/b6db2e11244e
https://hg.mozilla.org/mozilla-central/rev/ba695fcbedc8
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Depends on: 1277841
Depends on: 1278415
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.