-
-
Notifications
You must be signed in to change notification settings - Fork 102
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
docker: add action to deploy windows build image #3780
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
.
c90c9b2
to
da551f9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add in a doc change to the table in https://github.com/adoptium/infrastructure/blob/master/FAQ.md#what-about-the-builds-that-use-the-dockerbuild-tag too please as part of this PR?
done |
24063e6
to
3fdbedd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving in principle for when we've got this tested (We probably don't want to be running of my infra branch so we should perhaps merge those changes - or equivalents - into this PR once verified as reliable.
Just as a point of clarification - the use of this won't be going near production until after the release cycle so if this is a bit rough round the edges (quite likely) it will still be nice to have the image creation/publish process in place.
2cf12b3
to
2f7597c
Compare
f747c94
to
670f70d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to block this for now as the changes to remove openj9 support (added after I gave my approval) should not be in here. If we want to skip the inclusion of those installs in the adoptium build containers then let's skip the roles in the dockerfile for now but since our build scripts do support non-temurin builds and other projects may be using it this should be a separate issue for discussion and not just done inside this PR.
updated PTAL |
/merge - this is unrelated to the release and it would be good to get the docker image published in Azure so we can begin phase 2 |
Approval to merge during the lockdown cycle Please can two Adoptium PMC members comment |
/approve |
force: no | ||
checksum: 22bae6c3ddf1a464b285784599eef8698f64dde24378c77e42522a536b88cbbc | ||
checksum_algorithm: sha256 | ||
win_shell: c:\cygwin64\bin\curl -L -o /cygdrive/c/temp/ant-contrib.zip https://sourceforge.net/projects/ant-contrib/files/ant-contrib/ant-contrib-1.0b2/ant-contrib-1.0b2-bin.zip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to add in something here to verify the download on this download in the absence of the checksum check that was in win_get_url
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't consistently get the same shasum on that download URL (I have no idea why and not sure if we should be worried about this)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would be concerned.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed @karianna ... We had a SHA check with the previous win_get_url
operation so if something is preventing that when using curl then that would definitely need to be understood before this can go in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Since I'm going to be away for most of the next week, if these issues are resolved then feel free to dismiss my review)
/approve |
/thaw |
Pull Request unblocked - code freeze is over.
ping @gdams is there anything blocking this? |
step 1 of #3217
Checklist