adoptium / containers

Repo containing the dockerfiles and scripts to produce the official eclipse-temurin containers.
https://hub.docker.com/_/eclipse-temurin/
Apache License 2.0
216 stars 93 forks source link

Fix Gradle v7.5+ not working with UBI based Docker images #463

Closed ThoSap closed 11 months ago

ThoSap commented 11 months ago

Fixes #462 Gradle v7.5+ not working with UBI based Docker images.

ThoSap commented 11 months ago

For this change I just added the package here docker_templates/ubi9-minimal.Dockerfile.j2 and then generated the images using generate_dockerfiles.py

I manually added the package for these Dockerfile's, as the .j2 template does not influence these ones:

gdams commented 11 months ago

@ThoSap is there a reason you don't suggest that the gradle maintainers don't add this dependency to their base image? It kind of feels the wrong place to be adding package specific dependencies to our docker images.

I note that the gradle doesn't ship an UBI image (only Ubuntu and Alpine one): https://github.com/keeganwitt/docker-gradle/blob/master/jdk21/Dockerfile

If the requirement is to have a UBI image with Gradle preinstalled I'd suggest that we make the ask upstream?

gdams commented 11 months ago

CC @keeganwitt (the official Gradle image maintainer)

ThoSap commented 11 months ago

My change just installs the dependency findutils, which is required by Gradle to be able to use the command xargs and is usually pre-installed by each of the Linux distros in the wild.

The Ubuntu and Alpine Linux based Eclipse Temurin images ship with findutils, therefore Gradle 7.5+ works. Therefore I wanted to add this dependency, so each of the Temurin flavors have the same baseline. https://www.gnu.org/software/findutils/ https://man7.org/linux/man-pages/man1/xargs.1.html

gdams commented 11 months ago

My change just installs the dependency findutils, which is required by Gradle to be able to use the command xargs and is usually pre-installed by each of the Linux distros in the wild.

The Ubuntu and Alpine Linux based Eclipse Temurin images ship with findutils, therefore Gradle 7.5+ works. Therefore I wanted to add this dependency, so each of the Temurin flavors have the same baseline. https://www.gnu.org/software/findutils/ https://man7.org/linux/man-pages/man1/xargs.1.html

Yeah I totally understand what you're saying, my concern is explicitly adding a package to our images. Especially as they are meant to be minimal images. I'd like to see what @keeganwitt suggests first, I'd much prefer the Gradle project ship an official image for UBI (which would then contain any additional dependencies)

keeganwitt commented 11 months ago

I'm not currently providing UBI-based official Gradle images, though it's something I'd consider if I can understand the use-case a little better.

The Gradle images were meant for using a globally-installed Gradle rather than the wrapper, which it sounds like is what @ThoSap is currently using. Still, in general I'd agree that something that's a Gradle-specific dependency would likely make sense to avoid adding to this base image. If you're wanting to stick with the wrapper, maybe it'd make sense to invoke the wrapper using the Gradle image (kind redundant, but at least you'd have the dependencies you need there).

karianna commented 11 months ago

Going to close this based on the advice given (and our general policy to be as minimal as possible).

keeganwitt commented 10 months ago

@ThoSap Assuming it's of interest to you, please open an issue on https://github.com/keeganwitt/docker-gradle/ and explain the benefit of UBI-based images for us to consider a new image variant.