Feature: Dispatch separate requests for multiple versions

• [ ✔️ ] I have read the CONTRIBUTING guidelines

Feature request

Currently with sdk install gradle 5.6.4 4.10.3 SDKMAN only installs the first entry, 5.6.4, and ignores the second entry (only accepts one arg). sdk could support multiple args to be passed to the install command, where each arg is a different version which is dispatched in a separate sequential request. Thus making it easier for end users to install multiple versions of the same tool.

[rgoldberg]

This requires changes to sdk-main.sh, which I have already implemented in a commit in my commits-pending-merge queue.

These changes were to clean up convoluted code, to support any number of arguments to sdk commands, and to support varargs. I also created argument validation functions that would output both informative errors and color-coded command usage messages. I have only implemented varargs in one command, but I can easily add varargs support to additional commands once the main changes have been merged.

@rgoldberg Awesome! When you open the PR can you link it to this issue? I was mainly looking at sdk-install.sh's __sdk_install() function. The vararg is actually the second argument out of three in that function, version.

I have a little imperfect test boilerplate that could be used as a starting point for getting $version in there while still preserving $folder:

#!/usr/bin/env bash

### shift_args.sh

# ./shift_args.sh 1 2 3 4 blah_/bleh-/bluh.
# 1 : 2 : blah_/bleh-/bluh.
# 1 : 3 : blah_/bleh-/bluh.
# 1 : 4 : blah_/bleh-/bluh.

# ./shift_args.sh 1 2 (for single version)
# 1 : 2

candidate=
version=
folder=

folder_regex='[-_.\w]*'
args_count="${@: -1}"

if [[ "$#" > 2 ]]; then
    candidate="$1"
    while [ "$2" != "" ]; do
        version="$2"
        if [[ $args_count =~ $folder_regex ]]; then
            folder="$args_count"
        fi
        echo "$candidate : $version : $folder"
        shift
        if [ "$#" == 2 ]; then  
            break
        fi
    done
else
    candidate="$1"
    version="$2"
    echo "$candidate : $version"
fi

After looking at the logic for handling $folder again, this example needs work. It assumes that every time $version is set then $folder is set too unless there are only two args (i.e. sdk install gradle 4.10.3). $folder_regex may prove to be irrelevant later.

[rgoldberg]

I forgot about the local path argument, which is stored in the local variable folder.

@marc0der How do you want to handle local paths?

It would be simplest to just use a flagged option, like -p <local path> (-p for path; it could be -l for local, or something else).

I know that you were against flagged options, but, to support more complex usages, they're really handy. There are other places in which flagged options might be useful. I can't remember off the top of my head, but I remember thinking about them for other new features.

My not-yet-merged argument validation functions can validate flagged options (using single characters like -p, not words like --local-path). The argument validation refactor includes moving the calls to argument validation from _sdk to each command function itself, which is a better, more encapsulated way of handling things.

[rgoldberg]

varargs should also be allowed for the last argument of each of the following commands:

• current
• flush
• uninstall
• upgrade

[rgoldberg]

Also, supporting varargs requires updating argument validation for the vararg. Given that I have not-yet-merged argument validation functions that can handle varargs, we should probably switch to using them before supporting varargs in existing commands. Better to not waste time rewriting argument validation when a better validation system is already available.

[marc0der]

I don't think we should enable global varargs across all commands, let's start small and only do so on the install command for starters.

@D4n1k For now, let's rather go with your approach of bringing change in the sdkman-install.sh. I'd be very happy for you to pick this work up, as it was your idea and you originally volunteered to do the work.

@rgoldberg I also don't want to make huge changes to the way the CLI works wrt adding flags, changing behaviour of existing commands etc. as this will confuse and alienate our users. The CLI works (and has always worked) on imperative commands that read like sentences. Hence we have no flags in our CLI, which is a design choice from the start.

Let's get this feature working in the simplest way possible :-)

[rgoldberg]

You can’t just make the change in sdkman-install.sh. sdkman-main.sh must also be modified as per my next pending commit.

The commands other than install can support varargs later, but it won’t take long to add such support once general vararg code & arg validation code has been merged.

A vararg version for install, however, introduces ambiguity as to whether the third argument is a local path or a version (this ambiguity wouldn’t exist for varargs in any other command, so it might make sense to try varargs elsewhere first).

• Should it be considered a local path if it is currently a valid path, and a version otherwise?
• If there are more than 3 args, can the last arg ever be a local path, or can local paths only be used for exactly 3 args?
• If local path can possibly be used with 4+ args, is the penultimate arg the version for the local path specified in the last arg?

[rgoldberg]

@D4n1k
folder is the local path for a local installation, and should never be used for more than one version. It should only be used for at most one of the version arguments, but your code would apply it to every version.

Your regex will match any string since it uses *, so there's no point to it.

@rgoldberg Yes that is correct... I'll take a look at the code closer today.

@marc0der I will start working on this again today.

[rgoldberg]

@D4n1k It makes sense to determine how local paths will be handled before working on this. You can’t implement something if you don’t know what that something should be.

Yup, that is spot on. I'm trying to avoid restructuring the whole command just to support varargs. To me, that defeats the whole purpose, but maybe it does make sense to break apart local installations into a separate function altogether triggered by a flag. Perhaps something like:

sdk install -l java 1.8.0 .java/path

That just makes everything a lot cleaner and separates out the logic completely.

[rgoldberg]

Desired behavior must be decided upon for the following 1 issue with invalid versions, 1 issue with already installed versions, and 2 issues with setting the default version for a candidate when using a version vararg for sdk install.

Issue 1: Invalid Versions
If some, but not all, of the versions are invalid, should:

1. none of them be installed
2. all valid versions be installed with warnings for invalid versions
3. all valid versions before the first invalid version be installed with a warning for first encountered invalid version, and no attempts made to install versions after the first invalid one (I prefer the other options to this option)
4. something else

Issue 2: Already Installed Versions
If some, but not all, of the versions are already installed, should:

1. none of them be installed
2. all not-yet-installed versions be installed with warnings for already installed versions (my preference)
3. all not-yet-installed versions before the first already installed version be installed with a warning for first encountered already installed version, and no attempts made to install versions after the first already installed one
4. something else

Issue 3: Default Version User Prompts
Should the user prompt to change the default version:

1. be displayed as one user prompt that lets a user choose between all newly installed versions (thus no longer a Y/n prompt)
2. be displayed as one user prompt for each newly installed version, in argument order
3. not be displayed when more than one version is being installed
4. be displayed some other way

Issue 4: Default Version Auto Answer Settings
How will $sdkman_auto_answer & $auto_answer_upgrade be handled? i.e. If either are true, which version is selected to be the new default? I can imagine setting the default to:

1. the first version by argument order
2. the last version by argument order
3. the newest version (which would require some ordering for versions, so this is probably not feasible right now)
4. none, if more than one version is being installed
5. a version selected by some other algorithm

[marc0der]

@ghost this is a summary of what I suggested on Slack. I suggested we consider turning the local installation into a brand new command called local. That frees up install to take varargs of versions only. In other words we would have:

sdk local java 1.8.0 /path/to/java

and then to install multiple versions:

sdk install java a.b.c x.y.z

I also suggest that as always we start with tests. How about we update the existing install_candidate.feature and local_development_versions.feature to reflect our desired behaviour? Once we have come to agreement regarding the specs, we can think about the simplest way of implementing this.