Apply review changes from the gtk3 build to the gtk4 builder #2803
Conversation
|
Thanks for this, I really appreciate it. Could you also remove the installation of the default -jdk and the set of the SWT environment variable. That shouldn't be necessary as well. |
As the new swtgtk3nativebuild was copied from this Dockerfile and there is the hope to keep them as similar as possible I have applied the changes as discussed in eclipse-platform#2802 - Normalize end of file newlines - Remove JDK
|
With commit 390e234 the diffs are now really minimal between the two: $ diff -ru swtnativebuild/ swtgtk3nativebuild/
diff -ru swtnativebuild/Dockerfile swtgtk3nativebuild/Dockerfile
--- swtnativebuild/Dockerfile 2025-02-03 18:40:54.697327107 -0500
+++ swtgtk3nativebuild/Dockerfile 2025-02-03 18:41:50.882433027 -0500
@@ -1,4 +1,4 @@
-FROM debian:12
+FROM debian:10
### user name recognition at runtime w/ an arbitrary uid - for OpenShift deployments
COPY scripts/uid_entrypoint /usr/local/bin/uid_entrypoint
@@ -14,7 +14,6 @@
procps \
git \
libgtk-3-dev \
- libgtk-4-dev \
freeglut3-dev \
webkit2gtk-driverCommit message/summary updated too. |
jonahgraham
changed the title
| @@ -16,8 +16,7 @@ RUN apt-get update -qq && apt-get install -qq -y \ | |||
| libgtk-3-dev \ | |||
| libgtk-4-dev \ | |||
| freeglut3-dev \ | |||
| webkit2gtk-driver \ | |||
| default-jdk | |||
There was a problem hiding this comment.
I would keep the default jdk as we need it for compilation (the headers at least) and installing it afterwards safe some space in the image, but then we need to download (and install) this saved thing afterwards. Also it would hinder others to use the docker image outside the jenkinsbuild,
There was a problem hiding this comment.
As explained in #2802 (comment), that's not necessary because that's what is already happening.
Also it would hinder others to use the docker image outside the jenkinsbuild,
I don't think that's a relevant use-case for that docker-image. We have only considered the requirements of our use-case in releng/swt/equinox for these images in the past.
| @@ -32,6 +31,5 @@ RUN chgrp -R 0 ${HOME} && chmod -R g=u ${HOME} | |||
|
|
|||
| RUN localedef -i en_US -f UTF-8 en_US.UTF-8 | |||
| ENV LANG=en_US.UTF-8 | |||
| ENV SWT_JAVA_HOME=/usr/lib/jvm/default-java/ | |||
There was a problem hiding this comment.
the build scripts require this setting, and this is the "swtnativebuild" image so I think it is valid to have this
There was a problem hiding this comment.
As above (#2803 (comment)), that's already set by the corresponding Jenkinsfiles if necessary.
|
@laeubi and @HannesWell have differing opinions on the topic of including JDK or not. My experience with CDT (which uses a single docker image and cross compiles for windows/mac and other linux archs from it) is quite different than Platform that has many many different machines to manage. @HannesWell explains it better in #2802 (comment) Therefore I think this change is now complete and ready for review + merge. |
As the new swtgtk3nativebuild was copied from this Dockerfile and there is the hope to keep them as similar as possible I have applied the changes as discussed in #2802