Fix #34 - Windows based docker images for maven (jdk8 and jdk11 supported) #119
Conversation
|
awesome! waiting for some feedback |
|
Let the know if there is anything I need to address |
|
Is there some specific feedback you are looking for that I might be able to speed up? :-D |
|
Should give this a more descriptive title. |
|
Also see INFRA-1400. |
slide
changed the title
| @@ -0,0 +1,46 @@ | |||
| # escape=` | |||
There was a problem hiding this comment.
For lack of highlighting, see github-linguist/linguist#4566 (comment).
Co-Authored-By: Jesse Glick <[email protected]>
Co-Authored-By: Jesse Glick <[email protected]>
Add powershell to syntax highlighting
| @@ -0,0 +1,46 @@ | |||
| # escape=` | |||
| ARG WINDOWS_DOCKER_TAG=ltsc2019 | |||
| Expand-Archive -Path ${env:TEMP}/apache-maven.zip -Destination C:/ProgramData ; ` | ||
| Move-Item C:/ProgramData/apache-maven-${env:MAVEN_VERSION} C:/ProgramData/Maven ; ` | ||
| New-Item -ItemType Directory -Path C:/ProgramData/Maven/Reference | Out-Null ; ` | ||
| Remove-Item ${env:TEMP}/apache-maven.zip |
There was a problem hiding this comment.
Yow, a lot of duplicated content between images. Is there a straightforward way to share some of these steps?
There was a problem hiding this comment.
Yes, I agree. I am not a super user with docker, so I am not aware of any tricks for sharing content. I based my implementation on the existing image setup.
| # So the initial ~/.m2 is set with expected content. | ||
| # Don't override, as this is just a reference setup | ||
|
|
||
| function Copy-ReferenceFiles() { |
There was a problem hiding this comment.
Same here, we should not have a dozen copies of the same script.
Perhaps it would be better to refactor the repository layout to use a flat structure, so that any Dockerfile.* could freely COPY any resource file (perhaps just a temporary script to RUN and delete).
There was a problem hiding this comment.
Yes, the mvn-entrypoint.sh is copied across the various image directories as well, this is because Docker doesn't support COPY from outside the current directory (and children) into the image. I'm sure there could be some refactoring to take that into account, but I didn't want to make that part of this changset.
There was a problem hiding this comment.
Cleaning up an existing situation is indeed something that should be deferred to a separate PR. I mentioned this here because this PR introduces a lot of duplication.
Perhaps we could start by introducing a single directory with all the Windows images, and leave the Linux images untouched for now.
There was a problem hiding this comment.
I'm definitely open to that method if that is desired. I'd also like to know if that would work for @carlossg to have that method.
Co-Authored-By: Jesse Glick <[email protected]>
|
I have put a refactor of the structure into a branch so that folks can look at it. If it looks ok, I will open a new PR with those changes to reduce the commit history for the PR. https://github.com/slide/docker-maven/tree/structure-refactor This moves all the windows builds into a windows directory with Dockerfile.windows- |
|
It used to be that the official build could not build a file other than https://github.com/docker-library/official-images/blob/master/library/adoptopenjdk#L19 The |
|
Is this repo using builds on docker hub for releases? This will be something to overcome since docker hub builds don't support Windows. |
|
Yes this is the official dockerhub image built by docker inc |
|
I'm going to close this out for now until I can determine a way to build and publish the Windows images since Dockerhub builds does not support Windows. |
Add Windows images for everything but slim and IBM JDK