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

Java Temurin wrong version sort order #928

Closed
kien-truong opened this issue Oct 4, 2023 · 4 comments
Closed

Java Temurin wrong version sort order #928

kien-truong opened this issue Oct 4, 2023 · 4 comments

Comments

@kien-truong
Copy link

Describe the bug
Java Temurin-17 is having a newer version that appears before an older version.

temurin-17.0.8+101
temurin-17.0.8+7

Expected behavior
The versions should be in correct order

rtx doctor output

rtx version:
  2023.9.2 linux-x64 (155dadb 2023-09-25)

build:
  Target: x86_64-unknown-linux-gnu
  Features: DEFAULT, NATIVE_TLS, OPENSSL, SELF_UPDATE
  Built: Mon, 25 Sep 2023 17:09:06 +0000
  Rust Version: rustc 1.72.0 (5680fa18f 2023-08-23)
  Profile: release

shell:
  /usr/bin/zsh
  zsh 5.8.1 (x86_64-ubuntu-linux-gnu)

rtx data directory:
  /home/kien/.local/apps/rtx

rtx environment variables:
  RTX_DATA_DIR=/home/kien/.local/apps/rtx
  RTX_SHELL=zsh

settings:
  {"always_keep_download": "false", "always_keep_install": "false", "asdf_compat": "false", "disable_default_shorthands": "false", "disable_tools": "[]", "experimental": "false", "jobs": "4", "legacy_version_file": "true", "legacy_version_file_disable_tools": "[]", "log_level": "INFO", "missing_runtime_behavior": "warn", "plugin_autoupdate_last_check_duration": "10080", "raw": "false", "trusted_config_paths": "[]", "verbose": "false", "yes": "false"}

config files:
  /home/kien/.config/rtx/config.toml

plugins:
  awscli      https://github.com/MetricMike/asdf-awscli.git#2e31396
  bat         https://gitlab.com/wt0f/asdf-bat.git#d5a4f38
  chezmoi     https://github.com/joke/asdf-chezmoi.git#9643c7d
  delta       https://github.com/andweeb/asdf-delta.git#501318b
  fzf         https://github.com/kompiro/asdf-fzf.git#d19eb67
  go          (core)
  helm        https://github.com/Antiarchitect/asdf-helm.git#a39e17b
  java        (core)
  kubectl     https://github.com/asdf-community/asdf-kubectl.git#cbe6df4
  kustomize   https://github.com/Banno/asdf-kustomize.git#2c43c30
  mvnd        https://github.com/joschi/asdf-mvnd.git#03940a1
  node        (core)
  python      (core)
  ruby        (core)
  vault       https://github.com/asdf-community/asdf-hashicorp.git#0346f5d
  zoxide      https://github.com/nyrst/asdf-zoxide#8ed95c9

toolset:
  chezmoi@latest, fzf@latest, kubectl@latest, helm@latest, kustomize@latest, zoxide@latest, bat@latest, delta@latest, java@temurin-17 java@temurin-11, [email protected], awscli@latest, vault@latest, node@lts

No problems found

@kien-truong kien-truong added the bug label Oct 4, 2023
roele added a commit to roele/mise that referenced this issue Oct 7, 2023
@jdx jdx linked a pull request Oct 17, 2023 that will close this issue
@jdx
Copy link
Owner

jdx commented Oct 18, 2023

we sort these using the rust semver crate which isn't handling this version correctly.

looking at the output of rtx ls-remote java I really don't know if we'll ever be able to properly sort everything in there unfortunately. A lot of java runtimes just behave really strangely.

@kien-truong
Copy link
Author

What I find odds is sometimes it sorts correctly, for example

temurin-11.0.14+9
temurin-11.0.14+101

Also, I think temurin-17.0.8+101 is actually version 17.0.8.1+1, which somehow got converted into 17.0.8+101

@jdx
Copy link
Owner

jdx commented Dec 2, 2023

I think what needs to happen is 2 things:

  • plugin trait needs to be modified to include some sort of function to compare versions using semver as the default but allowing java.rs to override with custom logic.
  • somehow we need to figure out how to actually sort all of the versions and put that logic into this override function. This might need to be different for each flavor of java.

As far as what the override function could be, I'm not really sure how best to use the PartialOrd trait here but maybe we could just start with this:

fn partial_cmp(a: &str, b: &str) -> Option<std::cmp::Ordering>

If someone wanted to write that function even just for temurin, it could just be a simple playground script that I can work to integrate in the codebase.

@jdx jdx added the help wanted label Dec 3, 2023
@jdx
Copy link
Owner

jdx commented Dec 12, 2023

I think I managed to resolve this actually without needing to do what I mentioned before. Now rtx uses https://rtx-versions.jdx.dev/java for the versions which I pipe through sort -V and I think it sorts it correctly:

$ rtx ls-remote java | grep temurin-11
temurin-11.0.12+7
temurin-11.0.13+8
temurin-11.0.14+9
temurin-11.0.14+101
temurin-11.0.15+10
temurin-11.0.16+8
temurin-11.0.16+101
temurin-11.0.17+8
temurin-11.0.18+10
temurin-11.0.19+7
temurin-11.0.20+8
temurin-11.0.20+101
temurin-11.0.21+9

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

Successfully merging a pull request may close this issue.

2 participants