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

Launching "shell" commands on windows #4100

Closed
byrney opened this issue Jan 7, 2019 · 2 comments
Closed

Launching "shell" commands on windows #4100

byrney opened this issue Jan 7, 2019 · 2 comments
Labels

Comments

@byrney
Copy link

byrney commented Jan 7, 2019

OS: Debian 9, windows 8/10, OSX
PM2: 3.2.4

I have a fork with a work in progress branch that solves some minor issues running
scripts on windows and linux.

We run a mixture of windows and linux on the desktop. I am trying to use pm2
to run some processes in dev mode. One of these is webpack -d --watch.

After an npm install webpack gets set up like this:

app/
   node_modules/
      .bin/
         webpack              # this is a shell script
         webpack.cmd          # windows cmd file

It's a fairly common pattern (git-windows, ruby etc) follow to make the
whatever command work on both platforms. Linux will find the shell script,
windows will find (and prefer) the cmd so when you issue webpack the right
one gets run.

To make things a little harder, webpack is installed in the root of the app but
needs to be called from sub-folders. We get around this with yarn run which
does the same as npm run and puts the right node_modules/.bin on the path to
find the command.

I tried this:

{
    script: 'yarn',
    args: [ 'webpack', '-d', '--watch'],
    interpreter: 'none'
}

But it fails to start webpack because it tries to spawm: C:\PROGRAM FILES (X86)\YARN\BIN\YARN.CMD
which triggers a C:\PROGRAM not found because it's not correctly quoted/escaped.

I tried:

{
    script: 'yarn webpack',
    args: [ '-d', '--watch'],
    interpreter: 'none'
}

and

{
    script: '"c:/program files (x86)/yarn/bin/yarn"',
    args: ['weback', '-d', '--watch'],
    interpreter: 'none'
}

But both give an error 'bash and sh not available in $PATH' from Common.js
because there are spaces in the command. Much like #3767 which is closed for
the specific case of backslash but not in general.

I tried a few more and hit #2037 trying to launch the cmd directly.

Using the full path to the webpack main js file works
../node_modules/webpack/bin/webpack.js and since the pm2 config is js, I could
write code to export the right structure based on os.platform so I think there
are solutions without changing pm2.

I looked though the code and figured we could make
spawn use the shell itself which makes commands with spaces cross platform
(#3767), allows it to work on windows with scripts that are not js (e.g
git-windows, ruby) and (I think) will solve launching a .cmd file as well
(#2037).

This does remove the ipc handle when the script has interpreter 'none',
but AFAIK the ipc is only supported when the child is node see node docs.

The results of my investigation are here:
https://github.com/CityScience/pm2/pull/1/files.

Before I go further, I wanted to ask:

  1. Is this a positive change to make?
  2. If so, should it apply to 'none' or add a new interpreter option so that
    none remains backward compatible?
  3. Aside from adding some tests and updating the docs is there anything else?
@vmarchaud
Copy link
Contributor

Hey,

I reviewed the PR, i sadly don't have a windows box to test it, but i mostly want to make sure we do not break anything with those changes.
To answer your question:

  • yes every changes is welcome.
  • we don't have a major bump scheduled right now so it would be better to find a solution that doesn't force us to do one.
  • the doc is available there https://github.com/keymetrics/doc-pm2, and no i don't think there anything more the changes.

For the solution itself, could we just handle the spawning of the cmd under windows
to resolve everything for us ? Which i believe is the only problem there

@stale
Copy link

stale bot commented May 24, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label May 24, 2020
@stale stale bot closed this as completed Jun 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants