1531364 - `mach try` fails with "error: unrecognized arguments: --try-test-paths" after "migrating saved presets from …"

Status: RESOLVED WONTFIX

(Firefox Build System :: Try, defect)

Description

[Matthew N. [:MattN]]

Attachment: autotry.ini with the issue

If I run mach try --no-push I get the following output now:

migrating saved presets from '/Users/matthew/.mozbuild/autotry.ini' to '/Users/matthew/.mozbuild/try_presets.yml'
usage: mach [-h] [-b BUILDS] [-p PLATFORMS] [-u TESTS] [-t TALOS] [-j JOBS]
[--tag TAGS] [--and] [--no-artifact] [-v] [--detect-paths]
[-m [MESSAGE]] [--no-push] [--closed-tree]
[--save SAVE | --preset PRESET | --list-presets | --edit-presets]
[--chemspill-prio] [--all-emails] [--upload-xdbs] [--interactive]
[--setenv SETENV] [--artifact] [-e]
[--rebuild-talos REBUILD_TALOS] [-f] [--rebuild REBUILD]
[--no-retry] [--failure-emails]
[paths [paths ...]]
mach: error: unrecognized arguments: --try-test-paths

It seems like the migration in bug 1518572 doesn't handle --try-test-paths in the preset. I personally find saving the path to be the most useful feature of presets. If the path could be saved in a non-expanded form that would be even better as it wouldn't get stale.

Comment 1

[Andrew Halberstadt [:ahal]]

It looks like this errors out before finishing the migration, which is a good thing as it means there isn't any data loss! To clarify, ~/.mozbuild/autotry.ini still exists after this happens right?

This is happening because the --try-test-paths argument is kind of a big hack in that it doesn't get parsed by the try syntax parser and instead ends up in an environment variable that gets read in mozharness:
https://searchfox.org/mozilla-central/source/testing/mozharness/mozharness/mozilla/testing/try_tools.py#177

That's why they weren't migrated; the migration tool uses the TrySyntax parser to load the string. We'll need to parse these paths out manually.

To clarify, the new preset format is perfectly capable of saving paths, this is just a bug in the migration.

Comment 2

[Matthew N. [:MattN]]

(In reply to Andrew Halberstadt [:ahal] from comment #1)

It looks like this errors out before finishing the migration, which is a good thing as it means there isn't any data loss! To clarify, ~/.mozbuild/autotry.ini still exists after this happens right?

Yes.

This is happening because the --try-test-paths argument is kind of a big hack in that it doesn't get parsed by the try syntax parser and instead ends up in an environment variable that gets read in mozharness:
https://searchfox.org/mozilla-central/source/testing/mozharness/mozharness/mozilla/testing/try_tools.py#177

That's why they weren't migrated; the migration tool uses the TrySyntax parser to load the string. We'll need to parse these paths out manually.

OK, I figured it was just the migration that didn't handle them.

To clarify, the new preset format is perfectly capable of saving paths, this is just a bug in the migration.

Awesome!

Comment 3

[Andrew Halberstadt [:ahal]]

If you need to use |mach try| asap and can't wait for the patch, the easy workaround is to mv ~/.mozubild/autotry.ini to a different location so the migration doesn't get triggered.

If you need your presets in the meantime, you can migrate them manually by copy/pasting the preset into |mach try| and removing the --try-test-paths arg, then adding --save <name>. E.g:

$ ./mach try --save <name> <copied syntax minus paths> <path one> <path two>

Once ~/.mozbuild/try_presets.yml exists, the migration will cease to be triggered, so you'll need to migrate all your presets this way.

I'll try to get this fixed soon though.

Comment 4

[Andrew Halberstadt [:ahal]]

$ ./mach try --save <name> <copied syntax minus paths> <path one> <path two>

Also make sure <path one> etc omits the <suite>: at the start. It should just be the bare relative paths in mozilla-central.

Comment 5

[Andrew Halberstadt [:ahal]]

Hm, after looking into this a bit more, this is going to be really hard to solve perfectly. The problem is that try syntax flags get parsed all over the place (in taskgraph as well as mozharness), and |mach try syntax| ignores all of these values:
https://searchfox.org/mozilla-central/rev/00f3836a87b844b5e4bc82f698c559b9966e4be2/tools/tryselect/selectors/syntax.py#92

Basically all the flags in that list won't be migrated properly, and parsing them will be a big hack. I'm kind of tempted to just help people manually migrate their presets if they run into problems rather than bend over backwards trying to get the migration perfect. As more time passes, the return on fixing this diminishes.

I'll spend a bit more time though, maybe I'll think up an easier way to fix this.

Comment 6

[Matthew N. [:MattN]]

FWIW, I only have 1 saved preset affected so manual migration is fine with me… not sure how many others are affected.

Comment 7

[Andrew Halberstadt [:ahal]]

I have a pseudo solution, but I keep finding various edge cases that fail every time I fix something. Let's leave this open for now, and if anyone runs into this problem please feel free to send me your ~/.mozbuild/autotry.ini and I'll manually create a try_presets.yml for you to get you on your way. I have a feeling that will be far less time consuming.

If there aren't any other complaints in the coming days/weeks, I'll resolve.

Comment 8

[Andrew Halberstadt [:ahal]]

Haven't seen any complaints, and I think enough time has passed that most people should have hit the migration by now so I don't think this is worth spending more time solving properly.

If anyone hits this, feel free to ping me and I'll help you get migrated manually.