Skip to content

Commit

Permalink
template: Replace process substitution with pipe
Browse files Browse the repository at this point in the history
After digging deeper into process substitution vs. pipes during the
implementation of `@go.mirror_directory` from `lib/fileutil`, I
realized a few things:

* The same number of processes are spawned.
* There's little benefit to a process substitution for straightforward
  pipes when the output isn't consumed by the shell process.
* It's not straightforward to get the status from a process
  substitution, and neither '$!' nor `wait` help.
* The 'PIPESTATUS' variable is available to check the status of each
  process in the pipe, without the need for `set -o pipefail`.

Also adds a test case for `tar` failure. The 'fail to download ...' test
cases required a little tweaking to ensure they properly cover the cases
where `${download_cmd[@]}` fails.
  • Loading branch information
mbland committed Sep 10, 2017
1 parent c1f4c8b commit 73ac6fc
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 7 deletions.
6 changes: 3 additions & 3 deletions go-template
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
# Make sure the variables within this script are configured as necessary for
# your program. You can add any other initialization or configuration between:
#
# . "$_GO_SCRIPT_BASH_CORE_DIR/go-core.bash" "$GO_SCRIPTS_DIR"`
# . "$GO_SCRIPT_BASH_CORE_DIR/go-core.bash" "$GO_SCRIPTS_DIR"`
# `@go "$@"`

# Set to 'true' if your script is a standalone program, i.e. not bound to
Expand Down Expand Up @@ -50,7 +50,6 @@ declare GO_SCRIPT_BASH_DOWNLOAD_URL="${GO_SCRIPT_BASH_DOWNLOAD_URL:-${GO_SCRIPT_
download_go_script_bash_tarball() {
# GitHub removes the leading 'v' from the archive's output directory.
local unpacked_dir="go-script-bash-${GO_SCRIPT_BASH_VERSION#v}"
local unpacked_core="$unpacked_dir/go-core.bash"
local core_dir_parent="${GO_SCRIPT_BASH_CORE_DIR%/*}"
local url="$GO_SCRIPT_BASH_DOWNLOAD_URL"
local protocol="${url%%://*}"
Expand Down Expand Up @@ -84,7 +83,8 @@ download_go_script_bash_tarball() {
fi
printf "Downloading framework from '%s'...\n" "$url"

if ! tar -xzf - < <("${download_cmd[@]}") || [[ ! -f "$unpacked_core" ]]; then
if ! "${download_cmd[@]}" | tar -xzf - ||
[[ "${PIPESTATUS[0]}" -ne '0' ]]; then
printf "Failed to download from '%s'.\n" "$url" >&2
return 1
elif [[ ! -d "$core_dir_parent" ]] && ! mkdir -p "$core_dir_parent" ; then
Expand Down
31 changes: 27 additions & 4 deletions tests/template.bats
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ run_with_download_program() {

# We're forcing a local tarball "download" here.
TEST_USE_REAL_URL= create_fake_tarball_if_not_using_real_url
GO_SCRIPT_BASH_DOWNLOAD_URL="file://$TEST_GO_ROOTDIR/archive" \
GO_SCRIPT_BASH_DOWNLOAD_URL="$TEST_ARCHIVE_URL" \
PATH="$BATS_TEST_BINDIR" run "$BASH" "$TEST_GO_ROOTDIR/go-template"
restore_bats_shell_options
}
Expand Down Expand Up @@ -234,7 +234,7 @@ run_with_download_program() {
}

@test "$SUITE: fail to download a nonexistent repo" {
local url='https://bogus-url-that-does-not-exist'
local url="$GO_SCRIPT_BASH_DOWNLOAD_URL/bogus-url-that-does-not-exist"
local repo='bogus-repo-that-does-not-exist'

skip_if_none_present_on_system 'curl' 'fetch' 'wget'
Expand All @@ -251,13 +251,15 @@ run_with_download_program() {
}

@test "$SUITE: fail to download a nonexistent version" {
local url="$GO_SCRIPT_BASH_DOWNLOAD_URL/vnonexistent.tar.gz"
local url="$GO_SCRIPT_BASH_DOWNLOAD_URL"
local branch='vnonexistent'

skip_if_none_present_on_system 'curl' 'fetch' 'wget'
skip_if_system_missing 'git' 'tar'
GO_SCRIPT_BASH_VERSION="$branch" run "$TEST_GO_ROOTDIR/go-template"
assert_failure

assert_output_matches "Downloading framework from '$url/${branch}.tar.gz'"
assert_output_matches "Failed to download from '$url/${branch}.tar.gz'"
assert_output_matches 'Using git clone as fallback'
assert_output_matches "Cloning framework from '$GO_SCRIPT_BASH_REPO_URL'"
assert_output_matches "Cloning into '$CLONE_DIR'"
Expand Down Expand Up @@ -314,6 +316,27 @@ run_with_download_program() {
"Clone of '$GO_SCRIPT_BASH_REPO_URL' successful\."$'\n\n'
}

@test "$SUITE: tar failure uses git clone" {
skip_if_none_present_on_system 'curl' 'fetch' 'wget'
skip_if_system_missing 'git' 'tar'

TEST_USE_REAL_URL= create_fake_tarball_if_not_using_real_url
stub_program_in_path 'tar' \
"$(command -v 'tar') \"\$@\"" \
'exit 1'

GO_SCRIPT_BASH_DOWNLOAD_URL="$TEST_ARCHIVE_URL" \
run "$BASH" "$TEST_GO_ROOTDIR/go-template"
restore_program_in_path 'tar'

assert_output_matches "Downloading framework from '$EXPECTED_URL'\.\.\."
assert_output_matches "Failed to download from '$LOCAL_DOWNLOAD_URL'"
assert_output_matches "Using git clone as fallback"
assert_output_matches "Cloning framework from '$GO_SCRIPT_BASH_REPO_URL'"
assert_output_matches "Cloning into '$CLONE_DIR'"
assert_output_matches "Clone of '$GO_SCRIPT_BASH_REPO_URL' successful\."
}

@test "$SUITE: fail to create directory uses git clone" {
skip_if_none_present_on_system 'curl' 'fetch' 'wget'
skip_if_system_missing 'git' 'tar'
Expand Down

0 comments on commit 73ac6fc

Please sign in to comment.