feat: multiple install paths with fallbacks

[mistydemeo]

This adds a new feature which allows specifying multiple paths in install-path instead of a single path. If this is set, then we use the next paths in the set as fallbacks in case the earlier options aren't available. As a result, path errors for HomeSubdir and EnvSubdir that were formerly fatal instead cause us to fall back to the next case; the only actual fatal error is if we exhaust the array of options and find nothing viable.

I've added tests for two fallback options and confirmed they work, along with a test for what happens if the first path in the array is valid and we don't perform a fallback.

I haven't fully massaged the spacing in the jinja templates to keep it from adding extra newlines in places that don't need them. I did add a comment for people reading the generated script who might be confused by what looks like a pointless if statement.

This isn't yet backwards compatible with existing Cargo.tomls for users already using install-path. I've temporarily adjusted install-path to always be an array, which will cause errors for users already using it as a string. We'll want to adjust the parsing so that a string parses to a one-length array.

[Gankra]

Some pseudo-jinja-rust-shell code i think the impl should look like:

// start with nowhere to install
_install_dir = ""

// First off, if we get handed FORCE_INSTALL_DIR, just use that
if $CARGO_DIST_FORCE_INSTALL_DIR {
   // two major families of install-strategy, that the updater 
   %if strategies.contains("cargo_home") && strategies.len == 1
      _install_dir="$CARGO_DIST_FORCE_INSTALL_DIR/bin"
      _env_script_path="$CARGO_DIST_FORCE_INSTALL_DIR/env"
      _install_dir_expr="$(echo "$CARGO_DIST_FORCE_INSTALL_DIR/bin" | sed "s,$HOME,\$HOME,")"
      _env_script_path_expr="$(echo "$CARGO_DIST_FORCE_INSTALL_DIR/env" | sed "s,$HOME,\$HOME,")"
   %elseif !strategies.contains("cargo_home")
      _install_dir="$CARGO_DIST_FORCE_INSTALL_DIR"
      _env_script_path="$CARGO_DIST_FORCE_INSTALL_DIR/env"
      _install_dir_expr="$(echo "$CARGO_DIST_FORCE_INSTALL_DIR" | sed "s,$HOME,\$HOME,")"
      _env_script_path_expr="$(echo "$CARGO_DIST_FORCE_INSTALL_DIR/env" | sed "s,$HOME,\$HOME,")"
   %else
      jinjaerror "cannot combine CARGO_HOME with other strategies"
   %endif
}

// Try to find an appropriate place to install
%for strategy in strategies
  // only try other strategies if one hasn't worked
  if _install_dir.is_empty() {
    %if strategy.kind == "cargo_home"
      if $CARGO_HOME {
        _install_dir = "..."
        // ...
      } else if $HOME {
         _install_dir = "..."
        // ...
      }
    %elseif strategy.kind == "home"
      if $HOME {
        _install_dir = "..."
        // ...
      }
    %elseif strategy.kind == "env"
      if ${strategy.envvar} {
        _install_dir = "..."
        // ...
      }
    %else
      jinjaerror "unimplemented strategy"
    %endif
  }
%endfor

if _install_dir.is_empty() {
  error "oopsie whoopsie no install dir"
}

// ok install paths handled, now do the rest of setup
...

Notably:

• FORCE by construction avoids trying to think about how things were previously installed
• the resulting shell is "as if" a human hand-wrote it (no functionality they can't use)
• lots of logic shared between modes

[mistydemeo]

I think your suggestion makes sense, and I did write this with the consideration we might have some more ideas about #944. In the absence of that, yeah, we have to avoid mixing strategies. I wonder if it makes more sense to consider making this an error in Rust at the place we produce the installer stuff before feeding it into jinja, or even when loading the user's config, in order to make sure we get a nicer-looking error.

[Gankra]

I wonder if it makes more sense to consider making this an error in Rust at the place we produce the installer stuff before feeding it into jinja, or even when loading the user's config, in order to make sure we get a nicer-looking error.

oh yes absolutely, i just decided to fold it into the jinja to avoid making the idea more complicated to express

[Gankra]

The shell is looking great, just some minor nits.

[Gankra]

you don't need this else, it's implicit in the fact that CARGO_DIST_FORCE_INSTALL_DIR will set _install_dir, which nicely makes all the options look like a clean cascade :)

[mistydemeo]

This isn't trying to catch it being unset from the above case. We're generating an if [] for each of the for install_path in install_paths cases, so it's letting item 2 in the array check if it was assigned in case 1, and so on.

[Gankra]

yes, i'm just saying we can omit the else because every case has this if-guard, making the whole thing feel a lot more natural and removing some indentation (i applied this change)

[Gankra]

this comment should be shared between both paths

[Gankra]

moved this up into the main comment explaining early/late

[Gankra]

can/should we restructure this so it looks like the rest of the cascade options..? (especially the final error condition)

[Gankra]

punted on this. it can be done but it's wonkier

[Gankra]

Can this all be replaced with the sed approach?

[mistydemeo]

Sort of. This is slightly more restricted than the sed approach, so any changes we make here would change the behaviour of the installer going forward. My goal here was to minimize side changes like this other than the big one we're changing on purpose.

[Gankra]

Ah i see what you mean. I at least simplified it a bit.

[Gankra]

wait i thought \n didn't work in shell

[mistydemeo]

Oh god right this is probably completely dependent on which echo is being used. I'm sure this isn't portable.

[Gankra]

...we should really look into if we're supposed to be wrapping these in quotes like we do in shell...

[mistydemeo]

I was wondering the same thing. It's a bit inconsistent about that atm