Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: platform specific run scripts #122

Open
2ZeroSix opened this issue Jun 4, 2021 · 5 comments
Open

Proposal: platform specific run scripts #122

2ZeroSix opened this issue Jun 4, 2021 · 5 comments
Labels
enhancement New feature or request question Further information is requested

Comments

@2ZeroSix
Copy link
Contributor

2ZeroSix commented Jun 4, 2021

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

@Salakar
Copy link
Contributor

Salakar commented Jun 4, 2021

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 Salakar added question Further information is requested enhancement New feature or request labels Jun 4, 2021
@2ZeroSix
Copy link
Contributor Author

2ZeroSix commented Jun 4, 2021

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

@Salakar
Copy link
Contributor

Salakar commented Jun 4, 2021

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?

@2ZeroSix
Copy link
Contributor Author

2ZeroSix commented Jun 7, 2021

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): npm/npm#4040
issue in yarn caused by some custom escape character parsing: yarnpkg/yarn#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, ...)
  • ???

@Tokenyet
Copy link

Tokenyet commented Mar 20, 2022

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 >-.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants