2
\$\begingroup\$

I'm relatively familiar with the command line, but I've usually reached for other languages rather than bash for more full on scripts. It works, but I'm wondering if anything isn't really idiomatic. I've never used bash arrays before, and I've never had to write such an involved find command so I'm just wondering if I did all that fine. Also just overall is there a shorter way to do this?

.

#!/bin/bash
set -e
# The directories to search in
# Currently, lib + tools, and whatever command line args are provided
DIRS=(lib tools $@)
for i in "${!DIRS[@]}";
do
 DIRS[i]="$(pwd)/${DIRS[i]}"
done
# Get absolute path of directories with a package.json file and install dependencies
while IFS='' read -r line; do
 echo "Installing dependencies for $line"
 cd "$line" && yarn
done < <(
 find "${DIRS[@]}" -iname 'package.json' -not -path '*/node_modules/*' -exec dirname {} \;
)
# Do current directory
echo "Installing dependencies for current directory"
yarn
asked May 12, 2017 at 15:13
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

Working with $@

If some command line arguments contain spaces this won't work:

DIRS=(lib tools $@)

In 99% of use cases you probably want to double quote $@.

Unnecessary work

It's unnecessary to execute $(pwd) in a loop if the directory doesn't change in between cycles:

 DIRS[i]="$(pwd)/${DIRS[i]}"

A simple fix is using $PWD instead, which will not incur command execution:

 DIRS[i]="$PWD/${DIRS[i]}"

Building arrays

This is not very idiomatic:

DIRS=(lib tools $@)
for i in "${!DIRS[@]}";
do
 DIRS[i]="$(pwd)/${DIRS[i]}"
done

It would be better like this:

DIRS=()
for i in lib tools "$@"
do
 DIRS+=("$PWD/$i")
done

cd in a sub-shell

This works because you made sure the paths are absolute paths:

while IFS='' read -r line; do
 echo "Installing dependencies for $line"
 cd "$line" && yarn
done < <(
 find "${DIRS[@]}" -iname 'package.json' -not -path '*/node_modules/*' -exec dirname {} \;
)

If the paths had been relative, this wouldn't work. That's why you built DIRS, a considerable extra effort. There's a way to make this safe even with relative paths, by wrapping cd && yarn in a sub-shell:

while IFS='' read -r line; do
 echo "Installing dependencies for $line"
 (cd "$line" && yarn)
done < <(
 find lib tools "$@" -iname 'package.json' -not -path '*/node_modules/*' -exec dirname {} \;
)

Without DIRS, the script can be shorter and simpler.

find ... -execdir

If your version of find supports the -execdir option, and if you don't mind not printing the "Installing dependencies for ...", then you can further simplify the script:

#!/bin/bash
set -e
# Install dependencies in directories with a package.json file
find lib tools "$@" -iname 'package.json' -not -path '*/node_modules/*' \
 -execdir yarn \;
# Do current directory
echo "Installing dependencies for current directory"
yarn

If you really do want to print that informational line, that's possible, though not very sexy:

find lib tools "$@" -iname 'package.json' -not -path '*/node_modules/*' \
 -exec printf Installing dependencies for \; -execdir pwd \; -execdir yarn \;
answered May 14, 2017 at 17:01
\$\endgroup\$

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.