fix(jest-changed-files): avoid crashing if the sl command is taken by Steam Locomotive instead of Sapling

[mrazauskas]

Fixes #14046

Summary

Just an idea. If the sl command is taken by Steam Locomotive, it takes some 5-6 seconds to run its CLI animation. Adding timeout to Execa's options kills the process earlier, hence the getRoots() logic ends up in catch and returns null.

Unfortunately the timeout time is a penalty for those who have Steam Locomotive installed. For them --watch or --onlyChanged will be pause for that amount of time. So the number cannot be too large, but should be just enough to get response from Sapling. I don’t know Sapling at all. Seemed like the response was rather instant running sl root in a repo with Sapling enabled.

Test plan

Tested locally. All tests pass with Sapling installed; no unexpected crash with Steam Locomotive installed.

[mrazauskas]

This is just a hack unfortunately. The idea of 250ms penalty is not nice at all.

Perhaps next major could ship with scm: 'git' | 'mercurial' | 'sapling', which would default to 'git'? That would fix this problem nicely. Also this way it would be enough to call getRoots() just once. Now it is called three times for Git, for Mercurial and for Sapling separately for each run in watch mode. These calls are expansive, because Execa is spawning a process every time.

Removing this auto-detect behaviour would be a breaking change. Adding a TODO comment is all what I could do now (;

[SimenB]

I think we need a detection for what the command is. Super unfortunate steam locomotive does not have a version command.

I see we can do e.g. man sl | grep 'cure your' and see it's sl. Can we do something like that and bail? Sapling doesn't have a man page on my machine.

Would be nice with some guidance from Sapling folks.

[SimenB]

It might make sense for us to attempt to detect if we're in a VCS repo. Walk up the disk (in normalize, so once per run) and look for .git, .hg and .sl (or whatever the directories are named). I'm not a fan of requiring people to configure it.

[tvararu]

I see we can do e.g. man sl | grep 'cure your' and see it's sl.

@SimenB the man pages can be different between operating systems. My man page doesn't contain that string, see below.

$ man sl

SL(1)                           General Commands Manual                          SL(1)
NAME

sl  -  display  animations aimed to correct users who accidentally enter sl in‐

stead of ls.
SYNOPSIS

sl [ -acdeFlw ]
DESCRIPTION

sl Displays animations aimed to correct users who accidentally enter sl instead

of ls.  SL stands for Steam Locomotive.
   -a     An accident is occurring. People cry for help.

   -c     C51 appears instead of D51.

   -d     Disco mode. Alternate colors during animation.

   -e     Escape. Allow interrupt by Ctrl+C.

   -F     It flies like the galaxy express 999.

   -G     It's French like Oscar.

   -c     C51 appears instead of D51.

   -<number of cars>
          Specify the number of cars like `sl -5`.

   -l     Little version.

   -w     Windy day. Locomotive moves faster.

   -v     Prints the version and last updated.

SEE ALSO

ls(1)
BUGS

It sometimes lists directory contents.
AUTHOR

Toyoda Masashi (mtoyoda@acm.org)
                         March 19, 2019 - Version 5.04                       SL(1)

[vegerot]

As I said in #14046 (comment) , we could check the size of the sl binary. That will be a good indicator of what it is. We will have to resolve symlinks, etc.

[mrazauskas]

Closing. It does not look right.

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.