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
\$\endgroup\$
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 \;
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.