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.

Sign up for GitHub

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

Jump to bottom

Allow arguments to be passed to npm scripts #356

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Allow arguments to be passed to npm scripts #356

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions generators/workspace/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ class Generator extends Base {
// no pre post test tasks
Object.keys(pkg.scripts).filter((s) => !/^(pre|post).*/g.test(s)).forEach((s) => {
// generate scoped tasks
scripts[`${s}:${p}`] = `cd ${p} && npm run ${s}`;
scripts[`${s}:${p}`] = `cd ${p} && npm run ${s} --`; // add `--` in order to be able to pass arguments to the script
});
if (pkg.scripts.start) {
// start script, generate one with the package name
Expand Down Expand Up @@ -265,7 +265,7 @@ class Generator extends Base {
// no pre post test tasks
cmds.filter((s) => toPatch.test(s)).forEach((s) => {
// generate scoped tasks
let cmd = `.${path.sep}withinEnv "exec 'cd ${p} && npm run ${s}'"`;
let cmd = `.${path.sep}withinEnv "exec 'cd ${p} && npm run ${s}'" --`;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let cmd = `.${path.sep}withinEnv "exec 'cd ${p} && npm run ${s}'" --`;
let cmd = `.${path.sep}withinEnv "exec 'cd ${p} && npm run ${s} --'"`;

@oltionchampari Did you test this case? @anita-steiner and I assume that the -- must be placed directly after the run command (as in the other change above). It could even be that this does not work at all, as you want to pass arguments through multiple layers (e.g., withinEnv and exec).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Running npm run check:phovea_server or any other withinEnv command from the workspace results in an error.

withinEnv_test

I do not know if there is a problem with my docker setup or the withinEnv script is outdated.
@thinkh Could you please test if the commands work for you. Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. This whole command does not work...

Investigation report

  • (I had to switch the line ending for withinEnv from CRLF to LF, which might be caused by my Windows)
  • I can confirm your results; I get the same output
  • Executing the command from withinEnv manually (docker-compose run --service-ports api) starts the container
  • docker-compose run --service-ports api exec results in Error response from daemon: OCI runtime create failed: container_linux.go:349: starting container process caused "exec: \"exec\": executable file not found in $PATH": unknown
  • docker-compose run --service-ports api /bin/sh results in a terminal inside the container
    • However, npm -v fails with /bin/sh: 2: npm: not found, as npm is not part of the python:3.7 base image!
  • docker-compose run --service-ports api /bin/sh -c "cd phovea_server && npm run test" should be the correct command, but it also fails with /bin/sh: 1: npm: not found (see previous point)
  • Replacing docker-compose run --service-ports api with withinEnv (command: ./withinEnv /bin/sh -c "cd phovea_server && echo test") prints docker-compose run --service-ports api /bin/sh -c cd phovea_server && echo test, i.e., without additional quotes and results in an empty terminal output
  • Escaping the command ./withinEnv "/bin/sh -c \"cd phovea_server && echo test\"" results in docker-compose run --service-ports api /bin/sh -c "cd phovea_server && echo test" and prints phovea_server: 1: phovea_server: Syntax error: Unterminated quoted string
    • Since the print statement itself works, but the execution itself fails it must be a problem with the $@
    • Reading the documentation about $@ it seems that it is not possible to pass strings from incoming parameter to the subsequent command
    • An alternative syntax to -c is the <<< syntax (see https://stackoverflow.com/a/40487106)
  • ./withinEnv /bin/bash <<< "cd phovea_server && echo test" results in the expected output test
    • Note that the whole bin/sh command is not wrapped in single quotes so that $@ can expand it as different paramters and pass it down to docker-compose
  • Copying the new command to package.json ("test:phovea_server": "./withinEnv /bin/bash <<< \"cd phovea_server && npm run test\"") and executing npm run test:phovea_server results in the following error: sh: 1: Syntax error: redirection unexpected
    • I found no obvious way how to fix this syntax in the npm script
    • The only solution I found is to move the redirection into the withinEnv: docker-compose run --service-ports api /bin/bash <<< $@
    • Change the npm script to ./withinEnv \"cd phovea_server && npm run test\"
  • Running npm run test:phovea_server results in the expected /bin/sh: 1: npm: not found
  • Now only the npm script npm run start:phovea_server does not work anymore and needs to be changed to "start:phovea_server": "./withinEnv \"cd phovea_server && npm run start\"",
    • Alternatively we can use "start:phovea_server": "./withinEnv \"python phovea_server\"", to start the phovea server

if (/^(start|watch)/g.test(s)) {
// special case for start to have the right working directory
let fixedscript = pkg.scripts[s].replace(/__main__\.py/, p);
Expand Down