Skip to content
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

feat: add jre8 slices for Jammy #31

Merged
merged 7 commits into from
Jun 15, 2023

Conversation

cjdcordeiro
Copy link
Collaborator

These new slice definitions files constitute the minimal set of packages and files necessary to build a functional headless java 8 runtime filesystem.

@cjdcordeiro cjdcordeiro force-pushed the jre8-jammy-slices branch 2 times, most recently from 0d1a495 to f51165b Compare February 8, 2023 08:59
@cjdcordeiro cjdcordeiro requested review from vpa1977 and woky February 8, 2023 09:52
@cjdcordeiro cjdcordeiro requested a review from woky February 8, 2023 10:07
@cjdcordeiro cjdcordeiro force-pushed the jre8-jammy-slices branch 3 times, most recently from d81090e to f5e35b6 Compare February 20, 2023 08:04
@vpa1977
Copy link

vpa1977 commented Feb 20, 2023

Maybe we need tools slice and misc properties slice.

When adding websphere liberty/tomcat support I've stumbled on the fact that sometimes launcher scripts want to run keytool or access logging.properties that we've decided to map in.
In those cases we are happy with defaults, so we can add a separate slice for that/

@cjdcordeiro
Copy link
Collaborator Author

Maybe we need tools slice and misc properties slice.

When adding websphere liberty/tomcat support I've stumbled on the fact that sometimes launcher scripts want to run keytool or access logging.properties that we've decided to map in. In those cases we are happy with defaults, so we can add a separate slice for that/

So, that would be something like:

tools:
  /usr/lib/jvm/java-8-openjdk-amd64/jre/bin/keytool:
  /usr/lib/jvm/java-8-openjdk-amd64/bin/keytool:

Is that right? What would they depend on? Libs?

As for properties, there's already a slice, but it only contains a handful of properties. Should I add them all?

@vpa1977
Copy link

vpa1977 commented Feb 21, 2023

tools:
  /usr/lib/jvm/java-8-openjdk-amd64/jre/bin/keytool:
  /usr/lib/jvm/java-8-openjdk-amd64/bin/keytool:

Is that right? What would they depend on? Libs?
yes, needs libjli and jvm to work.

As for properties, there's already a slice, but it only contains a handful of properties. Should I add them all?
Probably, they do not take much space and its possible to replace/override them.

@cjdcordeiro
Copy link
Collaborator Author

alright, thanks @vpa1977 . Done

@vpa1977
Copy link

vpa1977 commented Feb 27, 2023

I was doing comparison with corretto, and I've found that they do not add glib libraries.

Looking into openjdk source code, the only downside is that those cause java to always think that it connects directly to internet.
Maybe those should be in a separate optional slice.

@cjdcordeiro
Copy link
Collaborator Author

I was doing comparison with corretto, and I've found that they do not add glib libraries.

Looking into openjdk source code, the only downside is that those cause java to always think that it connects directly to internet. Maybe those should be in a separate optional slice.

👍 this should be addressed in rockcrafters/chiselled-jre#50

@cjdcordeiro cjdcordeiro requested a review from niemeyer April 13, 2023 05:59
@cjdcordeiro cjdcordeiro added the Priority Look at me first label Apr 17, 2023
Copy link
Collaborator Author

@cjdcordeiro cjdcordeiro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bringing this PR back to draft mode. @vpa1977 I'll need your help to make some design changes on these slices, driven by the following feedback from @niemeyer (summarized):

creating slices with basis on the file format, although logical for some pkgs, doesn't really help much from a functional standpoint (eg. it doesn't make sense to always install all the Java properties together, as they are probably meant for different functional units). Maybe a better way to slice this would be to split slices based on their functional purpose, i.e. jars and properties (and other paths) can easily belong to the same slice, if they are part of the same functional unit.

So basically, the questions we need to ask ourselves are:

  • do these jars function by themselves? or do they expect certain properties files to coexist in the same filesystem?
  • are slices being overdesigned to include all paths of the same type, even though some of those are not generally needed?
  • in practice, for this specific Java example: "where are .properties (and others) used? they might be a functional dependency of some code in some other slice...so if that code is in a jar and that jar can't function without that .property, then let's put them together in the same slice. An even more functional approach would be to group .properties by jar, ie. each .jar has its own .properties, in a dedicated sliced"

@cjdcordeiro cjdcordeiro marked this pull request as draft May 12, 2023 10:08
@vpa1977
Copy link

vpa1977 commented May 14, 2023

Bringing this PR back to draft mode. @vpa1977 I'll need your help to make some design changes on these slices, driven by the following feedback from @niemeyer (summarized):

I think main point that we need to address here is separate security dependencies (nss3 ) and awt dependencies (fontconfig). Those make sense as separate slices and provide ~4/5 mb savings of the image size. Rest of the items does not provide such benefit, but we might do them in the same manner just for consistency.

Copy link

@woky woky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will leave the rest for tomorrow. The shared library paths should be changed though.

slices/fontconfig-config.yaml Show resolved Hide resolved
slices/libblkid1.yaml Outdated Show resolved Hide resolved
@cjdcordeiro cjdcordeiro requested review from woky and vpa1977 May 19, 2023 17:13
@cjdcordeiro cjdcordeiro marked this pull request as ready for review May 19, 2023 17:13
slices/openjdk-8-jre-headless.yaml Show resolved Hide resolved
slices/openjdk-8-jre-headless.yaml Outdated Show resolved Hide resolved
slices/fontconfig-config.yaml Show resolved Hide resolved
slices/fonts-dejavu-core.yaml Outdated Show resolved Hide resolved
The previous use of globs was dangerous as a
future introduction of fonts-dejavu-extra would
automatically introduce an unnecessary conflict.
@cjdcordeiro cjdcordeiro requested a review from woky May 22, 2023 13:16
@cjdcordeiro cjdcordeiro requested a review from vpa1977 May 23, 2023 06:34
Copy link

@niemeyer niemeyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lots of slices, but looks quite clean. Thanks for the reorg on the JRE slices.

@cjdcordeiro
Copy link
Collaborator Author

Thanks for the approvals ;)

I'll merge it

@cjdcordeiro cjdcordeiro self-assigned this Jun 15, 2023
@cjdcordeiro cjdcordeiro merged commit b78d1b0 into canonical:ubuntu-22.04 Jun 15, 2023
@cjdcordeiro cjdcordeiro deleted the jre8-jammy-slices branch June 15, 2023 13:26
gregory-schiano pushed a commit to gregory-schiano/chisel-releases that referenced this pull request Jun 22, 2024
* feat: add jre8 slices for Jammy

* feat: slice openjdk-8-jre-headless by functional unit

* fix: add comments and split profiledebug in JRE8

* feat: replace 'gnu' with glob for multi-arch

* fix: stcp -> sctp (typo)

* fix: remove globs for fonts-dejavu-core

The previous use of globs was dangerous as a
future introduction of fonts-dejavu-extra would
automatically introduce an unnecessary conflict.

* feat: add libnpt for arm64 in openjdk-8-jre-headless

---------

Co-authored-by: Vladimir Petko <[email protected]>
vpa1977 added a commit to vpa1977/chisel-releases that referenced this pull request Aug 6, 2024
* feat: add jre8 slices for Jammy

* feat: slice openjdk-8-jre-headless by functional unit

* fix: add comments and split profiledebug in JRE8

* feat: replace 'gnu' with glob for multi-arch

* fix: stcp -> sctp (typo)

* fix: remove globs for fonts-dejavu-core

The previous use of globs was dangerous as a
future introduction of fonts-dejavu-extra would
automatically introduce an unnecessary conflict.

* feat: add libnpt for arm64 in openjdk-8-jre-headless

---------

Co-authored-by: Vladimir Petko <[email protected]>
cjdcordeiro added a commit to ozanmakes/chisel-releases that referenced this pull request Sep 27, 2024
* feat: add jre8 slices for Jammy

* feat: slice openjdk-8-jre-headless by functional unit

* fix: add comments and split profiledebug in JRE8

* feat: replace 'gnu' with glob for multi-arch

* fix: stcp -> sctp (typo)

* fix: remove globs for fonts-dejavu-core

The previous use of globs was dangerous as a
future introduction of fonts-dejavu-extra would
automatically introduce an unnecessary conflict.

* feat: add libnpt for arm64 in openjdk-8-jre-headless

---------

Co-authored-by: Vladimir Petko <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority Look at me first
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants