Remove support for conditional names
Categories
(Firefox Build System :: Mach Core, task)
Tracking
(firefox71 fixed)
| Tracking | Status | |
|---|---|---|
| firefox71 | --- | fixed |
People
(Reporter: ahal, Assigned: ahal)
References
Details
Attachments
(2 files)
From https://bugzilla.mozilla.org/show_bug.cgi?id=1521996#c18:
I know I reviewed this and should have thought of this back then, but I was doing some debugging in bug 1580280 and realized this causes us to invoke configure before mach even parses the command line. So e.g, every mach invocation ever is going into configure.
So I just wanted to point out that mach supports a callable parser, so it would be totally possible to have different command line arguments for Android vs Desktop in a single mach implementation (as well as branching in the implementation itself). In fact, mochitest's ArgumentParser has this exact condition already:
https://searchfox.org/mozilla-central/rev/f1e99da78fe6c3c68696358dac06aed90f8112d3/testing/mochitest/mach_commands.py#212In other words, we could scrap conditional_names entirely and re-work the install and run commands to have different arguments and branches depending on whether we have an Android build or not. Wish I thought of this during review.
Anyway, it's not a super high priority or anything.. but I think it would be good to avoid using more conditional_names in the future.
|
Assignee | |
Comment 1•5 years ago
•
|
||
So turns out I was wrong. This doesn't cause us to invoke configure and it has nothing to do with bug 1580280.
But before I realized this, I went ahead and implemented the patches to fix it. I'd still prefer to get them landed as I think conditional_names can be problematic (breaks assumptions by changing the names of commands at runtime). Mach is simpler and cleaner without them.
However, these patches aren't solving any current problems. So if they cause problems or pose risk I'm happy to abandon them.
|
|
Assignee | |
Comment 2•5 years ago
|
||
|
|
Assignee | |
Comment 3•5 years ago
|
||
Depends on D47626
Pushed by ahalberstadt@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b50c4cdef2ac Merge desktop + android commands (run and install) back together, r=nalexander https://hg.mozilla.org/integration/autoland/rev/4fbb6d144880 [mach] Remove support for conditional_names, r=nalexander
|
||
Comment 5•5 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/b50c4cdef2ac
https://hg.mozilla.org/mozilla-central/rev/4fbb6d144880
Description
•