-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Add Debian 12 (bookworm) images #1385
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.
Thanks this looks pretty good, just some minor things.
base/base.bzl
Outdated
@@ -119,7 +119,7 @@ def distro_components(distro): | |||
base = ":static_" + user + "_" + arch + "_" + distro, | |||
tars = [ | |||
deb_pkg(arch, distro, "libc6"), | |||
deb_pkg(arch, distro, "libssl1.1"), | |||
deb_pkg(arch, distro, "libssl1.1" if distro == "debian11" else "libssl3"), | |||
deb_pkg(arch, distro, "openssl"), |
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 prefer we remove openssl from debian12 images. It can be available in debug
cc/BUILD
Outdated
@@ -10,6 +10,9 @@ DISTRO_DEBS = { | |||
"debian11": [ |
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.
if there are the same, just put them in the tars section. I feel like this might be remnants from debian10 days, and doesn't appear to be necessary anymore.
Thanks, addressed all your feedback, could you please have another look? |
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.
Looks like one test is failing, but there's also a minor documentation issue. otherwise lgtm.
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.
Awesome, thanks for contributing!
Would love to see the java targets added as well. |
I think the language targets will be added slowly as we roll this out gradually. |
@loosebazooka thanks for the quick review!
@Sineaggi working on it, will see if I can get it out this week. |
An attempt to fix #1337 and might supersede #1365
Requires #1384 to build on a fresh setup.
This builds
base
,static
,base-nossl
andcc
images. Happy to work on the other targets after feedback from maintainers.