1657650 - Make `mach` commands inherit from `MachCommandBase`

Status: RESOLVED FIXED

(Firefox Build System :: General, task)

Comment 1

[Ricky Stewart]

Attachment: Bug 1657650 - Require that Mach command providers subclass MachCommandBase.

Today we don't require that mach CommandProviders subclass from any particular parent class and we're very lax about the requirements they must meet. While that's convenient in certain circumstances, it has some unfortunate implications for feature development.

Today the only requirements that we have for CommandProviders are that they have an __init__() method that takes either 1 or 2 arguments, the second of which must be called context and is populated with the mach CommandContext. Again, while this flexibility is occasionally convenient, it is limiting. As we add features to mach, having a better idea what the shape of our CommandProviders are and how we can instantiate them and use them is increasingly important, and this gives us additional control when having mach configure CommandProviders based on data that is only available at the mach level. In particular, we plan to leverage this in bugs 985141 and 1654074.

Here we add validation to the CommandProvider decorator to ensure all classes inherit from MachCommandBase, update all CommandProviders in-tree to inherit from MachCommandBase, and update source and test code accordingly.

Follow-up work: we now require (de facto) that the context be populated with a topdir attribute by the populate_context_handler function, since instantiating the MachCommandBase requires a topdir be provided. This is fine for now in the interest of keeping this patch reasonably sized, but some additional refactoring could make this cleaner.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/80852dff4c7f
Require that Mach command providers subclass MachCommandBase. r=remote-protocol-reviewers,marionette-reviewers,maja_zf,mhentges,froydnj

Comment 3

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/80852dff4c7f

Comment 4

[Mitchell Hentges [:mhentges] 🦀]

Comment on attachment 9168579 [details]
Bug 1657650 - Require that Mach command providers subclass MachCommandBase.

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Needed to uplift https://phabricator.services.mozilla.com/D86256.
  This patch allows all mach commands to have the same parameters, and D86256 uses that to specify a "virtualenv_name" parameter.
• User impact if declined: None
• Fix Landed on Version: 81
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): It's a build-only patch
• String or UUID changes made by this patch:

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9168579 [details]
Bug 1657650 - Require that Mach command providers subclass MachCommandBase.

Needed to better support running mach on newer macOS releases. Approved for 78.8esr.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr78/rev/48d786b062f9