Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add smart version detection for changing versions #359

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

marquiswang
Copy link
Contributor

This commit changes the behavior of jenv add and jenv global|local|shell [version]

Previously, jenv add would add "short" aliases of the version. For
example, adding Zulu JDK 11.0.3 would add the following aliases:

  • 11
  • 11.0
  • 11.0.3
  • zulu64-11.0.3

This was very convenient in that it prevented you from needing to type
jenv version zulu64-11.0.3 to change versions, but led to some
confusion among users. See issues #345, #326, #300, #268, etc.

It also made jenv versions very verbose if you had a lot of versions.

This commit addresses this by adding "smart" version detection when
running jenv global|local|shell [version], allowing you to run:

$ jenv global 11      # Change to JDK 11
$ jenv global 11.0.2  # Change to JDK 11.0.2
$ jenv local zulu     # Change to Zulu JDK
$ jenv shell x86_64   # Change to x86_64 architecture

With all of these commands, jenv will attempt to avoid changing the
other properties. So if the current global version is a Temurin JDK 11,
and you run jenv global 18, it will change to a Temurin JDK 18
(assuming you have one installed). If instead you run jenv global zulu
it will change to a Zulu JDK 11.

With this change, jenv add no longer adds the short version aliases.

The old aliases still work, but users may want to remove those aliases
if they are no longer useful. The aliases take priority over the smart
behavior.

Marquis Wong added 2 commits May 8, 2022 22:49
Right now, jenv doesn't differentiate between JDKs for different
architectures, except for 32 bit vs 64 bit. For the vast majority of
systems, this is not a problem, since only one architecture is
supported. However, on the new M1 Macs, two 64 bit architectures are
supported: x86_64 and aarch64.

A user might want to be able to switch between two architectures of
JDKs. Current Jenv refuses to add the second JDK because it detects the
same version.

We can read the architecture by reading the `os.arch` system property.

This commit changes the jenv alias format from

  [provider][32|64]-[version]

to

  [provider]-[version]-[architecture]

For example, before, the full alias would be

  oracle64-17.0.2

and how it is

  oracle-17.0.2-x86_64

For M1 builds, it now shows

  orable-17.0.2-aarch64

This is a change to the alias format, so if you run `jenv add` with a
JDK that was already added to jenv, it will get re-added with the new
format.
This commit changes the behavior of `jenv add` and `jenv
global|local|shell [version]`

Previously, jenv add would add "short" aliases of the version. For
example, adding Zulu JDK 11.0.3 would add the following aliases:
* 11
* 11.0
* 11.0.3
* zulu64-11.0.3

This was very convenient in that it prevented you from needing to type
`jenv version zulu64-11.0.3` to change versions, but led to some
confusion among users. See issues jenv#345, jenv#326, jenv#300, jenv#268, etc.

It also made `jenv versions` very verbose if you had a lot of versions.

This commit addresses this by adding "smart" version detection when
running `jenv global|local|shell [version]`, allowing you to run:

$ jenv global 11      # Change to JDK 11
$ jenv global 11.0.2  # Change to JDK 11.0.2
$ jenv local zulu     # Change to Zulu JDK
$ jenv shell x86_64   # Change to x86_64 architecture

With all of these commands, `jenv` will attempt to avoid changing the
other properties. So if the current global version is a Temurin JDK 11,
and you run `jenv global 18`, it will change to a Temurin JDK 18
(assuming you have one installed). If instead you run `jenv global zulu`
it will change to a Zulu JDK 11.

With this change, `jenv add` no longer adds the short version aliases.

The old aliases still work, but users may want to remove those aliases
if they are no longer useful. The aliases take priority over the smart
behavior.
@marquiswang
Copy link
Contributor Author

@gcuisinier, @brunoborges @obfischer: What do you guys think about this idea? I think it's a fairly elegant solution to the various complaints about handling different versions with different providers/architectures.

@gcuisinier
Copy link
Collaborator

That seems interesting.
But I've two question :

  • Is it backward compatible (for people already using jenv and their "version" setup that are upgrading ? )
  • Do you have check the performance impact on shell loading / change directory ? As their is already some complain about the time impact.

@marquiswang
Copy link
Contributor Author

marquiswang commented May 11, 2022

That seems interesting. But I've two question :

  • Is it backward compatible (for people already using jenv and their "version" setup that are upgrading ? )

It is backwards compatible, with a few caveats:

  • Old aliases don't get removed, and they take precedence: If you have 11, 11.0, 11.0.3 aliases from before, and you add zulu-11.0.4-x86_64, then do jenv shell 11, it'll switch to whatever was aliased to 11 before, ignoring this code. You need to remove 11 in order to be able to use it.
  • The full alias is changed, so if you add a JDK again without removing the old format, you'll have duplicate entries.

So someone who wants to migrate fully to this new functionality will want to remove all aliases and re-add them. We could add a command to do that, but I haven't done so.

Additionally, if you do that then any local aliases you had set are going to break and need to be remade. There is unfortunately no way to detect those (I guess we could do a system-wide find, but I doubt anyone would want that).

  • Do you have check the performance impact on shell loading / change directory ? As their is already some complain about the time impact.

This does not affect the runtime of the hook, so shell loading / changing directory should be fine. There is a bit of a performance hit when doing jenv add and jenv global|local|shell, but those should be much more acceptable. It's honestly not that bad either.

Comment on lines +1 to +3
#!/usr/bin/env bash
# Summary: Add JDK into jenv. A alias name will be generated by parsing "java -version"
# Usage: jenv add /path/to/java_home

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably wrong doc-string


find_jdk2 $1 && exit

echo $1

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a formality, but files should have one empty line at the end.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants