melos icon indicating copy to clipboard operation
melos copied to clipboard

Proposal: platform specific run scripts

Open 2ZeroSix opened this issue 3 years ago • 5 comments

Problem:

It seems impossible to properly support multi-platform parsing of command in run section without invention of its own scripting language

For example this issue #15 is caused and cannot be easily and fully fixed because of custom cmd and sh handling

Proposal:

allow platform specific scripts

for example

scripts:
  my:command:
    run:
      windows: echo hey %USERNAME%
      unix: echo hey $USER

or

scripts:
  my:command:
    run:
      batch: path\to\script.bat
      shell: path/to/script.sh

then it would be possible to pass script in run section as-is to underlying interpreter without any custom parsing

2ZeroSix avatar Jun 04 '21 22:06 2ZeroSix

Would writing a dart script not be easier and cleaner for cross-platform execution, we have an example on the melos repo itself:

https://github.com/invertase/melos/blob/master/melos.yaml#L40 https://github.com/invertase/melos/blob/master/scripts/generate_version.dart

You could even add a pubspec.yaml at the root of your monorepo for any dev dependencies you may need for your scripts:

https://github.com/invertase/melos/blob/master/melos.yaml#L4 https://github.com/invertase/melos/blob/master/pubspec.yaml#L11-L12


I wanted to originally add something like you've mentioned before but opted not to since we can just write a cross-platform script using a single Dart file

Salakar avatar Jun 04 '21 23:06 Salakar

Thanks for nice example!

Probably for most cases dart script would be a perfect solution.

Then run section docs must properly state that it's not intended to be used for full-featured scripts. As I can see && for chains and \, ^ for line continuations are "safe" expressions

2ZeroSix avatar Jun 04 '21 23:06 2ZeroSix

We are going through and updating the docs to have more examples and information on cross-platform scripts 🙈 sorry it's not fully documented yet

Then run section docs must properly state that it's not intended to be used for full-featured scripts. As I can see && for chains and , ^ for line continuations are "safe" expressions

Is there any code improvements to Melos you would suggest we make to improve this?

Salakar avatar Jun 04 '21 23:06 Salakar

I was thinking about this over the weekend, and I can say that this problem is much more complicated than I initially expected

Here is long running issue in npm (this is what lerna use in lerna run <script> by default): https://github.com/npm/npm/issues/4040 issue in yarn caused by some custom escape character parsing: https://github.com/yarnpkg/yarn/issues/7732 and there is many more in the wild

It seems critical to choose appropriate solution as soon as possible while most of the users are early adopters

Possible solutions:

  • stick to small subset of shell script which could be unambiguously converted to a batch:
    • && or multiple lines for chains. It should behave the same way, exit on first error
    • \ for line continuations (remove ^ support to avoid confusion and simplify parser)
    • $ENV_VAR for environment variables
    • path arguments are not portable, underlying user script should be aware how to handle it.
    • recommend writing dart command as in your example for all complex use cases to avoid any ambiguity
  • Provide a way to use shell specific scripts beside default option (those shell specific scripts should be executed as is without any pre-formating)
  • use only sh and force users to install implementation of shell for windows (GitBash, WSL, ...)
  • ???

2ZeroSix avatar Jun 07 '21 10:06 2ZeroSix

I think this is a must feature for cross-platform, because I met a line break issue on windows quickly.

  test:
    run: |
      flutter test --exclude-tags "integration" && \
      flutter pub run melos exec -c 1 --fail-fast -- \
        "flutter test --no-pub --exclude-tags "integration"
    description: Run `flutter test` for a specific package.
    select-package:
      dir-exists:
        - test
  test:windows:
    run: |
      flutter test --exclude-tags "integration" && ^
      flutter pub run melos exec -c 1 --fail-fast -- \
        "flutter test --no-pub --exclude-tags "integration"
    description: Run `flutter test` for a specific package.
    select-package:
      dir-exists:
        - test

Line break is different on windows (cmd), so can't be used easily, but almost the same code.

Edit Nevermind, Line break can be resolved by changing YAML syntax from | to >-.

Tokenyet avatar Mar 20 '22 18:03 Tokenyet