Add selfupdate_enable config and respect auto_answer when auto updating

[felipecrs]

This also deprecates the auto_selfupdate config.

The automatic version check makes the things a lot harder to script in non-attended environments -- such as CI, as it keeps waiting for a response if an updated version is found.

Also, I use my own tool to manage my updates system widely, which I run automatically.

Fixes #925
Depends-On: sdkman/sdkman-hooks#28
Depends-On: sdkman/sdkman-website-playframework#49

[marc0der]

Hi, @felipecrs. To clarify, do you mean to prevent the self-update check from happening in a CI environment, or do I misunderstand your intent? If this is indeed the problem you are facing, we already have a config variable called sdkman_auto_selfupdate to cover this.

[felipecrs]

Hi, @marc0der. The sdkman_auto_selfupdate=false configuration makes the update not to install automatically. But still, sdk checks for updates on almost every command, and prompts for the user to update if a newer version is found.

The new configuration sdkman_version_check=false will prevent sdk from checking for updates automatically and thus, also prevent the update prompt from appearing as well.

Different things, right?

To be very specific with you, in scripts I run for example sdk version, to know what is the SDKMAN version installed. Without this new configuration, if a newer version of sdk exists, the sdk version command would hang, waiting for input on wheter the user wants to install the new update or not.

[felipecrs]

This issue can be replicated:

❯ docker run -it --rm mcr.microsoft.com/vscode/devcontainers/java:0.201.5-15 bash -c 'echo "sdkman_auto_selfupdate=false" > $SDKMAN_DIR/etc/config && touch -d 20120101 $SDKMAN_DIR/var/delay_upgrade && exec bash -ic "sdk version"'
We periodically need to update the local cache. Please run:

  $ sdk update

==== BROADCAST =================================================================
* 2021-06-18: mvnd 0.5.2 available on SDKMAN! https://git.io/Jn0Kl
* 2021-06-17: micronaut 3.0.0-M2 available on SDKMAN!
* 2021-06-15: micronaut 2.5.6 available on SDKMAN!
================================================================================

SDKMAN 5.11.2+698


ATTENTION: A new version of SDKMAN is available...

The current version is 5.11.5+713, but you have 5.11.2+698.

Would you like to upgrade now? (Y/n):

[marc0der]

Hey @felipecrs, I understand the problem now. Could we make the naming of this config a little clearer to reflect the what, not the how? How about calling it sdkman_selfupdate_disable to make it very clear what the behaviour is? This will also require changes to be made in the sdkman-hooks service. I would also like to have a test to cover this if at all possible.

[felipecrs]

Sure! But isn't the existence of both sdkman_auto_selfupdate and sdkman_selfupdate_disable a bit confusing? Just a thought: what if we extend sdkman_auto_selfupdate to include what I'm introducing?

[marc0der]

The name sdkman_auto_selfupdate implies that you are making the self-update automatic (or not). This is distinctly different from not doing a self-update at all. I prefer this as a separate configuration, and we should document both clearly on the website. On second thought, perhaps it should be called sdkman_selfupdate_enable to avoid confusing double negatives.

Incidentally, the associated changes in the sdkman-hooks service need to be made in the install and selfupdate scripts.

[felipecrs]

This is distinctly different from not doing a self-update at all

But the new configuration is not removing this ability, it's available there with sdk selfupdate command. Actually, it's disabling the "automatic selfupdate", and that's why I think it would be less confusing to just extend the capabilities of sdkman_auto_selfupdate=false instead of creating a new config.

Technically speaking I know exactly the difference between them as I'm the one who proposed, but practically speaking, I'm just wondering if there would be a real use case for both configs. Either the person wants to disable both or no.

[marc0der]

This really is quite different, and most certainly a feature used by many people. Think about developers who are using this on their machines (as opposed to in a CI environment) who don't want to be asked if the next update should/shouldn't be installed. For this very scenario, we have auto-update.

[felipecrs]

What about this existing config:

# make sdkman non-interactive, preferred for CI environments
sdkman_auto_answer=true|false

Should not it be respected? If it should, there would not be a point for sdkman_auto_selfupdate to exist.

[felipecrs]

So I believe the optimal solution would be to:

1. Extend sdkman_auto_answer=true to include the current capabilities of sdkman_auto_selfupdate=true
2. Change the sdkman_auto_selfupdate config to do what the new config being introduced in this PR would do:
   • false would prevent automatic checking for newer versions and thus the prompt
   • true would keep automatic checking for newer versions, and the automatic answer to proceed with the upgrade would be decided by sdkman_auto_answer.

[marc0der]

I totally agree with what you are proposing, but the name of sdkman_auto_selfupdate is completely wrong and will confuse many people. What you are describing should be called sdkman_selfupdate_enable. So we should deprecate the old sdkman_auto_selfupdate, favouring the new config and behaviour you proposed.

BTW, the "auto" in sdkman_auto_selfupdate refers to the fact that it is an unattended update.

[felipecrs]

@marc0der good! I will work on it.

[felipecrs]

BTW, the "auto" in sdkman_auto_selfupdate refers to the fact that it is an unattended update.

Well, it could be what it was intended to mean, but still auto tends to mean more automatic than unattended if someone reads it without the knowledge you have.

In context, you can turn on/off the sdkman automatic self-update (sdkman_auto_selfupdate) with true/false.

On the other hand: sdkman_selfupdate_enable has nothing to reference the "automatic" part of the process (and it needs to be referenced, as the normal selfupdate (sdk selfupdate command) will remain functional even though sdkman_selfupdate_enable=false.

Anyway, I'm just discussing to make sure we introduce a good config name to prevent future refactors/breaking changes.

[marc0der]

I'm happy with sdkman_selfupdate_enable and will merge your PR once this is in place. However, remember that we need those two sdkman-hooks scripts updated before we can merge this one, and we will need appropriate changes made on the usage page of the website.

[marc0der]

@felipecrs any update on this one?

[felipecrs]

@marc0der I believe all the changes are ready to review.

[marc0der]

Looking great, just some minor comments on this PR.

Also, we require the changes to be made in the sdkman-hooks service.

[felipecrs]

@marc0der I made the changes. Most of them was due to my lack of attention, while the others I replied you in their own conversation.

[felipecrs]

@marc0der all done.