Added feature to download boot jdk on demand #3725

judovana · 2024-03-23T11:11:12Z

currenlty only dne for linux, shoudl be easy to extend for windows, not sure what whith other platfroms.

Is done via magic parameter of download for -J: -J download

judovana · 2024-03-23T11:15:21Z

The windows label is wrong. I can not change t on my own. Please fix it to (linux) if possible.
I think I will add windows support once I get some cygwn machine, but it will take time.

github-actions

A block has been put on this Pull Request as this repository is temporarily under a code freeze due to an ongoing release cycle.

If this pull request needs to be merged during the release cycle then please comment /merge and a PMC member will be able to remove the block.

If the code freeze is over you can remove this block by commenting /thaw.

judovana · 2024-03-23T18:45:12Z

I guess on windows, I will extract

temurin-build/build-farm/platform-specific-configurations/windows.sh

Lines 47 to 99 in 95385f4

    
               elif [ "$JDK_BOOT_VERSION" -ge 8 ]; then # Adoptium has no build pre-8 
        
                 # This is needed to convert x86-32 to x32 which is what the API uses 
        
                 export downloadArch 
        
                 case "$ARCHITECTURE" in 
        
                    "x86-32") downloadArch="x32";; 
        
                   "aarch64") downloadArch="x64";; 
        
                           *) downloadArch="$ARCHITECTURE";; 
        
                 esac 
        
                 releaseType="ga" 
        
                 vendor="eclipse" 
        
                 api="adoptium" 
        
                 apiUrlTemplate="https://api.\${api}.net/v3/binary/latest/\${JDK_BOOT_VERSION}/\${releaseType}/windows/\${downloadArch}/jdk/hotspot/normal/\${vendor}" 
        
                 apiURL=$(eval echo ${apiUrlTemplate}) 
        
                 echo "Downloading GA release of boot JDK version ${JDK_BOOT_VERSION} from ${apiURL}" 
        
                 # make-adopt-build-farm.sh has 'set -e'. We need to disable that for 
        
                 # the fallback mechanism, as downloading of the GA binary might fail 
        
                 set +e 
        
                 wget -q "${apiURL}" -O openjdk.zip 
        
                 retVal=$? 
        
                 set -e 
        
                 if [ $retVal -ne 0 ]; then 
        
                   # We must be a JDK HEAD build for which no boot JDK exists other than 
        
                   # nightlies? 
        
                   echo "Downloading GA release of boot JDK version ${JDK_BOOT_VERSION} failed." 
        
                   # shellcheck disable=SC2034 
        
                   releaseType="ea" 
        
                   # shellcheck disable=SC2034 
        
                   vendor="adoptium" 
        
                   apiURL=$(eval echo ${apiUrlTemplate}) 
        
                   echo "Attempting to download EA release of boot JDK version ${JDK_BOOT_VERSION} from ${apiURL}" 
        
                   set +e 
        
                   wget -q "${apiURL}" -O openjdk.zip 
        
                   retVal=$? 
        
                   set -e 
        
                   if [ $retVal -ne 0 ]; then 
        
                     # If no binaries are available then try from adoptopenjdk 
        
                     echo "Downloading Temurin release of boot JDK version ${JDK_BOOT_VERSION} failed." 
        
                     # shellcheck disable=SC2034 
        
                     releaseType="ga" 
        
                     # shellcheck disable=SC2034 
        
                     vendor="adoptopenjdk" 
        
                     # shellcheck disable=SC2034 
        
                     api="adoptopenjdk" 
        
                     apiURL=$(eval echo ${apiUrlTemplate}) 
        
                     echo "Attempting to download GA release of boot JDK version ${JDK_BOOT_VERSION} from ${apiURL}" 
        
                     wget -q "${apiURL}" -O openjdk.zip 
        
                   fi 
        
                 fi 
        
                 unzip -q openjdk.zip 
        
                 mv "$(ls -d jdk-"${JDK_BOOT_VERSION}"*)" "$bootDir" 
        
               fi 
        
             fi 
        
           fi

to the downloader.sh as downloadWindowsBootJDK aybe with some adjustments... thoughts?

judovana · 2024-03-24T13:37:15Z

ps: I had noticed the bash-lint issues, mos tof them are valid. Will elaborate as the review progresses here.

judovana · 2024-03-26T16:48:38Z

Now works for windows and linuxes. I do not have more paltforms to try on. Please consider this.

sbin/prepareWorkspace.sh

makejdk-any-platform.1

build-farm/platform-specific-configurations/downloaders.sh

judovana · 2024-04-01T13:54:54Z

Thanx a lot for eyball!

karianna · 2024-04-02T07:40:48Z

@judovana Needs a rebase

judovana · 2024-04-02T14:01:19Z

@judovana Needs a rebase

yup. Thats exactly the reason why I do not want to change the extracted download code in this PR.

currenlty only dne for linux, shoudl be easy to extend for windows, not sure what whith other platfroms. Is done via magic parameter of `download` for -J: -J download

It seems that the directory was changed in recent times from jdk-8... to jdk8...

Co-authored-by: Martijn Verburg <martijnverburg@gmail.com>

karianna

I'm broadly OK with the implementation. Requesting other reviewers

karianna · 2024-04-03T02:48:50Z

/thaw

Pull Request unblocked - code freeze is over.

sxa · 2024-04-03T11:56:07Z

The windows label is wrong. I can not change t on my own. Please fix it to (linux) if possible. I think I will add windows support once I get some cygwn machine, but it will take time.

Tried but I don't trust the labeller not to add windows back in again ;-)

sxa · 2024-04-03T12:01:40Z

sbin/prepareWorkspace.sh

+    if  [ -e "$bootDir" ] ; then
+      echo "Reusing $bootDir"
+    else
+      source "$SCRIPT_DIR/../build-farm/platform-specific-configurations/downloaders.sh"


Architecturally I'm not a fan of this. The stuff in the root of this repository and sbin is supposed to be fully independent. The bits in build-farm is for the adoptium build processes and configures the environment for our setup and wraps around the other stuff. Calling out from sbin to platform-specific-configurations is therefore not ideal.

I like the idea of the option to specify a boot JDK to download, but I think this implementation will need some refinement first.

It should be quite strightforward to move downloaders.sh next to common.sh and refactor them so the $bootDir is not missused.

I will elaborate on the bootDir as I really do not like it. Do you have any prefferences on downloaders.sh location?

I'm undecided but marginally leaning towards sbin or sbin/common. WDYT @andrew-m-leonard ?

Architecturally I'm not a fan of this. The stuff in the root of this repository and sbin is supposed to be fully independent. The bits in build-farm is for the adoptium build processes and configures the environment for our

Actually there is an misunderstanding, caused by bootJdk variable name clash. I will rework it so it is no longer there.

The if [ -e "$bootDir" ] ; then is independent local check, if boot jdk was already downloaded, so it is not downloaded again.

Pleae advice on downloaders.sh location

done 00b599c

hmmh. I would probably leave the file in build-farm. Just to be sure, that it is clear, where the logic is borrowed from.

I'm undecided but marginally leaning towards sbin or sbin/common. WDYT @andrew-m-leonard ?

@sxa yes I agree with you, looking at this again I feel uncomfortable that "sbin" code is calling out to "build-farm", as this could lead code-creep in the future architecturally in the wrong direction.
@judovana We should move the downloaders.sh to sbin/common, which is the common place that both sbin and build-farm pull "common" things from please.

Sure, Thanx a lot, on it!

done. Tested on win x86, el8 aarch64 and f38 x64 aganst jdk8,17 and 21.

build-farm/platform-specific-configurations/downloaders.sh

judovana · 2024-04-22T16:37:09Z

ping please?

andrew-m-leonard

looks good

judovana · 2024-04-23T10:33:04Z

@andrew-m-leonard TY!

@sxa its upt o you now

Architecturally I'm not a fan of this. The stuff in the root of this repository and sbin is supposed to be fully independent. The bits in build-farm is for the adoptium build processes and configures the environment for our setup and wraps around the other stuff. Calling out from sbin to platform-specific-configurations is therefore not ideal.

I like the idea of the option to specify a boot JDK to download, but I think this implementation will need some refinement first.

I had resolved the major issue you had (the global variable) but the shared file remained in build-farm/platform-specific-configurations/downloaders.sh . I left it here, as all the code is originally from build-farm/platform-specific-configurations/ files. Maybe I would vote for leaving it there, with future work on other platforms, and move it only once it have been finished. But that can be very long time, thus the build-farm/platform-specific-configurations/downloaders.sh will be breaking the ' The stuff in the root of this repository and sbin is supposed to be fully independent." which I like, and do not want to be cause of its fade away.

judovana · 2024-05-06T16:10:40Z

/integrate

andrew-m-leonard · 2024-05-07T08:35:00Z

sbin/prepareWorkspace.sh

+    if  [ -e "$bootDir" ] ; then
+      echo "Reusing $bootDir"
+    else
+      source "$SCRIPT_DIR/../build-farm/platform-specific-configurations/downloaders.sh"


I'm undecided but marginally leaning towards sbin or sbin/common. WDYT @andrew-m-leonard ?

@sxa yes I agree with you, looking at this again I feel uncomfortable that "sbin" code is calling out to "build-farm", as this could lead code-creep in the future architecturally in the wrong direction.
@judovana We should move the downloaders.sh to sbin/common, which is the common place that both sbin and build-farm pull "common" things from please.

build-farm/platform-specific-configurations/downloaders.sh -> sbin/common/downloaders.sh

judovana · 2024-05-07T09:12:04Z

done. Tested on win x86, el8 aarch64 and f38 x64 aganst jdk8,17 and 21.

sxa · 2024-05-09T13:33:07Z

sbin/prepareWorkspace.sh

    rm -rf "${BUILD_CONFIG[WORKSPACE_DIR]}/${BUILD_CONFIG[WORKING_DIR]}/installedalsa" || true
    rm -rf "${BUILD_CONFIG[WORKSPACE_DIR]}/${BUILD_CONFIG[WORKING_DIR]}/installedfreetype" || true
+    rm -rf "${BUILD_CONFIG[WORKSPACE_DIR]}/downloaded-boot-jdk-${BUILD_CONFIG[OPENJDK_FEATURE_NUMBER]}" || true


I'm inclined to suggest leaving this in place as a cache for anyone using our scripts since the JDK is quite a big download, but I'm also ok with this.

I was going to comment on the fact that || true is unnecessary with rm -f but sine that's also in the previous three lines I'll ignore it ;-)

I agree, and I'm keeping it. It will be removed only if if [[ "${BUILD_CONFIG[CLEAN_LIBS]}" == "true" ]]; then
oook? Yah, I was wondering on the double passig rf || true, but left in the style!

sxa

LGTM and I've very happy to see all of this logic moved out of the platform-specific-configurations directory :-)

I've done a few tests on Linux but not Windows.

In the future we should look at doing the same for AIX, Solaris and macos so I suggest creating an issue for that since I'm not going to mandate that we do it now.

Also, one other question for anyone else reading this - should we default to download if we can't find a JDK in any other location?

judovana · 2024-05-09T14:10:17Z

/integrate

judovana · 2024-05-09T14:11:49Z

LGTM and I've very happy to see all of this logic moved out of the platform-specific-configurations directory :-)

I've done a few tests on Linux but not Windows.

In the future we should look at doing the same for AIX, Solaris and macos so I suggest creating an issue for that since I'm not going to mandate that we do it now.

Also, one other question for anyone else reading this - should we default to download if we can't find a JDK in any other location?

that would be NACK by me. It is not small dependence, and one should have boot jdk around. The -J download is highly comfortable option

issue created: #3804

andrew-m-leonard

looks good

github-actions bot added the windows Issues that affect or relate to the WINDOWS OS label Mar 23, 2024

github-actions bot previously requested changes Mar 23, 2024

View reviewed changes

karianna requested changes Mar 30, 2024

View reviewed changes

judovana force-pushed the downloadBootJdks branch from d4a7f6f to 040bc17 Compare April 2, 2024 14:44

judovana and others added 10 commits April 2, 2024 17:09

Added feature to download boot jdk on demand

a392a92

currenlty only dne for linux, shoudl be easy to extend for windows, not sure what whith other platfroms. Is done via magic parameter of `download` for -J: -J download

removed unused variabble

2747543

Refactor also for windows

6e6bbad

Changed mask for finding the jdk dir

a60a4f7

It seems that the directory was changed in recent times from jdk-8... to jdk8...

Added small readme to the downloader.sh

cb770b3

More quoting to make bash lint happy

92b5dcb

Update sbin/prepareWorkspace.sh

280a8eb

Co-authored-by: Martijn Verburg <martijnverburg@gmail.com>

Finished wording of comment about downloaders.sh

58fc2f9

Mentioned download on -J/--jdk-boot-dir man line

6c57490

Update build-farm/platform-specific-configurations/downloaders.sh

10bc5b5

Co-authored-by: Martijn Verburg <martijnverburg@gmail.com>

judovana force-pushed the downloadBootJdks branch from 9b2614d to 10bc5b5 Compare April 2, 2024 15:09

judovana requested a review from karianna April 2, 2024 17:38

karianna requested review from sxa and andrew-m-leonard April 3, 2024 02:47

karianna approved these changes Apr 3, 2024

View reviewed changes

relying on $bootDir being set - shellcheck disable=SC2154

661f1bc

sxa removed the windows Issues that affect or relate to the WINDOWS OS label Apr 3, 2024

sxa added p-linux Issues that affect or relate to the ppc LINUX OS z-linux Issues that affect or relate to the s390x LINUX OS labels Apr 3, 2024

github-actions bot added the windows Issues that affect or relate to the WINDOWS OS label Apr 3, 2024

sxa reviewed Apr 3, 2024

View reviewed changes

gdams reviewed Apr 3, 2024

View reviewed changes

build-farm/platform-specific-configurations/downloaders.sh Outdated Show resolved Hide resolved

judovana added 2 commits April 3, 2024 16:16

Fixed coyrigth header

763c02b

No longer using global bootDir

00b599c

judovana requested review from gdams and sxa April 3, 2024 14:30

Merge branch 'master' into downloadBootJdks

2963f89

andrew-m-leonard approved these changes Apr 23, 2024

View reviewed changes

Merge branch 'master' into downloadBootJdks

60e60f2

Merge branch 'master' into downloadBootJdks

78f6c94

andrew-m-leonard requested changes May 7, 2024

View reviewed changes

moved downloaders.sh to sbin/common/

47f9cf2

build-farm/platform-specific-configurations/downloaders.sh -> sbin/common/downloaders.sh

Merge branch 'master' into downloadBootJdks

9482b54

karianna requested a review from andrew-m-leonard May 7, 2024 20:32

sxa reviewed May 9, 2024

View reviewed changes

sxa approved these changes May 9, 2024

View reviewed changes

gdams approved these changes May 9, 2024

View reviewed changes

judovana mentioned this pull request May 9, 2024

enable -J download for all architectures #3804

Open

3 tasks

andrew-m-leonard approved these changes May 9, 2024

View reviewed changes

andrew-m-leonard merged commit 07d523c into adoptium:master May 9, 2024
28 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added feature to download boot jdk on demand #3725

Added feature to download boot jdk on demand #3725

judovana commented Mar 23, 2024

judovana commented Mar 23, 2024

github-actions bot left a comment

judovana commented Mar 23, 2024

judovana commented Mar 24, 2024

judovana commented Mar 26, 2024

judovana commented Apr 1, 2024

karianna commented Apr 2, 2024

judovana commented Apr 2, 2024 •

edited

Loading

karianna left a comment

karianna commented Apr 3, 2024

sxa commented Apr 3, 2024

sxa Apr 3, 2024

judovana Apr 3, 2024

sxa Apr 3, 2024

judovana Apr 3, 2024 •

edited

Loading

judovana Apr 3, 2024

judovana Apr 4, 2024

andrew-m-leonard May 7, 2024

judovana May 7, 2024

judovana May 7, 2024

judovana commented Apr 22, 2024

andrew-m-leonard left a comment

judovana commented Apr 23, 2024 •

edited

Loading

judovana commented May 6, 2024

andrew-m-leonard May 7, 2024

judovana commented May 7, 2024

sxa May 9, 2024

judovana May 9, 2024

sxa left a comment

judovana commented May 9, 2024

judovana commented May 9, 2024 •

edited

Loading

andrew-m-leonard left a comment

Added feature to download boot jdk on demand #3725

Added feature to download boot jdk on demand #3725

Conversation

judovana commented Mar 23, 2024

judovana commented Mar 23, 2024

github-actions bot left a comment

Choose a reason for hiding this comment

judovana commented Mar 23, 2024

judovana commented Mar 24, 2024

judovana commented Mar 26, 2024

judovana commented Apr 1, 2024

karianna commented Apr 2, 2024

judovana commented Apr 2, 2024 • edited Loading

karianna left a comment

Choose a reason for hiding this comment

karianna commented Apr 3, 2024

sxa commented Apr 3, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

judovana Apr 3, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

judovana commented Apr 22, 2024

andrew-m-leonard left a comment

Choose a reason for hiding this comment

judovana commented Apr 23, 2024 • edited Loading

judovana commented May 6, 2024

Choose a reason for hiding this comment

judovana commented May 7, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sxa left a comment

Choose a reason for hiding this comment

judovana commented May 9, 2024

judovana commented May 9, 2024 • edited Loading

andrew-m-leonard left a comment

Choose a reason for hiding this comment

judovana commented Apr 2, 2024 •

edited

Loading

judovana Apr 3, 2024 •

edited

Loading

judovana commented Apr 23, 2024 •

edited

Loading

judovana commented May 9, 2024 •

edited

Loading