Closed Bug 1384593 Opened 2 years ago Closed 2 years ago

Create a fuzzy try selector

Categories

(Testing :: General, enhancement)

enhancement
Not set

Tracking

(firefox56 fixed)

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: ahal, Assigned: ahal)

References

Details

Attachments

(2 files)

Create a try selector based on fzf:
https://github.com/junegunn/fzf
Here's a demo of this in action:
https://ahal.ca/static/vid//blog/2017/mach-fuzzy.mp4
If you want to try this out locally, you'll need the latest version-control-tools:
./mach mercurial-setup --update

I tested this on Windows, and with git-cinnabar.
Comment on attachment 8890488 [details]
Bug 1384593 - Abstract version control functionality out of syntax.py to vcs.py,

https://reviewboard.mozilla.org/r/161632/#review168422

What a nice clean up. Land this as long as you intended to have a constructor to set self.root to root.

::: tools/tryselect/vcs.py:43
(Diff revision 3)
> +class VCSHelper(object):
> +    """A abstract base VCS helper that detects hg or git"""
> +    __metaclass__ = ABCMeta
> +
> +    def __init__(self, root):
> +        self.root = root

What is self.root for?
Attachment #8890488 - Flags: review?(armenzg) → review+
Comment on attachment 8890488 [details]
Bug 1384593 - Abstract version control functionality out of syntax.py to vcs.py,

https://reviewboard.mozilla.org/r/161632/#review168422

> What is self.root for?

Good catch! That got copied from:
https://dxr.mozilla.org/mozilla-central/source/python/mozlint/mozlint/vcs.py#13

and isn't being used here. I'm still tempted to leave it in though, just because we're already spending time running the `hg` or `git` command to detect the root and it could be useful in the future.

That said if you prefer I remove it, I'd be happy to do so.
Comment on attachment 8892076 [details]
Bug 1384593 - Add an fzf based fuzzy try selector,

https://reviewboard.mozilla.org/r/163092/#review168426

No issues preventing landing. Pick and choose what needs addressing and what not.

Fantastic job. Looking forward to using this.

::: tools/tryselect/mach_commands.py:71
(Diff revision 1)
>          kwargs = vars(parser.parse_args(args))
>          return self._mach_context.commands.dispatch(
>              'try', subcommand='syntax', context=self._mach_context, **kwargs)
>  
>      @SubCommand('try',
> +                'fuzzy',

Is "fuzzy" going to be the final name?

For me when I think of this topic I'm inclined to think of what I want to happen ("select tasks" or "define what to schedule") rather than the matching algorithm ('fuzzy finding').

*I* would have named it 'task-selection'. I'm not strongly tied to my opinion.

I might be thinking this too much because English is not my first language.

::: tools/tryselect/selectors/fuzzy.py:176
(Diff revision 1)
> +def run_fuzzy_try(update):
> +    fzf = fzf_bootstrap(update)
> +
> +    if not fzf:
> +        print(FZF_NOT_FOUND)
> +        return

No need for sys.exit()?

::: tools/tryselect/tasks.py:25
(Diff revision 1)
> +def invalidate(cache):
> +    if not os.path.isfile(cache):
> +        return
> +
> +    tc_dir = os.path.join(build.topsrcdir, 'taskcluster')
> +    tmod = max(os.path.getmtime(os.path.join(tc_dir, p)) for p, _ in FileFinder(tc_dir))

Clever.

It would be cool if our checkouts had a faster way to determine if anything has changed since last time.

Could there be an artifact we can fetch from the gecko decision task?
Or only check the files that have change since we branched?

This might be a candidate for a follow up project.
Attachment #8892076 - Flags: review?(armenzg) → review+
Comment on attachment 8892076 [details]
Bug 1384593 - Add an fzf based fuzzy try selector,

https://reviewboard.mozilla.org/r/163092/#review168426

Thanks for the reviews, I'm looking forward to getting this out there for more feedback too!

> Is "fuzzy" going to be the final name?
> 
> For me when I think of this topic I'm inclined to think of what I want to happen ("select tasks" or "define what to schedule") rather than the matching algorithm ('fuzzy finding').
> 
> *I* would have named it 'task-selection'. I'm not strongly tied to my opinion.
> 
> I might be thinking this too much because English is not my first language.

That's a good point, but I think the name needs to describe *how* the tasks are being selected. That's because all of the `try selectors` (right now just `syntax` and `fuzzy`, but in my vision there could be 4-5 of them eventually), are trying to accomplish that goal you stated of "defining what to schedule".

E.g `task-selection` could refer to either the fuzzy finder or try syntax. And it will get more confusing if we add a curses selector + regex/glob selector + a mozbuild.testing.TestResolver based selector. They're all trying to select tasks in various ways.

> No need for sys.exit()?

Hm, yeah I guess my use of sys.exit() vs return is pretty inconsistent. It was mostly to avoid checking return codes. I'll see if I can clean it up a bit.

> Clever.
> 
> It would be cool if our checkouts had a faster way to determine if anything has changed since last time.
> 
> Could there be an artifact we can fetch from the gecko decision task?
> Or only check the files that have change since we branched?
> 
> This might be a candidate for a follow up project.

While this is running on a lot of files, it's actually surprisingly fast. I'm not even sure that downloading an artifact would be an improvement.

We could do this using build backends (i.e re-use the thing that the makefiles do to detect if the build configuration is out of date), but I think that means we'd need some kind of taskcluster -> build integration. It would be a pretty big project.
Pushed by ahalberstadt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ac8b6cc020b9
Abstract version control functionality out of syntax.py to vcs.py, r=armenzg
https://hg.mozilla.org/integration/autoland/rev/9d617ec5226c
Add an fzf based fuzzy try selector, r=armenzg
https://hg.mozilla.org/mozilla-central/rev/ac8b6cc020b9
https://hg.mozilla.org/mozilla-central/rev/9d617ec5226c
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Blocks: 1389366
Depends on: 1394391
You need to log in before you can comment on or make changes to this bug.