diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index bc11cb7..b72f8a5 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -2,8 +2,8 @@ name: test on: [push, pull_request] jobs: - test-make-tarball: - name: "Test that we can build the binary distro and package it as a tarball" + test-env-scripts: + name: "Test the env scripts packaged in the dune binary distro" runs-on: ${{ matrix.os }} strategy: matrix: @@ -33,12 +33,56 @@ jobs: run: ./make_tarball.sh $NAME ${{ matrix.output }} - name: "Extract the tarball" run: tar xf $NAME.tar.gz - - name: "Test that we can env scripts and the dune executable is in the expected place" + - name: "Test that we can run the env scripts and the dune executable is in the expected place" run: | bash -c '. $NAME/share/dune/env/env.bash; __dune_env $PWD/$NAME; test $(which dune) = $PWD/$NAME/bin/dune' zsh -c '. $NAME/share/dune/env/env.zsh; __dune_env $PWD/$NAME; test $(which dune) = $PWD/$NAME/bin/dune' - fish -c '. $NAME/share/dune/env/env.fish; __dune_env $PWD/$NAME; test $(which dune) = $PWD/$NAME/bin/dune' sh -c '. $NAME/share/dune/env/env.sh; __dune_env $PWD/$NAME; test $(which dune) = $PWD/$NAME/bin/dune' + fish -c '. $NAME/share/dune/env/env.fish; __dune_env $PWD/$NAME; test $(which dune) = $PWD/$NAME/bin/dune' + - name: "Exercise that running __dune_env twice is benign since some logic only runs the second time they are sourced" + run: | + bash -c '. $NAME/share/dune/env/env.bash; __dune_env $PWD/$NAME; __dune_env $PWD/$NAME; test $(which dune) = $PWD/$NAME/bin/dune' + zsh -c '. $NAME/share/dune/env/env.zsh; __dune_env $PWD/$NAME; __dune_env $PWD/$NAME; test $(which dune) = $PWD/$NAME/bin/dune' + sh -c '. $NAME/share/dune/env/env.sh; __dune_env $PWD/$NAME; __dune_env $PWD/$NAME; test $(which dune) = $PWD/$NAME/bin/dune' + fish -c '. $NAME/share/dune/env/env.fish; __dune_env $PWD/$NAME; __dune_env $PWD/$NAME; test $(which dune) = $PWD/$NAME/bin/dune' + - name: "Make a fake opam switch and add it to PATH in between calls to __dune_env simulating a shell config with both opam and dune installed" + run: | + # Make a fake opam installation of dune + mkdir -p ~/.opam/default/bin + printf '%s\necho "%s"' '#!/bin/sh' 'fake dune' > ~/.opam/default/bin/dune + chmod a+x ~/.opam/default/bin/dune + + # Test adding the fake opam switch to the path after __dune_env + # causes opam's dune to take precedence, and that calling __dune_env + # a second time causes the binary distro's dune to take precedence. + bash -c '. $NAME/share/dune/env/env.bash; __dune_env $PWD/$NAME; export PATH=~/.opam/default/bin:$PATH; test $(which dune) = ~/.opam/default/bin/dune' + bash -c '. $NAME/share/dune/env/env.bash; __dune_env $PWD/$NAME; export PATH=~/.opam/default/bin:$PATH; __dune_env $PWD/$NAME; test $(which dune) = $PWD/$NAME/bin/dune' + zsh -c '. $NAME/share/dune/env/env.zsh; __dune_env $PWD/$NAME; export PATH=~/.opam/default/bin:$PATH; test $(which dune) = ~/.opam/default/bin/dune' + zsh -c '. $NAME/share/dune/env/env.zsh; __dune_env $PWD/$NAME; export PATH=~/.opam/default/bin:$PATH; __dune_env $PWD/$NAME; test $(which dune) = $PWD/$NAME/bin/dune' + sh -c '. $NAME/share/dune/env/env.sh; __dune_env $PWD/$NAME; export PATH=~/.opam/default/bin:$PATH; test $(which dune) = ~/.opam/default/bin/dune' + sh -c '. $NAME/share/dune/env/env.sh; __dune_env $PWD/$NAME; export PATH=~/.opam/default/bin:$PATH; __dune_env $PWD/$NAME; test $(which dune) = $PWD/$NAME/bin/dune' + fish -c '. $NAME/share/dune/env/env.fish; __dune_env $PWD/$NAME; fish_add_path --prepend --move --path ~/.opam/default/bin; test $(which dune) = ~/.opam/default/bin/dune' + fish -c '. $NAME/share/dune/env/env.fish; __dune_env $PWD/$NAME; fish_add_path --prepend --move --path ~/.opam/default/bin; __dune_env $PWD/$NAME; test $(which dune) = $PWD/$NAME/bin/dune' + + - name: "Test that opam paths later in the PATH variable are preserved by multiple calls to __dune_env" + run: | + printf '%s\necho "%s"' '#!/bin/sh' 'only in opam bin' > ~/.opam/default/bin/only-in-opam-bin + chmod a+x ~/.opam/default/bin/only-in-opam-bin + bash -c '. $NAME/share/dune/env/env.bash; export PATH=~/.opam/default/bin:$PATH; __dune_env $PWD/$NAME; __dune_env $PWD/$NAME; only-in-opam-bin' + zsh -c '. $NAME/share/dune/env/env.zsh; export PATH=~/.opam/default/bin:$PATH; __dune_env $PWD/$NAME; __dune_env $PWD/$NAME; only-in-opam-bin' + sh -c '. $NAME/share/dune/env/env.sh; export PATH=~/.opam/default/bin:$PATH; __dune_env $PWD/$NAME; __dune_env $PWD/$NAME; only-in-opam-bin' + fish -c '. $NAME/share/dune/env/env.fish; fish_add_path --prepend --move --path ~/.opam/default/bin; __dune_env $PWD/$NAME; __dune_env $PWD/$NAME; only-in-opam-bin' + + - name: "Test that non-opam changes to PATH between two calls to __dune_env have their effect preserved" + run: | + # Simulate the user setting up a custom version of dune (e.g. what a dune developer might do to try out local changes) + mkdir -p ~/bin + printf '%s\necho "%s"' '#!/bin/sh' 'fake dune' > ~/bin/dune + chmod a+x ~/bin/dune + bash -c '. $NAME/share/dune/env/env.bash; __dune_env $PWD/$NAME; export PATH=~/bin:$PATH; __dune_env $PWD/$NAME; test $(which dune) = ~/bin/dune' + zsh -c '. $NAME/share/dune/env/env.zsh ; __dune_env $PWD/$NAME; export PATH=~/bin:$PATH; __dune_env $PWD/$NAME; test $(which dune) = ~/bin/dune' + sh -c '. $NAME/share/dune/env/env.sh; __dune_env $PWD/$NAME; export PATH=~/bin:$PATH; __dune_env $PWD/$NAME; test $(which dune) = ~/bin/dune' + fish -c '. $NAME/share/dune/env/env.fish; __dune_env $PWD/$NAME; fish_add_path --prepend --move --path ~/bin; __dune_env $PWD/$NAME; test $(which dune) = ~/bin/dune' test-make-tarball-shellcheck: @@ -65,5 +109,4 @@ jobs: sudo apt-get install -y shellcheck - name: "Run shellcheck on the shell env script" run: | - # exclude warning for sourcing missing external file shellcheck extra/share/dune/env/env.sh extra/share/dune/env/env.bash diff --git a/extra/share/dune/env/env.bash b/extra/share/dune/env/env.bash index 0444e32..043f17e 100644 --- a/extra/share/dune/env/env.bash +++ b/extra/share/dune/env/env.bash @@ -1,5 +1,23 @@ #!/usr/bin/env bash +# Equivalent to `which dune`, but `which` might not be installed. Bash's `type` +# supports -P which acts like `which` but is guaranteed to be installed, but +# older versions of bash might not support -P so we'll err on the side of +# caution and implement it here in the interest of portability. +__dune_which() { + echo "$PATH" | \ + tr ':' '\n' |\ + while read -r entry; do + if test -x "$entry/dune"; then + echo "$entry/dune" + return 0 + fi + done + return 1 +} + +export __DUNE_SETUP_STATE="${__DUNE_SETUP_STATE:-incomplete}" + __dune_env() { if [ "$#" != "1" ]; then echo "__dune_env expected 1 argument, got $#" @@ -7,24 +25,81 @@ __dune_env() { fi local ROOT="$1" - # Add dune to PATH unless it's already present. - # Affix colons on either side of $PATH to simplify matching (based on - # rustup's env script). - case :"$PATH": in - *:"$ROOT/bin":*) - # Do nothing since the bin directory is already in PATH. + case "$__DUNE_SETUP_STATE" in + incomplete) + # This is the first time __dune_env has been called, so attempt to set up + # the environment for dune and record in the global variable + # __DUNE_SETUP_STATE whether or not it succeeded. + + # Add dune to PATH unless it's already present. + # Affix colons on either side of $PATH to simplify matching (based on + # rustup's env script). + case :"$PATH": in + *:"$ROOT/bin":*) + # Do nothing since the bin directory is already in PATH. + ;; + *) + export PATH="$ROOT/bin:$PATH" + ;; + esac + + if [ "$(__dune_which)" = "$ROOT/bin/dune" ]; then + export __DUNE_SETUP_STATE=success + + # Only load completions if the shell is interactive. + if [ -t 0 ]; then + # Load bash completions for dune. + # Suppress warning from shellcheck as it can't see the completion script. + # shellcheck disable=SC1091 + . "$ROOT"/share/bash-completion/completions/dune + fi + else + # Despite modifying the environment, running `dune` would resolve to + # the wrong dune instance. This can happen if the bin directory + # containing our dune was already present in PATH behind the bin + # directory of the default opam switch. + # TODO Possibly print a warning/hint in this case to help users fix + # their environment. + export __DUNE_SETUP_STATE=failure + fi + ;; + success) + # This is at least the second time this function was called in the + # current environment (possibly by the user sourcing their shell config + # or nesting shell sessions), and the previous time it was called it + # successfully modified the environment to give precedence to our dune + # executable. check that our dune still has precedence, and attempt to + # undo any opam-specific path shenanigans that have taken place since the + # last time this function was called. + if [ "$(__dune_which)" != "$ROOT/bin/dune" ]; then + case :"$PATH": in + *:"$ROOT/bin":*) + # Remove all opam bin directories from the PATH variable + # between the start of the PATH variable and the first + # occurrence of the dune binary distro's bin directory. + PATH_MAYBE_FIXED=$(echo "$PATH" | \ + tr ':' '\n' |\ + sed "1,\#^$ROOT/bin\$# { \#^$HOME/.opam#d; }" |\ + paste -sd ':' -) + # Only commit the change if it actually fixed the problem. + if [ "$(PATH=$PATH_MAYBE_FIXED __dune_which)" = "$ROOT/bin/dune" ]; then + export PATH="$PATH_MAYBE_FIXED" + else + # The attempt to fix the PATH variable failed, so give up. + export __DUNE_SETUP_STATE=failure + fi + ;; + *) + # The dune binary distro is no longer in the PATH variable at + # all, so give up. + export __DUNE_SETUP_STATE=failure + ;; + esac + fi ;; - *) - # Prepending path in case a system-installed dune needs to be overridden - export PATH="$ROOT/bin:$PATH" + failure) + # A previous attempt at modifying the environment failed, so don't + # attempt further environment modifications here. ;; esac - - # Only load completions if the shell is interactive. - if [ -t 0 ]; then - # Load bash completions for dune. - # Suppress warning from shellcheck as it can't see the completion script. - # shellcheck disable=SC1091 - . "$ROOT"/share/bash-completion/completions/dune - fi } diff --git a/extra/share/dune/env/env.fish b/extra/share/dune/env/env.fish index e34cc21..62ae5d8 100644 --- a/extra/share/dune/env/env.fish +++ b/extra/share/dune/env/env.fish @@ -1,8 +1,63 @@ #!/usr/bin/env fish +if ! set -q __DUNE_SETUP_STATE + set --export __DUNE_SETUP_STATE "incomplete" +end + function __dune_env - set dune_bin_path "$argv[1]/bin" - if ! contains "$dune_bin_path" $PATH; - fish_add_path --prepend --path "$dune_bin_path" + if [ "$(count $argv)" != "1" ] + echo "__dune_env expected 1 argument, got $(count $argv)" + end + + set --local dune_root "$argv[1]" + set --local dune_bin "$dune_root/bin" + + switch "$__DUNE_SETUP_STATE" + case incomplete + if ! contains "$dune_bin" $PATH; and [ -d "$dune_bin" ] + fish_add_path --prepend --path "$dune_bin" + end + if [ "$(type -P dune)" = "$dune_bin/dune" ] + set --export __DUNE_SETUP_STATE "success" + else + # Despite modifying the environment, running `dune` would resolve to + # the wrong dune instance. This can happen if the bin directory + # containing our dune was already present in PATH behind the bin + # directory of the default opam switch. + # TODO Possibly print a warning/hint in this case to help users fix + # their environment. + set --export __DUNE_SETUP_STATE "failure" + end + case success + # This is at least the second time this function was called in the + # current environment (possibly by the user sourcing their shell config + # or nesting shell sessions), and the previous time it was called it + # successfully modified the environment to give precedence to our dune + # executable. check that our dune still has precedence, and attempt to + # undo any opam-specific path shenanigans that have taken place since the + # last time this function was called. + if [ "$(type -P dune)" != "$dune_bin/dune" ] + if contains "$dune_bin" $PATH + # Remove all opam bin directories from the PATH variable + # between the start of the PATH variable and the first + # occurrence of the dune binary distro's bin directory. + set --local PATH_maybe_fixed $(printf "%s\n" $PATH | \ + sed "1,\#^$dune_bin\$# { \#^$HOME/.opam#d; }") + # Only commit the change if it actually fixed the problem. + if [ "$(PATH=$PATH_maybe_fixed type -P dune)" = "$dune_bin/dune" ] + set --export PATH $PATH_maybe_fixed + else + # The attempt to fix the PATH variable failed, so give up. + set --export __DUNE_SETUP_STATE "failure" + end + else + # The dune binary distro is no longer in the PATH variable at + # all, so give up. + set --export __DUNE_SETUP_STATE "failure" + end + end + case failure + # A previous attempt at modifying the environment failed, so don't + # attempt further environment modifications here. end end diff --git a/extra/share/dune/env/env.sh b/extra/share/dune/env/env.sh index f6ad1dc..9dc5d48 100644 --- a/extra/share/dune/env/env.sh +++ b/extra/share/dune/env/env.sh @@ -1,22 +1,94 @@ #!/bin/sh +# Equivalent to `which dune`, but `which` might not be installed. +__dune_which() { + echo "$PATH" | \ + tr ':' '\n' |\ + while read -r entry; do + if test -x "$entry/dune"; then + echo "$entry/dune" + return 0 + fi + done + return 1 +} + +export __DUNE_SETUP_STATE="${__DUNE_SETUP_STATE:-incomplete}" + __dune_env() { if [ "$#" != "1" ]; then echo "__dune_env expected 1 argument, got $#" return fi - __dune_root="$1" + __DUNE_ROOT="$1" - # Add dune to PATH unless it's already present. - # Affix colons on either side of $PATH to simplify matching (based on - # rustup's env script). - case :"$PATH": in - *:"$__dune_root/bin":*) - # Do nothing since the bin directory is already in PATH. + case "$__DUNE_SETUP_STATE" in + incomplete) + # This is the first time __dune_env has been called, so attempt to set up + # the environment for dune and record in the global variable + # __DUNE_SETUP_STATE whether or not it succeeded. + + # Add dune to PATH unless it's already present. + # Affix colons on either side of $PATH to simplify matching (based on + # rustup's env script). + case :"$PATH": in + *:"$__DUNE_ROOT/bin":*) + # Do nothing since the bin directory is already in PATH. + ;; + *) + export PATH="$__DUNE_ROOT/bin:$PATH" + ;; + esac + + if [ "$(__dune_which)" = "$__DUNE_ROOT/bin/dune" ]; then + export __DUNE_SETUP_STATE=success + else + # Despite modifying the environment, running `dune` would resolve to + # the wrong dune instance. This can happen if the bin directory + # containing our dune was already present in PATH behind the bin + # directory of the default opam switch. + # TODO Possibly print a warning/hint in this case to help users fix + # their environment. + export __DUNE_SETUP_STATE=failure + fi + ;; + success) + # This is at least the second time this function was called in the + # current environment (possibly by the user sourcing their shell config + # or nesting shell sessions), and the previous time it was called it + # successfully modified the environment to give precedence to our dune + # executable. check that our dune still has precedence, and attempt to + # undo any opam-specific path shenanigans that have taken place since the + # last time this function was called. + if [ "$(__dune_which)" != "$__DUNE_ROOT/bin/dune" ]; then + case :"$PATH": in + *:"$__DUNE_ROOT/bin":*) + # Remove all opam bin directories from the PATH variable + # between the start of the PATH variable and the first + # occurrence of the dune binary distro's bin directory. + PATH_MAYBE_FIXED=$(echo "$PATH" | \ + tr ':' '\n' |\ + sed "1,\#^$__DUNE_ROOT/bin\$# { \#^$HOME/.opam#d; }" |\ + paste -sd ':' -) + # Only commit the change if it actually fixed the problem. + if [ "$(PATH=$PATH_MAYBE_FIXED __dune_which)" = "$__DUNE_ROOT/bin/dune" ]; then + export PATH="$PATH_MAYBE_FIXED" + else + # The attempt to fix the PATH variable failed, so give up. + export __DUNE_SETUP_STATE=failure + fi + ;; + *) + # The dune binary distro is no longer in the PATH variable at + # all, so give up. + export __DUNE_SETUP_STATE=failure + ;; + esac + fi ;; - *) - # Prepending path in case a system-installed dune needs to be overridden - export PATH="$__dune_root/bin:$PATH" + failure) + # A previous attempt at modifying the environment failed, so don't + # attempt further environment modifications here. ;; esac } diff --git a/extra/share/dune/env/env.zsh b/extra/share/dune/env/env.zsh index 081325e..e6a049f 100644 --- a/extra/share/dune/env/env.zsh +++ b/extra/share/dune/env/env.zsh @@ -1,5 +1,20 @@ #!/usr/bin/env zsh +# Equivalent to `which dune`, but `which` might not be installed. +__dune_which() { + echo "$PATH" | \ + tr ':' '\n' |\ + while read -r entry; do + if test -x "$entry/dune"; then + echo "$entry/dune" + return 0 + fi + done + return 1 +} + +export __DUNE_SETUP_STATE="${__DUNE_SETUP_STATE:-incomplete}" + __dune_env() { if [ "$#" != "1" ]; then echo "__dune_env expected 1 argument, got $#" @@ -7,25 +22,83 @@ __dune_env() { fi local ROOT="$1" - # Add dune to PATH unless it's already present. - # Affix colons on either side of $PATH to simplify matching (based on - # rustup's env script). - case :"$PATH": in - *:"$ROOT/bin":*) - # Do nothing since the bin directory is already in PATH. + case "$__DUNE_SETUP_STATE" in + incomplete) + # This is the first time __dune_env has been called, so attempt to set up + # the environment for dune and record in the global variable + # __DUNE_SETUP_STATE whether or not it succeeded. + + # Add dune to PATH unless it's already present. + # Affix colons on either side of $PATH to simplify matching (based on + # rustup's env script). + case :"$PATH": in + *:"$ROOT/bin":*) + # Do nothing since the bin directory is already in PATH. + ;; + *) + export PATH="$ROOT/bin:$PATH" + ;; + esac + + if [ "$(__dune_which)" = "$ROOT/bin/dune" ]; then + export __DUNE_SETUP_STATE=success + + # Only load completions if the shell is interactive. + if [ -t 0 ]; then + # completions via bash compat + autoload -Uz compinit bashcompinit + compinit + bashcompinit + . "$ROOT"/share/bash-completion/completions/dune + fi + + else + # Despite modifying the environment, running `dune` would resolve to + # the wrong dune instance. This can happen if the bin directory + # containing our dune was already present in PATH behind the bin + # directory of the default opam switch. + # TODO Possibly print a warning/hint in this case to help users fix + # their environment. + export __DUNE_SETUP_STATE=failure + fi + ;; + success) + # This is at least the second time this function was called in the + # current environment (possibly by the user sourcing their shell config + # or nesting shell sessions), and the previous time it was called it + # successfully modified the environment to give precedence to our dune + # executable. check that our dune still has precedence, and attempt to + # undo any opam-specific path shenanigans that have taken place since the + # last time this function was called. + if [ "$(__dune_which)" != "$ROOT/bin/dune" ]; then + case :"$PATH": in + *:"$ROOT/bin":*) + # Remove all opam bin directories from the PATH variable + # between the start of the PATH variable and the first + # occurrence of the dune binary distro's bin directory. + PATH_MAYBE_FIXED=$(echo "$PATH" | \ + tr ':' '\n' |\ + sed "1,\#^$ROOT/bin\$# { \#^$HOME/.opam#d; }" |\ + paste -sd ':' -) + # Only commit the change if it actually fixed the problem. + if [ "$(PATH=$PATH_MAYBE_FIXED __dune_which)" = "$ROOT/bin/dune" ]; then + export PATH="$PATH_MAYBE_FIXED" + else + # The attempt to fix the PATH variable failed, so give up. + export __DUNE_SETUP_STATE=failure + fi + ;; + *) + # The dune binary distro is no longer in the PATH variable at + # all, so give up. + export __DUNE_SETUP_STATE=failure + ;; + esac + fi ;; - *) - # Prepending path in case a system-installed dune needs to be overridden - export PATH="$ROOT/bin:$PATH" + failure) + # A previous attempt at modifying the environment failed, so don't + # attempt further environment modifications here. ;; esac - - # Only load completions if the shell is interactive. - if [ -t 0 ]; then - # completions via bash compat - autoload -Uz compinit bashcompinit - compinit - bashcompinit - . "$ROOT"/share/bash-completion/completions/dune - fi }