Skip to content

Make jdk better at finding versions#235

Open
tewaro wants to merge 1 commit into
mainfrom
tewaro/update-jdk-memcached
Open

Make jdk better at finding versions#235
tewaro wants to merge 1 commit into
mainfrom
tewaro/update-jdk-memcached

Conversation

@tewaro
Copy link
Copy Markdown
Contributor

@tewaro tewaro commented Jun 5, 2026

jdk-25 has gone up a minor version number, just fix prepare_ycsb to adopt whatever the minor version is by doing with little bit of shell scripting.

jdk-25 has gone up a minor version number, just fix prepare_ycsb to adopt whatever the
minor version is by doing with little bit of shell scripting.
@tewaro tewaro requested a review from a team as a code owner June 5, 2026 05:46
Copy link
Copy Markdown
Contributor

@arthurp arthurp left a comment

Choose a reason for hiding this comment

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

Hopefully this will change/go away once we are in the new benchmark last, but this is reasonable.

Also, I'm amused and you of all people are making the build less reproducible. :-D You can have your Nix later, I guess.

realpath "$latest_jdk"
}

export JDK_PATH=$(resolve_jdk_path || true)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why is this set here, and then again in prepare_ycsb?

fi

local latest_jdk
latest_jdk=$(printf '%s\n' "${jdk_dirs[@]}" | sort -V | tail -n 1)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This seems really complicated for what it does. Why not latest_jdk=$(ls -1 "${CACHE_DIR}"/jdk-* | sort -V | tail -n 1) (with nullglob as you astutely included, I didn't know that existed) and then just

if [ -z "$latest_jdk" ]; then
    return 1
fi

for the error check.

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.

2 participants