0

I want to run something like this in my shell script:

./node_modules/.bin/npm-run-all -p "ng lint myapp"

I have a for loop to generate the quoted string:

LINT=""
for app in $APPS
do
    LINT="$LINT \"ng lint ${app}\""
done

I thought using \" will be able to generate the string I want. But when I run:

./node_modules/.bin/npm-run-all -p ${LINT}

I get:

ERROR: Task not found: ""ng", myapp""

Seems like this is being run:

./node_modules/.bin/npm-run-all -p \"ng lint myapp\"

What is the proper way to escape the double quote so that I will be running this instead?:

./node_modules/.bin/npm-run-all -p "ng lint myapp"
techguy2000
  • 4,861
  • 6
  • 32
  • 48
  • Shell experts may not be familiar with npm and ng. Edit your post to include an example of the correct `npm-run-all` command line when `$APPS` is `myapp yourapp`. – rob mayoff Dec 20 '18 at 04:13
  • You most likely want to use an array. But what should your final command look like if there are multiple values in `$APPS`? (For that matter, `APPS` itself should probably be an array, not a regular variable.) – chepner Dec 20 '18 at 04:13
  • 3
    https://mywiki.wooledge.org/BashFAQ/050 – Biffen Dec 20 '18 at 07:59

1 Answers1

0

Updated: Thanks for pointing out the other duplicate question and @CharlesDuffy's comments. My problem can be solved like this:

#!/bin/bash

APPS="app1 app2"
LIBS="lib1 lib2"
PROJECTS="${APPS} ${LIBS}"

projects=()
for project in $PROJECTS
do
  projects+=("ng lint ${project}")
done

./node_modules/.bin/npm-run-all -p "${projects[@]}"

An alternative way with WAIT:

#!/bin/bash

APPS="app1 app2"
LIBS="lib1 lib2"
PROJECTS="${APPS} ${LIBS}"

FAIL=0

pids=()

for project in $PROJECTS
do
  ng lint "${project}" & pids+=( "$!" )
done

for pid in "${pids[@]}"
do
  wait "${pid}" || FAIL=$(( FAIL + 1 ));
done

if [ "$FAIL" != "0" ];
  then
    exit 1
fi
techguy2000
  • 4,861
  • 6
  • 32
  • 48
  • Umm. `"$projects"` refers to `"${projects[0]}"`; it ignores all but the first element of an array. You might consider running the code in this answer through http://shellcheck.net/ to catch some other outstanding issues. – Charles Duffy Dec 21 '18 at 23:28
  • Similarly, `for project in $PROJECTS` is innately buggy on its face -- unquoted expansion performs string-splitting and globbing. And `let` is deprecated legacy syntax which isn't portable across shells -- the 1992 POSIX sh standard specifies `FAIL=$(( FAIL + 1 ))` as the modern way to do shell arithmetic. – Charles Duffy Dec 21 '18 at 23:30
  • ...so, what you probably **want** is `npm-run-all -p "${projects[@]}"`, passing *all* of the projects through. (assuming the desired usage is `npm-run-all -p "ng lint first" "ng lint second" "ng lint third"`). – Charles Duffy Dec 21 '18 at 23:31
  • ...and `$PROJECTS` should be modified to be an array in the first place rather than a string. `for project in "${projects[@]}"` is then the correct way to iterate through, once you're specifying `projects=( first second third )` rather than `projects="first second third"`. – Charles Duffy Dec 21 '18 at 23:32
  • Also, job control isn't guaranteed to be available in noninterctive shells. Instead of using `jobs -p`, initialize an empty array of PIDs: `pids=( )`; then, in your loop, append to it: `ng lint "$project" & pids+=( "$!" )`; then, later, `for pid in "${pids[@]}"; do wait "$pid" || fail=$(( fail + 1 )); done` – Charles Duffy Dec 21 '18 at 23:33
  • @CharlesDuffy Thanks very much for pointing out the issues. I have updated my answer and checked the scripts on shellcheck.net. For my alternative approach, I wonder why I don't need & at the end of pids+=( "$!" ). I thought I needed to end a statement with & to achieve concurrency. – techguy2000 Dec 22 '18 at 07:03
  • Insofar as the parser is concerned, `&` is a command separator, like `;` is. So when you run `foo & bar`, it starts `foo` in the background and `bar` in the foreground; whereas `foo & bar &` would start both `foo` *and* `bar` in the background. But `pids+=( "$!" )` needs to be in the foreground, or else it wouldn't be changing the variable in your main shell (but would just be setting it in a temporary background-process shell that immediately exits). – Charles Duffy Dec 22 '18 at 16:26
  • BTW, personally, I might write this as one of the versions given in https://gist.github.com/charles-dyfis-net/066effaaa7e908b6a95ac88e5980188f – Charles Duffy Dec 22 '18 at 16:29
  • Very concise. Thank you very much for all the tips! – techguy2000 Dec 22 '18 at 17:20
  • BTW, I see some people use UPPER case for shell script variable names, but you use lower case. Is there a convention or just personal preference? – techguy2000 Dec 22 '18 at 17:23
  • POSIX-specified convention. Upper case is used for names that are meaningful to the shell and operating-system-provided tools; see http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html: *The name space of environment variable names containing lowercase letters is reserved for applications. Applications can define any environment variables with names from this name space without modifying the behavior of the standard utilities.* -- by using lower-case names you don't break things by accident (one common error is `for PATH in */; do`, after which no executables can be found). – Charles Duffy Dec 22 '18 at 17:30
  • See http://wooledge.org/~greybot/meta/varcap for the history of the relevant factoid from the irc.freenode.org #bash channel. – Charles Duffy Dec 22 '18 at 17:32