1584567 - Remove support for conditional names

Status: RESOLVED FIXED

(Firefox Build System :: Mach Core, task)

Description

[Andrew Halberstadt [:ahal]]

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#212

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.

Comment 1

[Andrew Halberstadt [:ahal]]

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.

Comment 2

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1584567 - Merge desktop + android commands (run and install) back together, r?nalexander

Comment 3

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1584567 - [mach] Remove support for conditional_names, r?nalexander

Depends on D47626

Comment 4

[Pulsebot]

Pushed by [e-mail]:
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

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/b50c4cdef2ac
https://hg.mozilla.org/mozilla-central/rev/4fbb6d144880