Skip to content

Conversation

tw29845
Copy link

@tw29845 tw29845 commented Aug 18, 2025

@tw29845 tw29845 changed the title Update Java, Gradle, & Maven in enterprise-java image Refactor enterprise-java image to use SDKMAN for versioning Aug 18, 2025
@matifali matifali requested a review from Copilot August 20, 2025 06:59
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the enterprise-java Docker image to use SDKMAN for managing Java development tool versions instead of manual downloads and installations. This approach simplifies version management and resolves previous Maven download issues.

  • Replaces manual JDK, Gradle, and Maven installation with SDKMAN-based management
  • Updates to latest LTS versions: OpenJDK 21.0.8, Gradle 8.14.3, and Maven 3.9.11
  • Eliminates hardcoded download URLs and hash verification by delegating to SDKMAN

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@tw29845 tw29845 requested a review from matifali August 20, 2025 12:50
@tw29845
Copy link
Author

tw29845 commented Aug 20, 2025

Unsure why the pipeline building the Docker images is failing, but I don't believe it is anything to do with my changes

@matifali matifali requested review from johnstcn and removed request for matifali August 20, 2025 15:47
@tw29845 tw29845 requested a review from johnstcn August 21, 2025 09:50
@tw29845 tw29845 force-pushed the version-bump-maven-gradle branch from 6dc51d6 to da1d939 Compare August 21, 2025 10:05
@tw29845
Copy link
Author

tw29845 commented Aug 21, 2025

@johnstcn Are you able to tell why the pipeline building the images fails? If it's auth issues due to me merging in a fork then I'm not sure what I can do to resolve that myself

@johnstcn
Copy link
Member

johnstcn commented Aug 21, 2025

If it's auth issues due to me merging in a fork then I'm not sure what I can do to resolve that myself

Yeah this is on us to figure out, sorry about that

For now I cherry-picked your changes into a different PR #308

@johnstcn
Copy link
Member

@tw29845 I get this when building on linux/arm/v7:

22 3.478 'curl --fail --location --progress-bar "${SDKMAN_SERVICE}/broker/download/native/install/${SDKMAN_NATIVE_VERSION}/${SDKMAN_PLATFORM}" > "$sdkman_zip_file"': command failed with exit code 22.

https://github.com/coder/images/actions/runs/17132138297/job/48598919038?pr=308

@tw29845
Copy link
Author

tw29845 commented Aug 21, 2025

@tw29845 I get this when building on linux/arm/v7:

22 3.478 'curl --fail --location --progress-bar "${SDKMAN_SERVICE}/broker/download/native/install/${SDKMAN_NATIVE_VERSION}/${SDKMAN_PLATFORM}" > "$sdkman_zip_file"': command failed with exit code 22.

https://github.com/coder/images/actions/runs/17132138297/job/48598919038?pr=308

Are you able to tell me what the output of uname -m is? That command, along with uname -s, determines the platform and architecture within SDKMAN. There is an ARM v7 arch mentioned in the SDKMAN install script called armv7l so if that matches then it should be good.

@johnstcn
Copy link
Member

Are you able to tell me what the output of uname -m is? That command, along with uname -s, determines the platform and architecture within SDKMAN. There is an ARM v7 arch mentioned in the SDKMAN install script called armv7l so if that matches then it should be good.

if memory serves, armv7l is correct. There's an existing script in deprecated/rust/rustup.sh that has some similar logic:

get_architecture() {
    local _ostype _cputype _bitness _arch _clibtype
    _ostype="$(uname -s)"
    _cputype="$(uname -m)"
    _clibtype="gnu"

...

case "$_cputype" in 
    ...
        armv7l | armv8l)
            _cputype=armv7
            if [ "$_ostype" = "linux-android" ]; then
                _ostype=linux-androideabi
            else
                _ostype="${_ostype}eabihf"
            fi
            ;;
esac

@tw29845
Copy link
Author

tw29845 commented Aug 22, 2025

So far as I can find, SDKMAN does not support 32-bit ARM architecture. There is a ticket asking about just that, but no replies (see here). I have hacked together a workaround for the installation of SDKMAN itself that uses the 64-bit ARM release, but even after that I get errors when verifying the installation of Maven.
As much as I think using SDKMAN for management of the SDKs would be useful to many, I unfortunately am not able to spend much more time blindly debugging the ARM images. I would welcome @johnstcn or anybody else to take a look at getting this working though. Otherwise, the PR may have to be closed.

@johnstcn
Copy link
Member

johnstcn commented Aug 22, 2025

So far as I can find, SDKMAN does not support 32-bit ARM architecture.

@matifali AFAICT the only reason I'm aware we support the armv7 architecture is for the older models of Raspberry Pi. I'd rather not drop support for this platform as it's a popular community use-case for Coder.

@tw29845 another possibility is that we add this as a new image enterprise-java-sdkman that excludes the armv7 arch.

Thoughts?

@tw29845
Copy link
Author

tw29845 commented Aug 26, 2025

AFAICT the only reason I'm aware we support the armv7 architecture is for the older models of Raspberry Pi. I'd rather not drop support for this platform as it's a popular community use-case for Coder.

That's absolutely fine, happy to see continued support for older platforms!

another possibility is that we add this as a new image enterprise-java-sdkman that excludes the armv7 arch.

I'd be happy with that, though I'm unsure of how one specific image could be configured to not being one specific architecture fitting with your current build processes. The easiest solution would be to add a new depot command to build_images.sh specific for the new enterprise-java-sdkman image

@tw29845 tw29845 force-pushed the version-bump-maven-gradle branch from 7aefce2 to 1adc1bb Compare August 29, 2025 10:04
…4 and arm64 for java-sdkman. Refactored java image to resolve breakage when new Maven is released and to improve layer caching.
@tw29845 tw29845 force-pushed the version-bump-maven-gradle branch from 1adc1bb to 490d548 Compare August 29, 2025 10:44
@tw29845
Copy link
Author

tw29845 commented Aug 29, 2025

@johnstcn ready for review

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.

3 participants