Closed Bug 1584567 Opened 2 months ago Closed Last month

Remove support for conditional names


(Firefox Build System :: Mach Core, task)

Not set


(firefox71 fixed)

Tracking Status
firefox71 --- fixed


(Reporter: ahal, Assigned: ahal)




(2 files)


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:

In 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.

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.

Pushed by
Merge desktop + android commands (run and install) back together, r=nalexander
[mach] Remove support for conditional_names, r=nalexander
Closed: Last month
Resolution: --- → FIXED
Target Milestone: --- → mozilla71
Regressions: 1585828
You need to log in before you can comment on or make changes to this bug.