1518572 - Overhaul |mach try| preset mechanism

Status: RESOLVED FIXED

(Developer Infrastructure :: Try, enhancement, P1)

Description

[Andrew Halberstadt [:ahal]]

With many |mach try| subcommands, you can save and load presets. E.g:

$ ./mach try fuzzy --save foobar -q "'some 'query"
$ ./mach try fuzzy --preset foobar

However, there are a lot of problems with this system. Most of them stem from the fact that the storage backend isn't adequate enough. It's a key/value store in an ini file (with subcommand names as the section headers). This makes it hard to save things that require more than one value.

Instead, I propose we use a format as follows (using yaml in the example):

foo:
    subcommand: fuzzy
    args:
        queries:
            - 'some 'query
            - 'someotherquery
        artifact: false
        env:
            CUSTOM_ENV: value
bar:
    subcommand: syntax
    args:
        syntax: -b o -p linux -u mochitest
        ...

This simply stores the subcommand to run, and the arguments to pass into that subcommand. This makes it flexible enough to store as much (or as little) information along with the preset as desired.

Another benefit is that subcommands don't have to worry about loading presets at all. We could even restrict the ability to use --preset unless passed to the root |mach try| command. We could possibly even handle --save outside of the subcommands (though I'm on the fence about this, because it would mean no --save with fuzzy interactive mode).

Completing this task will allow us to fix several outstanding preset bugs, as well as provide a lot of new goodness.

Comment 2

[Andrew Halberstadt [:ahal]]

Status update: this is mostly done. Just need to update some tests/documentation and get it reviewed. Hopefully landed in the next week or two.

Comment 4

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1518572 - [mach] Stuff the 'handler' instance into the context when applicable

For mach commands that have 'pass_context=True', we should implicitly add the
handler instance to the context. This will give mach command implementations an
easy way to access things like the command/subcommand names, the parser, argv
list, etc.

Comment 5

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1518572 - [tryselect] Remove unused 'flask-wtf' dependency from |mach try chooser|

I forgot to remove this after re-implementing without this dependency.

Depends on D20521

Comment 6

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1518572 - [tryselect] Store all arguments when saving a preset

Depends on D20522

Comment 7

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1518572 - [tryselect] Migrate old preset files to the new format

Depends on D20523

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9afd71742756
[mach] Stuff the 'handler' instance into the context when applicable r=firefox-build-system-reviewers,mshal
https://hg.mozilla.org/integration/autoland/rev/d7a45b84e063
[tryselect] Remove unused 'flask-wtf' dependency from |mach try chooser| r=gbrown
https://hg.mozilla.org/integration/autoland/rev/1b5a4da48de3
[tryselect] Store all arguments when saving a preset r=gbrown
https://hg.mozilla.org/integration/autoland/rev/24105ffcf6f5
[tryselect] Migrate old preset files to the new format r=gbrown

Comment 9

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/9afd71742756
https://hg.mozilla.org/mozilla-central/rev/d7a45b84e063
https://hg.mozilla.org/mozilla-central/rev/1b5a4da48de3
https://hg.mozilla.org/mozilla-central/rev/24105ffcf6f5

Comment 10

[Andrew Halberstadt [:ahal]]

Sadly there was a bug in the preset migration despite the test. If you are here because of warnings about presets not being migrated, please see:
https://bugzilla.mozilla.org/show_bug.cgi?id=1530775#c3