From fa38b660e2a6e3953ff28780b0b7c61dd55b059f Mon Sep 17 00:00:00 2001 From: Ivan Kuchin Date: Mon, 17 Nov 2014 15:44:50 +0100 Subject: [PATCH 1/8] Abort with usage help on wrong number of CLI arguments - rbenv-install accepts 0 or 1 argument - rbenv-uninstall accepts 1 argument - ruby-build accepts 2 arguments --- bin/rbenv-install | 2 ++ bin/rbenv-uninstall | 9 ++++++++ bin/ruby-build | 3 +++ test/arguments.bats | 53 +++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 67 insertions(+) create mode 100644 test/arguments.bats diff --git a/bin/rbenv-install b/bin/rbenv-install index 9bb6c108..83d233db 100755 --- a/bin/rbenv-install +++ b/bin/rbenv-install @@ -111,6 +111,8 @@ DEFINITION="${ARGUMENTS[0]}" [ -n "$DEFINITION" ] || DEFINITION="$(rbenv-local 2>/dev/null || true)" [ -n "$DEFINITION" ] || usage 1 +# Fail if number of arguments is greater than 1 +[ "${#ARGUMENTS[@]}" -le 1 ] || usage 1 # Define `before_install` and `after_install` functions that allow # plugin hooks to register a string of code for execution before or diff --git a/bin/rbenv-uninstall b/bin/rbenv-uninstall index 10aa6e93..834f048f 100755 --- a/bin/rbenv-uninstall +++ b/bin/rbenv-uninstall @@ -17,6 +17,12 @@ if [ "$1" = "--complete" ]; then exec rbenv versions --bare fi +usage() { + # We can remove the sed fallback once rbenv 0.4.0 is widely available. + rbenv-help uninstall 2>/dev/null || sed -ne '/^#/!q;s/.//;s/.//;1,4d;p' < "$0" + [ -z "$1" ] || exit "$1" +} + if [ -z "$RBENV_ROOT" ]; then RBENV_ROOT="${HOME}/.rbenv" fi @@ -38,6 +44,9 @@ case "$DEFINITION" in ;; esac +# Fail if number of arguments is not equal 1 +[ "$#" -eq 1 ] || usage 1 + declare -a before_hooks after_hooks before_uninstall() { diff --git a/bin/ruby-build b/bin/ruby-build index 64a750a2..0434011b 100755 --- a/bin/ruby-build +++ b/bin/ruby-build @@ -1046,6 +1046,9 @@ elif [ "${PREFIX_PATH#/}" = "$PREFIX_PATH" ]; then PREFIX_PATH="${PWD}/${PREFIX_PATH}" fi +# Fail if number of arguments is not equal to 2 +[ "${#ARGUMENTS[@]}" -eq 2 ] || usage + if [ -z "$TMPDIR" ]; then TMP="/tmp" else diff --git a/test/arguments.bats b/test/arguments.bats new file mode 100644 index 00000000..3939cb08 --- /dev/null +++ b/test/arguments.bats @@ -0,0 +1,53 @@ +#!/usr/bin/env bats + +load test_helper + +@test "not enought arguments for ruby-build" { + # use empty inline definition so nothing gets built anyway + local definition="${TMP}/build-definition" + echo '' > "$definition" + + run ruby-build "$definition" + assert_failure + assert_output_contains 'usage: ruby-build' +} + +@test "extra arguments for ruby-build" { + # use empty inline definition so nothing gets built anyway + local definition="${TMP}/build-definition" + echo '' > "$definition" + + run ruby-build "$definition" 2.1.2 "${TMP}/install" + assert_failure + assert_output_contains 'usage: ruby-build' +} + +@test "extra arguments for rbenv-install" { + stub ruby-build "--lib : $BATS_TEST_DIRNAME/../bin/ruby-build --lib" + stub rbenv-hooks + stub rbenv-rehash + + run rbenv-install 2.1.1 2.1.2 + assert_failure + assert_output_contains 'Usage: rbenv install' + + unstub ruby-build + unstub rbenv-hooks + unstub rbenv-rehash +} + +@test "not enough arguments rbenv-uninstall" { + run rbenv-uninstall + assert_failure + assert_output_contains 'Usage: rbenv uninstall' +} + +@test "extra arguments for rbenv-uninstall" { + stub rbenv-hooks + + run rbenv-uninstall 2.1.1 2.1.2 + assert_failure + assert_output_contains 'Usage: rbenv uninstall' + + unstub rbenv-hooks +} From 38ae3b56b0bbfe0e201c9d32edc7faed439573c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 26 Nov 2014 20:40:45 -0800 Subject: [PATCH 2/8] Improve ruby-build test for too many arguments Use exactly the same arguments that would have `ruby-build` succeed at executing, but add one more extra to test argument counting. --- test/arguments.bats | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/arguments.bats b/test/arguments.bats index 3939cb08..b3a289da 100644 --- a/test/arguments.bats +++ b/test/arguments.bats @@ -17,7 +17,7 @@ load test_helper local definition="${TMP}/build-definition" echo '' > "$definition" - run ruby-build "$definition" 2.1.2 "${TMP}/install" + run ruby-build "$definition" "${TMP}/install" "" assert_failure assert_output_contains 'usage: ruby-build' } From 22c73bf3f5b210f3d1e761d71dfc368efa1de7e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 26 Nov 2014 19:45:52 -0800 Subject: [PATCH 3/8] Perform CLI argument validation as early as possible --- bin/rbenv-install | 5 ++--- bin/rbenv-uninstall | 5 ++--- bin/ruby-build | 5 ++--- 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/bin/rbenv-install b/bin/rbenv-install index 83d233db..d4dc8255 100755 --- a/bin/rbenv-install +++ b/bin/rbenv-install @@ -101,6 +101,8 @@ for option in "${OPTIONS[@]}"; do esac done +[ "${#ARGUMENTS[@]}" -le 1 ] || usage 1 + unset VERSION_NAME # The first argument contains the definition to install. If the @@ -111,9 +113,6 @@ DEFINITION="${ARGUMENTS[0]}" [ -n "$DEFINITION" ] || DEFINITION="$(rbenv-local 2>/dev/null || true)" [ -n "$DEFINITION" ] || usage 1 -# Fail if number of arguments is greater than 1 -[ "${#ARGUMENTS[@]}" -le 1 ] || usage 1 - # Define `before_install` and `after_install` functions that allow # plugin hooks to register a string of code for execution before or # after the installation process. diff --git a/bin/rbenv-uninstall b/bin/rbenv-uninstall index 834f048f..ff97c978 100755 --- a/bin/rbenv-uninstall +++ b/bin/rbenv-uninstall @@ -33,6 +33,8 @@ if [ "$1" = "-f" ] || [ "$1" = "--force" ]; then shift fi +[ "$#" -eq 1 ] || usage 1 + DEFINITION="$1" case "$DEFINITION" in "" | -* ) @@ -44,9 +46,6 @@ case "$DEFINITION" in ;; esac -# Fail if number of arguments is not equal 1 -[ "$#" -eq 1 ] || usage 1 - declare -a before_hooks after_hooks before_uninstall() { diff --git a/bin/ruby-build b/bin/ruby-build index 0434011b..1e367194 100755 --- a/bin/ruby-build +++ b/bin/ruby-build @@ -1022,6 +1022,8 @@ for option in "${OPTIONS[@]}"; do esac done +[ "${#ARGUMENTS[@]}" -eq 2 ] || usage 1 + DEFINITION_PATH="${ARGUMENTS[0]}" if [ -z "$DEFINITION_PATH" ]; then usage @@ -1046,9 +1048,6 @@ elif [ "${PREFIX_PATH#/}" = "$PREFIX_PATH" ]; then PREFIX_PATH="${PWD}/${PREFIX_PATH}" fi -# Fail if number of arguments is not equal to 2 -[ "${#ARGUMENTS[@]}" -eq 2 ] || usage - if [ -z "$TMPDIR" ]; then TMP="/tmp" else From 4a6b9280bb5538dacb7b3172e3395165877f6fee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 26 Nov 2014 19:51:11 -0800 Subject: [PATCH 4/8] Provide CLI usage help on stout/stderr appropriately When requested via `-h|--help`, usage text should be displayed on stdout and the exit status should be 0. When usage text is shown due to invalid arguments, it should be printed to stderr and exit status should be 1. --- bin/rbenv-install | 6 +++--- bin/rbenv-uninstall | 8 ++------ bin/ruby-build | 16 +++++++--------- 3 files changed, 12 insertions(+), 18 deletions(-) diff --git a/bin/rbenv-install b/bin/rbenv-install index d4dc8255..543b709b 100755 --- a/bin/rbenv-install +++ b/bin/rbenv-install @@ -96,12 +96,12 @@ for option in "${OPTIONS[@]}"; do exec ruby-build --version ;; * ) - usage 1 + usage 1 >&2 ;; esac done -[ "${#ARGUMENTS[@]}" -le 1 ] || usage 1 +[ "${#ARGUMENTS[@]}" -le 1 ] || usage 1 >&2 unset VERSION_NAME @@ -111,7 +111,7 @@ unset VERSION_NAME # version is not specified. DEFINITION="${ARGUMENTS[0]}" [ -n "$DEFINITION" ] || DEFINITION="$(rbenv-local 2>/dev/null || true)" -[ -n "$DEFINITION" ] || usage 1 +[ -n "$DEFINITION" ] || usage 1 >&2 # Define `before_install` and `after_install` functions that allow # plugin hooks to register a string of code for execution before or diff --git a/bin/rbenv-uninstall b/bin/rbenv-uninstall index ff97c978..b1948341 100755 --- a/bin/rbenv-uninstall +++ b/bin/rbenv-uninstall @@ -33,16 +33,12 @@ if [ "$1" = "-f" ] || [ "$1" = "--force" ]; then shift fi -[ "$#" -eq 1 ] || usage 1 +[ "$#" -eq 1 ] || usage 1 >&2 DEFINITION="$1" case "$DEFINITION" in "" | -* ) - # We can remove the sed fallback once rbenv 0.4.0 is widely available. - { rbenv-help uninstall 2>/dev/null || - sed -ne '/^#/!q;s/.\{1,2\}//;1,4d;p' < "$0" - } >&2 - exit 1 + usage 1 >&2 ;; esac diff --git a/bin/ruby-build b/bin/ruby-build index 1e367194..900644fc 100755 --- a/bin/ruby-build +++ b/bin/ruby-build @@ -957,11 +957,9 @@ usage() { { version echo "usage: ruby-build [-k|--keep] [-v|--verbose] [-p|--patch] definition prefix" echo " ruby-build --definitions" - } >&2 + } >&1 - if [ -z "$1" ]; then - exit 1 - fi + [ -z "$1" ] || exit "$1" } list_definitions() { @@ -992,14 +990,14 @@ parse_options "$@" for option in "${OPTIONS[@]}"; do case "$option" in "h" | "help" ) - usage without_exiting + usage { echo echo " -k/--keep Do not remove source tree after installation" echo " -v/--verbose Verbose mode: print compilation status to stdout" echo " -p/--patch Apply a patch from stdin before building" echo " --definitions List all built-in definitions" echo - } >&2 + } >&1 exit 0 ;; "definitions" ) @@ -1022,11 +1020,11 @@ for option in "${OPTIONS[@]}"; do esac done -[ "${#ARGUMENTS[@]}" -eq 2 ] || usage 1 +[ "${#ARGUMENTS[@]}" -eq 2 ] || usage 1 >&2 DEFINITION_PATH="${ARGUMENTS[0]}" if [ -z "$DEFINITION_PATH" ]; then - usage + usage 1 >&2 elif [ ! -f "$DEFINITION_PATH" ]; then for DEFINITION_DIR in "${RUBY_BUILD_DEFINITIONS[@]}"; do if [ -f "${DEFINITION_DIR}/${DEFINITION_PATH}" ]; then @@ -1043,7 +1041,7 @@ fi PREFIX_PATH="${ARGUMENTS[1]}" if [ -z "$PREFIX_PATH" ]; then - usage + usage 1 >&2 elif [ "${PREFIX_PATH#/}" = "$PREFIX_PATH" ]; then PREFIX_PATH="${PWD}/${PREFIX_PATH}" fi From 960e183d4ce5e0fbf25b6b3bcbcaebd82eae4d77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 26 Nov 2014 20:01:38 -0800 Subject: [PATCH 5/8] Extract ruby-build usage from comments like we do in rbenv --- bin/ruby-build | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/bin/ruby-build b/bin/ruby-build index 900644fc..914e72d2 100755 --- a/bin/ruby-build +++ b/bin/ruby-build @@ -1,4 +1,13 @@ #!/usr/bin/env bash +# +# usage: ruby-build [-kvp] +# ruby-build --definitions +# +# -k/--keep Do not remove source tree after installation +# -v/--verbose Verbose mode: print compilation status to stdout +# -p/--patch Apply a patch from stdin before building +# --definitions List all built-in definitions +# RUBY_BUILD_VERSION="20141113" @@ -954,11 +963,7 @@ version() { } usage() { - { version - echo "usage: ruby-build [-k|--keep] [-v|--verbose] [-p|--patch] definition prefix" - echo " ruby-build --definitions" - } >&1 - + sed -ne '/^#/!q;s/.\{1,2\}//;1,2d;p' < "$0" [ -z "$1" ] || exit "$1" } @@ -990,15 +995,9 @@ parse_options "$@" for option in "${OPTIONS[@]}"; do case "$option" in "h" | "help" ) - usage - { echo - echo " -k/--keep Do not remove source tree after installation" - echo " -v/--verbose Verbose mode: print compilation status to stdout" - echo " -p/--patch Apply a patch from stdin before building" - echo " --definitions List all built-in definitions" - echo - } >&1 - exit 0 + version + echo + usage 0 ;; "definitions" ) list_definitions From 3ae43a44e261047a5f4250584b04b923b8ed7479 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 26 Nov 2014 20:03:01 -0800 Subject: [PATCH 6/8] Capitalize "Usage:" header for consistency with rbenv help system --- bin/ruby-build | 2 +- test/arguments.bats | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/bin/ruby-build b/bin/ruby-build index 914e72d2..54a31406 100755 --- a/bin/ruby-build +++ b/bin/ruby-build @@ -1,6 +1,6 @@ #!/usr/bin/env bash # -# usage: ruby-build [-kvp] +# Usage: ruby-build [-kvp] # ruby-build --definitions # # -k/--keep Do not remove source tree after installation diff --git a/test/arguments.bats b/test/arguments.bats index b3a289da..352fcff6 100644 --- a/test/arguments.bats +++ b/test/arguments.bats @@ -9,7 +9,7 @@ load test_helper run ruby-build "$definition" assert_failure - assert_output_contains 'usage: ruby-build' + assert_output_contains 'Usage: ruby-build' } @test "extra arguments for ruby-build" { @@ -19,7 +19,7 @@ load test_helper run ruby-build "$definition" "${TMP}/install" "" assert_failure - assert_output_contains 'usage: ruby-build' + assert_output_contains 'Usage: ruby-build' } @test "extra arguments for rbenv-install" { From 96d9cbe77c089ce68ebf134fefdda11759799a5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 26 Nov 2014 20:39:13 -0800 Subject: [PATCH 7/8] Move rbenv-install/uninstall tests to `rbenv.bats` --- test/arguments.bats | 30 ------------------------------ test/rbenv.bats | 26 ++++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 30 deletions(-) diff --git a/test/arguments.bats b/test/arguments.bats index 352fcff6..77eb91cf 100644 --- a/test/arguments.bats +++ b/test/arguments.bats @@ -21,33 +21,3 @@ load test_helper assert_failure assert_output_contains 'Usage: ruby-build' } - -@test "extra arguments for rbenv-install" { - stub ruby-build "--lib : $BATS_TEST_DIRNAME/../bin/ruby-build --lib" - stub rbenv-hooks - stub rbenv-rehash - - run rbenv-install 2.1.1 2.1.2 - assert_failure - assert_output_contains 'Usage: rbenv install' - - unstub ruby-build - unstub rbenv-hooks - unstub rbenv-rehash -} - -@test "not enough arguments rbenv-uninstall" { - run rbenv-uninstall - assert_failure - assert_output_contains 'Usage: rbenv uninstall' -} - -@test "extra arguments for rbenv-uninstall" { - stub rbenv-hooks - - run rbenv-uninstall 2.1.1 2.1.2 - assert_failure - assert_output_contains 'Usage: rbenv uninstall' - - unstub rbenv-hooks -} diff --git a/test/rbenv.bats b/test/rbenv.bats index dc189f07..3e441daf 100644 --- a/test/rbenv.bats +++ b/test/rbenv.bats @@ -146,3 +146,29 @@ ${RBENV_ROOT}/plugins/bar/share/ruby-build ${RBENV_ROOT}/plugins/foo/share/ruby-build OUT } + +@test "not enough arguments for rbenv-install" { + stub_ruby_build + run rbenv-install + assert_failure + assert_output_contains 'Usage: rbenv install' +} + +@test "too many arguments for rbenv-install" { + stub_ruby_build + run rbenv-install 2.1.1 2.1.2 + assert_failure + assert_output_contains 'Usage: rbenv install' +} + +@test "not enough arguments rbenv-uninstall" { + run rbenv-uninstall + assert_failure + assert_output_contains 'Usage: rbenv uninstall' +} + +@test "too many arguments for rbenv-uninstall" { + run rbenv-uninstall 2.1.1 2.1.2 + assert_failure + assert_output_contains 'Usage: rbenv uninstall' +} From 34246f9f2e85faf7a1e87d3ec358625dfef5b6f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 26 Nov 2014 20:40:09 -0800 Subject: [PATCH 8/8] Have rbenv-uninstall respect `-h|--help` --- bin/rbenv-uninstall | 4 ++++ test/rbenv.bats | 13 +++++++++++++ 2 files changed, 17 insertions(+) diff --git a/bin/rbenv-uninstall b/bin/rbenv-uninstall index b1948341..ea65c45a 100755 --- a/bin/rbenv-uninstall +++ b/bin/rbenv-uninstall @@ -27,6 +27,10 @@ if [ -z "$RBENV_ROOT" ]; then RBENV_ROOT="${HOME}/.rbenv" fi +if [ "$1" = "-h" ] || [ "$1" = "--help" ]; then + usage 0 +fi + unset FORCE if [ "$1" = "-f" ] || [ "$1" = "--force" ]; then FORCE=true diff --git a/test/rbenv.bats b/test/rbenv.bats index 3e441daf..b51ed167 100644 --- a/test/rbenv.bats +++ b/test/rbenv.bats @@ -161,6 +161,13 @@ OUT assert_output_contains 'Usage: rbenv install' } +@test "show help for rbenv-install" { + stub_ruby_build + run rbenv-install -h + assert_success + assert_output_contains 'Usage: rbenv install' +} + @test "not enough arguments rbenv-uninstall" { run rbenv-uninstall assert_failure @@ -172,3 +179,9 @@ OUT assert_failure assert_output_contains 'Usage: rbenv uninstall' } + +@test "show help for rbenv-uninstall" { + run rbenv-uninstall -h + assert_success + assert_output_contains 'Usage: rbenv uninstall' +}