From 571208c8eeef8426372d14144761506eb3815a55 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Sun, 28 Mar 2021 01:50:15 +0100 Subject: [PATCH 1/4] Only call `find` on directories. --- Library/Homebrew/cask/pkg.rb | 35 +++++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/Library/Homebrew/cask/pkg.rb b/Library/Homebrew/cask/pkg.rb index 0a63d49df1..d999c35f21 100644 --- a/Library/Homebrew/cask/pkg.rb +++ b/Library/Homebrew/cask/pkg.rb @@ -111,27 +111,34 @@ module Cask set -euo pipefail for path in "${@}"; do - if [[ ! -e "${path}" ]]; then + symlink=true + [[ -L "${path}" ]] || symlink=false + + directory=false + if [[ -d "${path}" ]]; then + directory=true + + if [[ -e "${path}/.DS_Store" ]]; then + /bin/rm -- "${path}/.DS_Store" + fi + + # Some packages leave broken symlinks around; we clean them out before + # attempting to `rmdir` to prevent extra cruft from accumulating. + /usr/bin/find -f "${path}" -- -mindepth 1 -maxdepth 1 -type l ! -exec /bin/test -e {} \; -delete + elif ! ${symlink} && [[ ! -e "${path}" ]]; then + # Skip paths that don't exists and aren't a broken symlink. continue fi - if [[ -e "${path}/.DS_Store" ]]; then - /bin/rm -f "${path}/.DS_Store" - fi - - # Some packages leave broken symlinks around; we clean them out before - # attempting to `rmdir` to prevent extra cruft from accumulating. - /usr/bin/find "${path}" -mindepth 1 -maxdepth 1 -type l ! -exec /bin/test -e {} \; -delete - - if [[ -L "${path}" ]]; then + if ${symlink}; then # Delete directory symlink. - /bin/rm "${path}" - elif [[ -d "${path}" ]]; then + /bin/rm -- "${path}" + elif ${directory}; then # Delete directory if empty. - /usr/bin/find "${path}" -maxdepth 0 -type d -empty -exec /bin/rmdir {} \; + /usr/bin/find -f "${path}" -- -maxdepth 0 -type d -empty -exec /bin/rmdir -- {} \; else # Try `rmdir` anyways to show a proper error. - /bin/rmdir "${path}" + /bin/rmdir -- "${path}" fi done BASH From 23376bad7512e5e66869e512aca1e0177c51a54f Mon Sep 17 00:00:00 2001 From: Max Mueggler Date: Thu, 18 Mar 2021 11:17:41 -0400 Subject: [PATCH 2/4] Speed up cask pkg uninstallation when there's many directories to delete. Results in an order of magnitude speedup for uninstalling large packages such as mactex. --- Library/Homebrew/cask/pkg.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Library/Homebrew/cask/pkg.rb b/Library/Homebrew/cask/pkg.rb index d999c35f21..47134fbc42 100644 --- a/Library/Homebrew/cask/pkg.rb +++ b/Library/Homebrew/cask/pkg.rb @@ -110,6 +110,12 @@ module Cask RMDIR_SH = <<~'BASH' set -euo pipefail + # Try removing as many empty directories as possible with a single + # `rmdir` call to avoid or at least speed up the loop below. + if /bin/rmdir -- "${@}" &>/dev/null; then + exit + fi + for path in "${@}"; do symlink=true [[ -L "${path}" ]] || symlink=false From 5aa0dbe1f96f0528e3a9662fb1a5dbe29ddf56ca Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Tue, 30 Mar 2021 01:09:01 +0200 Subject: [PATCH 3/4] Move `RMDIR_SH` into separate file. --- Library/Homebrew/cask/pkg.rb | 44 ++------------------------ Library/Homebrew/cask/utils/rmdir.sh | 41 ++++++++++++++++++++++++ Library/Homebrew/test/cask/pkg_spec.rb | 2 +- 3 files changed, 44 insertions(+), 43 deletions(-) create mode 100755 Library/Homebrew/cask/utils/rmdir.sh diff --git a/Library/Homebrew/cask/pkg.rb b/Library/Homebrew/cask/pkg.rb index 47134fbc42..83447c4b6e 100644 --- a/Library/Homebrew/cask/pkg.rb +++ b/Library/Homebrew/cask/pkg.rb @@ -107,54 +107,14 @@ module Cask # Helper script to delete empty directories after deleting `.DS_Store` files and broken symlinks. # Needed in order to execute all file operations with `sudo`. - RMDIR_SH = <<~'BASH' - set -euo pipefail - - # Try removing as many empty directories as possible with a single - # `rmdir` call to avoid or at least speed up the loop below. - if /bin/rmdir -- "${@}" &>/dev/null; then - exit - fi - - for path in "${@}"; do - symlink=true - [[ -L "${path}" ]] || symlink=false - - directory=false - if [[ -d "${path}" ]]; then - directory=true - - if [[ -e "${path}/.DS_Store" ]]; then - /bin/rm -- "${path}/.DS_Store" - fi - - # Some packages leave broken symlinks around; we clean them out before - # attempting to `rmdir` to prevent extra cruft from accumulating. - /usr/bin/find -f "${path}" -- -mindepth 1 -maxdepth 1 -type l ! -exec /bin/test -e {} \; -delete - elif ! ${symlink} && [[ ! -e "${path}" ]]; then - # Skip paths that don't exists and aren't a broken symlink. - continue - fi - - if ${symlink}; then - # Delete directory symlink. - /bin/rm -- "${path}" - elif ${directory}; then - # Delete directory if empty. - /usr/bin/find -f "${path}" -- -maxdepth 0 -type d -empty -exec /bin/rmdir -- {} \; - else - # Try `rmdir` anyways to show a proper error. - /bin/rmdir -- "${path}" - fi - done - BASH + RMDIR_SH = (HOMEBREW_LIBRARY_PATH/"cask/utils/rmdir.sh").freeze private_constant :RMDIR_SH sig { params(path: T.any(Pathname, T::Array[Pathname])).void } def rmdir(path) @command.run!( "/usr/bin/xargs", - args: ["-0", "--", "/bin/bash", "-c", RMDIR_SH, "--"], + args: ["-0", "--", RMDIR_SH.to_s], input: Array(path).join("\0"), sudo: true, ) diff --git a/Library/Homebrew/cask/utils/rmdir.sh b/Library/Homebrew/cask/utils/rmdir.sh new file mode 100755 index 0000000000..b1a271d1b1 --- /dev/null +++ b/Library/Homebrew/cask/utils/rmdir.sh @@ -0,0 +1,41 @@ +#!/bin/bash + +set -euo pipefail + +# Try removing as many empty directories as possible with a single +# `rmdir` call to avoid or at least speed up the loop below. +if /bin/rmdir -- "${@}" &>/dev/null; then + exit +fi + +for path in "${@}"; do + symlink=true + [[ -L "${path}" ]] || symlink=false + + directory=false + if [[ -d "${path}" ]]; then + directory=true + + if [[ -e "${path}/.DS_Store" ]]; then + /bin/rm -- "${path}/.DS_Store" + fi + + # Some packages leave broken symlinks around; we clean them out before + # attempting to `rmdir` to prevent extra cruft from accumulating. + /usr/bin/find -f "${path}" -- -mindepth 1 -maxdepth 1 -type l ! -exec /bin/test -e {} \; -delete + elif ! ${symlink} && [[ ! -e "${path}" ]]; then + # Skip paths that don't exists and aren't a broken symlink. + continue + fi + + if ${symlink}; then + # Delete directory symlink. + /bin/rm -- "${path}" + elif ${directory}; then + # Delete directory if empty. + /usr/bin/find -f "${path}" -- -maxdepth 0 -type d -empty -exec /bin/rmdir -- {} \; + else + # Try `rmdir` anyways to show a proper error. + /bin/rmdir -- "${path}" + fi +done diff --git a/Library/Homebrew/test/cask/pkg_spec.rb b/Library/Homebrew/test/cask/pkg_spec.rb index 5c7b4eecca..2579525675 100644 --- a/Library/Homebrew/test/cask/pkg_spec.rb +++ b/Library/Homebrew/test/cask/pkg_spec.rb @@ -97,7 +97,7 @@ describe Cask::Pkg, :cask do allow(fake_system_command).to receive(:run!).and_call_original expect(fake_system_command).to receive(:run!).with( "/usr/bin/xargs", - args: ["-0", "--", "/bin/bash", "-c", a_string_including("/bin/rmdir"), "--"], + args: ["-0", "--", a_string_including("rmdir")], input: [fake_dir].join("\0"), sudo: true, ).and_return(instance_double(SystemCommand::Result, stdout: "")) From c2c93d1cc9e436db0e0ff18c39e017d7ba5abf57 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Tue, 30 Mar 2021 01:15:05 +0200 Subject: [PATCH 4/4] Run `shellcheck` on cask utils and shim scripts. --- Library/Homebrew/shims/linux/super/make | 5 +++-- Library/Homebrew/shims/mac/super/ruby | 3 +++ Library/Homebrew/shims/mac/super/xcrun | 11 ++++++----- Library/Homebrew/shims/scm/git | 2 ++ Library/Homebrew/shims/utils.sh | 6 ++++-- Library/Homebrew/style.rb | 11 +++++++---- 6 files changed, 25 insertions(+), 13 deletions(-) diff --git a/Library/Homebrew/shims/linux/super/make b/Library/Homebrew/shims/linux/super/make index 36412314a1..e8bd9f82ef 100755 --- a/Library/Homebrew/shims/linux/super/make +++ b/Library/Homebrew/shims/linux/super/make @@ -7,7 +7,7 @@ pathremove() { NEWPATH=${NEWPATH:+$NEWPATH:}$DIR fi done - export $PATHVARIABLE="$NEWPATH" + export "$PATHVARIABLE"="$NEWPATH" } if [[ -n "$HOMEBREW_MAKE" && "$HOMEBREW_MAKE" != "make" ]] @@ -16,7 +16,8 @@ then else SAVED_PATH="$PATH" pathremove "$HOMEBREW_LIBRARY/Homebrew/shims/linux/super" - export MAKE="$(which make)" + MAKE="$(which make)" + export MAKE export PATH="$SAVED_PATH" fi diff --git a/Library/Homebrew/shims/mac/super/ruby b/Library/Homebrew/shims/mac/super/ruby index 91921781ff..9a125dc7e2 100755 --- a/Library/Homebrew/shims/mac/super/ruby +++ b/Library/Homebrew/shims/mac/super/ruby @@ -1,6 +1,9 @@ #!/bin/bash + # System Ruby's mkmf on Mojave (10.14) and later require SDKROOT set to work correctly. +# Don't need shellcheck to follow the `source`. +# shellcheck disable=SC1090 source "$HOMEBREW_LIBRARY/Homebrew/shims/utils.sh" try_exec_non_system "$SHIM_FILE" "$@" diff --git a/Library/Homebrew/shims/mac/super/xcrun b/Library/Homebrew/shims/mac/super/xcrun index 9217ca4bbe..00c34f31b7 100755 --- a/Library/Homebrew/shims/mac/super/xcrun +++ b/Library/Homebrew/shims/mac/super/xcrun @@ -1,4 +1,5 @@ #!/bin/bash + # Historically, xcrun has had various bugs, and in some cases it didn't # work at all (e.g. CLT-only in the Xcode 4.3 era). This script emulates # it and attempts to avoid these issues. @@ -27,10 +28,10 @@ arg0=$1 shift exe="/usr/bin/${arg0}" -if [ -x "$exe" ]; then - if [ -n "$HOMEBREW_PREFER_CLT_PROXIES" ]; then +if [[ -x "$exe" ]]; then + if [[ -n "$HOMEBREW_PREFER_CLT_PROXIES" ]]; then exec "$exe" "$@" - elif [ -z "$HOMEBREW_SDKROOT" -o ! -d "$HOMEBREW_SDKROOT" ]; then + elif [[ -z "$HOMEBREW_SDKROOT" || ! -d "$HOMEBREW_SDKROOT" ]]; then exec "$exe" "$@" fi fi @@ -38,7 +39,7 @@ fi SUPERBIN=$(cd "${0%/*}" && pwd -P) exe=$(/usr/bin/xcrun --find "$arg0" 2>/dev/null) -if [ -x "$exe" -a "${exe%/*}" != "$SUPERBIN" ]; then +if [[ -x "$exe" && "${exe%/*}" != "$SUPERBIN" ]]; then exec "$exe" "$@" fi @@ -57,7 +58,7 @@ done IFS=$old_IFS echo >&2 " -Failed to execute $arg0 $@ +Failed to execute ${arg0} ${*} Xcode and/or the CLT appear to be misconfigured. Try one or both of the following: xcodebuild -license diff --git a/Library/Homebrew/shims/scm/git b/Library/Homebrew/shims/scm/git index 788b1cbb07..6b0382ad6e 100755 --- a/Library/Homebrew/shims/scm/git +++ b/Library/Homebrew/shims/scm/git @@ -9,6 +9,8 @@ then exit 1 fi +# Don't need shellcheck to follow the `source`. +# shellcheck disable=SC1090 source "$HOMEBREW_LIBRARY/Homebrew/shims/utils.sh" case "$(lowercase "$SHIM_FILE")" in diff --git a/Library/Homebrew/shims/utils.sh b/Library/Homebrew/shims/utils.sh index 7d90d7b697..9252bf7653 100644 --- a/Library/Homebrew/shims/utils.sh +++ b/Library/Homebrew/shims/utils.sh @@ -60,8 +60,10 @@ safe_exec() { fi if [[ "$HOMEBREW" = "print-path" ]] then - local dir="$(quiet_safe_cd "${arg0%/*}/" && pwd)" - local path="$(dirbasepath "$dir" "$arg0")" + local dir + dir="$(quiet_safe_cd "${arg0%/*}/" && pwd)" + local path + path="$(dirbasepath "$dir" "$arg0")" echo "$path" exit fi diff --git a/Library/Homebrew/style.rb b/Library/Homebrew/style.rb index 7511754fdb..8346871cc0 100644 --- a/Library/Homebrew/style.rb +++ b/Library/Homebrew/style.rb @@ -177,13 +177,16 @@ module Homebrew files = [ HOMEBREW_BREW_FILE, # TODO: HOMEBREW_REPOSITORY/"completions/bash/brew", - *Pathname.glob("#{HOMEBREW_LIBRARY}/Homebrew/*.sh"), - *Pathname.glob("#{HOMEBREW_LIBRARY}/Homebrew/cmd/*.sh"), - *Pathname.glob("#{HOMEBREW_LIBRARY}/Homebrew/utils/*.sh"), + *HOMEBREW_LIBRARY.glob("Homebrew/*.sh"), + *HOMEBREW_LIBRARY.glob("Homebrew/shims/**/*").map(&:realpath).uniq + .reject { |path| path.directory? || path.basename.to_s == "cc" }, + *HOMEBREW_LIBRARY.glob("Homebrew/{dev-,}cmd/*.sh"), + *HOMEBREW_LIBRARY.glob("Homebrew/{cask/,}utils/*.sh"), ] end - args = ["--shell=bash", "--", *files] # TODO: Add `--enable=all` to check for more problems. + # TODO: Add `--enable=all` to check for more problems. + args = ["--shell=bash", "--external-sources", "--", *files] case output_type when :print