Fix scripts cmd slashes for windows

[wclr]

Summary
This address the issue #1729 which as about that on windows yarn fails to execute scripts that contain commands with unix-style slashes, for example this will fail on windows:

"scripts": {
  "compile": "../node_modules/.bin/tsc"
}

So to run such script on windows machine successfully we need to replace slashes from / to \,

  "compile": "..\\node_modules\\.bin\\tsc"

For we need to find all the executable commands in the script line, so we assume that exec commands that need to be fixed:

• Are located in the beginning of the line
• Or located after symbols: && or & or || or |
• && or & or || or | symbols after which command go may not be inside the quotes. In opposite case this will mean that it is inside the nested command like:
  docker run container bash -c "cat some.thing | some/bin/here"
• Can be inside of single or double quotes (may contain spaces)
• Can be without quotes (no spaces)
• In the substrings that satisfies upper criteria all backslashes / are replaced with windows slashes \

Test plan

Added test that check correct transformation for different command cases.

So this need some triage and opinions.

[Daniel15]

Internally, Windows actually supports both \ and /, so the original path should be fine. For example, try running c:/windows/system32/calc.exe in either the run dialog or a command prompt - It'll work fine. Because of this, I'm wondering if the problem is actually elsewhere. I feel like we shouldn't need to do this.

Does it fail in npm too?

[Daniel15]

Ah I see, it only fails when there's a dot at the start

C:\Windows>./system32/calc.exe
'.' is not recognized as an internal or external command, operable program or batch file.

Absolute paths work fine. I wonder if we could resolve the path before running the command. Node.js should have a built-in path method for that.

[wclr]

Yes this actually fails for relative paths, and as mostly exec paths in scripts are relative. Yes NPM has the same the old issue. Windows users seem to have a big tolerance (which is not very good).

So I think it should be fixed in a proper way. Windows people are complaining in different repos, but it is still not fixed in years.

Node.js should have a built-in path method for that.

Well node path, has normalization method, which replaces slashes, but we should not replace all the spashes in the script line. We should only do it for a exec commands. So my proposed approach to find such commands in a script line need to be evaluated by some others. I think, I could make a PR to npm to have parallel evaluation process.

[Daniel15]

This diff is pretty complex, but the tests seem comprehensive. Let's ship it!

[ctaggart]

I just tested on Windows with the latest nightly build and this is still not working for me:

yarn install v0.20.0-20170107.0149
info No lockfile found.
[1/4] Resolving packages...
warning mocha > glob > minimatch@0.3.0: Please update to minimatch 3.0.2 or higher to avoid a RegExp DoS issue
warning mocha > to-iso-string@0.0.2: to-iso-string has been deprecated, use @segment/to-iso-string instead.
warning mocha > jade@0.26.3: Jade has been renamed to pug, please install the latest version of pug instead of jade
[2/4] Fetching packages...
warning fsevents@1.0.17: The platform "win32" is incompatible with this module.
info "fsevents@1.0.17" is an optional dependency and failed compatibility check. Excluding it from installation.
[3/4] Linking dependencies...
[4/4] Building fresh packages...
success Saved lockfile.
$ ./node_modules/.bin/cpx ./node_modules/fixed-data-table/dist/*.css ../Assets/fixed-data-table
'.' is not recognized as an internal or external command,
operable program or batch file.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.

It just needs to run the cpx command.

that is in packages.json like so:

  "scripts": {
    "postinstall": "./node_modules/.bin/cpx ./node_modules/fixed-data-table/dist/*.css ../Assets/fixed-data-table",

[Daniel15]

@ctaggart - You should be able to omit the node_modules/.bin from scripts. Try this:

"postinstall": "cpx ./node_modules/fixed-data-table/dist/*.css ../Assets/fixed-data-table"

[ctaggart]

@Daniel15 Thank you! That worked great with nightly v0.20.0-20170107.0149 and the latest stable release 0.18.1.

[Daniel15]

😃 By the way, npm has the same behaviour - It'll look in node_modules/.bin by default, so you don't need to explicitly include it in your command

[wclr]

Btw I would recommend on windows 10 to completely migrate development process to "Ubuntu bash for windows".