From 3228a7465474e2d994a340d0e67b3d0c0f444e9e Mon Sep 17 00:00:00 2001 From: Mike Bland Date: Wed, 6 Jun 2018 23:37:05 -0400 Subject: [PATCH] bin/bats,install.sh: Fix symlink bugs Closes #90. Closes #83. Closes #56. Closes #55. Closes #53. Resolves outstanding issues from #32. Per all these earlier issues and PRs: - The original bin/bats symlink was causing problems on Windows. - The attempted fix from #32 created problems for install.sh. - Per #90, changing bin/bats to a one-line script in #88 broke some Bats installations that rely on symlink schemes (such as when installed via https://github.com/basherpm/basher). For an idea of why I wanted to keep bin/bats as a script due to how symlinks complicate things on Windows, see the following results I discovered during a search for "git symlink windows": - https://github.com/git-for-windows/git/wiki/Symbolic-Links - https://blogs.windows.com/buildingapps/2016/12/02/symlinks-windows-10/ - https://github.com/libgit2/libgit2/issues/4107 - https://stackoverflow.com/q/5917249 In the course of applying these changes, I realized libexec/bats computed some extraneous variables, so I removed them, eliminating a few external processes and subshells. I also cleaned up other small bits of logic. On top of making install.sh, bin/bats, and libexec/bats more resilient and compact, the existing test suite (before adding the new test/installer.bats file) sped up significantly, running 0.6s-0.7s faster on my MacBook Pro (2.9 GHz Intel Core i5, 8GB RAM): Bash 3.2.57(1)-release before: 58 tests, 0 failures real 0m4.924s user 0m3.045s sys 0m1.798s Bash 3.2.57(1)-release after: 58 tests, 0 failures real 0m4.341s user 0m2.808s sys 0m1.540s Bash 4.4.23(1)-release before: 58 tests, 0 failures real 0m5.228s user 0m3.046s sys 0m1.952s Bash 4.4.23(1)-release after: 58 tests, 0 failures real 0m4.582s user 0m2.791s sys 0m1.643s Also tweaks the Dockerfile to update the symlink to point to bin/bats, not libexec/bats. --- Dockerfile | 2 +- bin/bats | 34 +++++++++++++++++++++- install.sh | 62 +++++++++++++++++++++++++--------------- libexec/bats | 42 +++------------------------ libexec/bats-exec-test | 6 +--- test/install.bats | 65 ++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 143 insertions(+), 68 deletions(-) create mode 100644 test/install.bats diff --git a/Dockerfile b/Dockerfile index b84d3b9b..2d16b5c9 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,7 +1,7 @@ FROM alpine:3.6 RUN apk --no-cache add bash \ - && ln -s /opt/bats/libexec/bats /usr/sbin/bats + && ln -s /opt/bats/bin/bats /usr/sbin/bats COPY . /opt/bats/ diff --git a/bin/bats b/bin/bats index ea99998e..53141f8b 100755 --- a/bin/bats +++ b/bin/bats @@ -1,3 +1,35 @@ #! /usr/bin/env bash -exec "${0%/*}/../libexec/bats" "$@" +set -e + +export BATS_READLINK='true' +if command -v 'greadlink' >/dev/null; then + BATS_READLINK='greadlink' +elif command -v 'readlink' >/dev/null; then + BATS_READLINK='readlink' +fi + +bats_resolve_link() { + if ! "$BATS_READLINK" "$1"; then + return 0 + fi +} + +bats_absolute_path() { + local cwd="$PWD" + local path="$1" + + while [[ -n "$path" ]]; do + cd "${path%/*}" + local name="${path##*/}" + path="$(bats_resolve_link "$name")" + done + + printf -v "$2" -- '%s' "$PWD" + cd "$cwd" +} + +export BATS_ROOT +bats_absolute_path "$0" 'BATS_ROOT' +BATS_ROOT="${BATS_ROOT%/*}" +exec "$BATS_ROOT/libexec/bats" "$@" diff --git a/install.sh b/install.sh index 822b5575..59e31ac4 100755 --- a/install.sh +++ b/install.sh @@ -1,41 +1,57 @@ #!/usr/bin/env bash + set -e -resolve_link() { - $(type -p greadlink readlink | head -n1) "$1" +PREFIX="$1" +if [ -z "$1" ]; then + { echo "usage: $0 " + echo " e.g. $0 /usr/local" + } >&2 + exit 1 +fi + +export BATS_READLINK='true' +if command -v 'greadlink' >/dev/null; then + BATS_READLINK='greadlink' +elif command -v 'readlink' >/dev/null; then + BATS_READLINK='readlink' +fi + +bats_resolve_link() { + if ! "$BATS_READLINK" "$1"; then + return 0 + fi } -abs_dirname() { - local cwd="$(pwd)" +bats_absolute_path() { + local cwd="$PWD" local path="$1" - while [ -n "$path" ]; do + while [[ -n "$path" ]]; do cd "${path%/*}" local name="${path##*/}" - path="$(resolve_link "$name" || true)" + path="$(bats_resolve_link "$name")" done - pwd + printf -v "$2" -- '%s' "$PWD" cd "$cwd" } -PREFIX="$1" -if [ -z "$1" ]; then - { echo "usage: $0 " - echo " e.g. $0 /usr/local" - } >&2 - exit 1 -fi +bats_install_scripts() { + local scripts_dir + local scripts=() + + for scripts_dir in "$@"; do + scripts=("$BATS_ROOT/$scripts_dir"/*) + cp "${scripts[@]}" "$PREFIX/$scripts_dir" + chmod a+x "${scripts[@]/#$BATS_ROOT[/]/$PREFIX/}" + done +} -BATS_ROOT="$(abs_dirname "$0")" mkdir -p "$PREFIX"/{bin,libexec,share/man/man{1,7}} -cp -R "$BATS_ROOT"/bin/* "$PREFIX"/bin -cp -R "$BATS_ROOT"/libexec/* "$PREFIX"/libexec -cp "$BATS_ROOT"/man/bats.1 "$PREFIX"/share/man/man1 -cp "$BATS_ROOT"/man/bats.7 "$PREFIX"/share/man/man7 - -# fix file permission -chmod a+x "$PREFIX"/bin/* -chmod a+x "$PREFIX"/libexec/* +bats_absolute_path "$0" 'BATS_ROOT' +bats_install_scripts 'bin' 'libexec' +cp "$BATS_ROOT/man/bats.1" "$PREFIX/share/man/man1" +cp "$BATS_ROOT/man/bats.7" "$PREFIX/share/man/man7" echo "Installed Bats to $PREFIX/bin/bats" diff --git a/libexec/bats b/libexec/bats index ba7a9591..502ad710 100755 --- a/libexec/bats +++ b/libexec/bats @@ -26,57 +26,23 @@ help() { echo } -export BATS_READLINK='true' -if command -v 'greadlink' >/dev/null; then - BATS_READLINK='greadlink' -elif command -v 'readlink' >/dev/null; then - BATS_READLINK='readlink' -fi - -resolve_link() { - if ! "$BATS_READLINK" "$1"; then - return 0 - fi -} - -abs_dirname() { - local cwd="$PWD" - local path="$1" - - while [[ -n "$path" ]]; do - cd "${path%/*}" - local name="${path##*/}" - path="$(resolve_link "$name")" - done - - printf -v "$2" -- '%s' "$PWD" - cd "$cwd" -} - expand_path() { local path="${1%/}" local dirname="${path%/*}" if [[ "$dirname" == "$path" ]]; then dirname="$PWD" - elif cd "$dirname" 2>/dev/null; then + else + cd "$dirname" dirname="$PWD" cd "$OLDPWD" - else - printf '%s' "$path" - return fi printf -v "$2" '%s/%s' "$dirname" "${path##*/}" } -abs_dirname "$0" 'BATS_LIBEXEC' -abs_dirname "$BATS_LIBEXEC" 'BATS_PREFIX' -abs_dirname '.' 'BATS_CWD' - -export BATS_PREFIX -export BATS_CWD +export BATS_CWD="${PWD%/*}" export BATS_TEST_PATTERN="^[[:blank:]]*@test[[:blank:]]+(.*[^[:blank:]])[[:blank:]]+\{(.*)\$" -export PATH="$BATS_LIBEXEC:$PATH" +export PATH="$BATS_ROOT/libexec:$PATH" options=() arguments=() diff --git a/libexec/bats-exec-test b/libexec/bats-exec-test index 8ea72b91..5301bec7 100755 --- a/libexec/bats-exec-test +++ b/libexec/bats-exec-test @@ -211,11 +211,7 @@ bats_strip_string() { } bats_trim_filename() { - if [[ "$1" =~ ^${BATS_CWD}/ ]]; then - printf -v "$2" '%s' "${1#$BATS_CWD/}" - else - printf -v "$2" '%s' "$1" - fi + printf -v "$2" '%s' "${1#$BATS_CWD/}" } bats_debug_trap() { diff --git a/test/install.bats b/test/install.bats new file mode 100644 index 00000000..5c8bb745 --- /dev/null +++ b/test/install.bats @@ -0,0 +1,65 @@ +#!/usr/bin/env bats + +load test_helper + +INSTALL_DIR= + +setup() { + make_bats_test_suite_tmpdir + INSTALL_DIR="$BATS_TEST_SUITE_TMPDIR/bats-core" +} + +@test "install.sh creates a valid installation" { + run "$BATS_ROOT/install.sh" "$INSTALL_DIR" + [ "$status" -eq 0 ] + [ "$output" == "Installed Bats to $INSTALL_DIR/bin/bats" ] + [ -x "$INSTALL_DIR/bin/bats" ] + [ -x "$INSTALL_DIR/libexec/bats" ] + [ -x "$INSTALL_DIR/libexec/bats-exec-suite" ] + [ -x "$INSTALL_DIR/libexec/bats-exec-test" ] + [ -x "$INSTALL_DIR/libexec/bats-format-tap-stream" ] + [ -x "$INSTALL_DIR/libexec/bats-preprocess" ] + [ -f "$INSTALL_DIR/share/man/man1/bats.1" ] + [ -f "$INSTALL_DIR/share/man/man7/bats.7" ] + + run "$INSTALL_DIR/bin/bats" -v + [ "$status" -eq 0 ] + [ "${output%% *}" == 'Bats' ] +} + +@test "install.sh only updates permissions for Bats files" { + mkdir -p "$INSTALL_DIR"/{bin,libexec} + + local dummy_bin="$INSTALL_DIR/bin/dummy" + printf 'dummy' >"$dummy_bin" + + local dummy_libexec="$INSTALL_DIR/libexec/dummy" + printf 'dummy' >"$dummy_libexec" + + run "$BATS_ROOT/install.sh" "$INSTALL_DIR" + [ "$status" -eq 0 ] + [ -f "$dummy_bin" ] + [ ! -x "$dummy_bin" ] + [ -f "$dummy_libexec" ] + [ ! -x "$dummy_libexec" ] +} + +@test "bin/bats is resilient to symbolic links" { + run "$BATS_ROOT/install.sh" "$INSTALL_DIR" + [ "$status" -eq 0 ] + + # Simulate a symlink to bin/bats (without using a symlink, for Windows sake) + # by creating a wrapper script that executes bin/bats via a relative path. + # + # The real test is in the .travis.yml script using the Dockerfile, which uses + # a real symbolic link. + local bats_symlink="$INSTALL_DIR/bin/bats-link" + printf '%s\n' '#! /usr/bin/env bash' \ + "cd '$INSTALL_DIR/bin'" \ + './bats "$@"' >"$bats_symlink" + chmod 700 "$bats_symlink" + + run "$bats_symlink" -v + [ "$status" -eq 0 ] + [ "${output%% *}" == 'Bats' ] +}