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

Switching default to include layers index for fat jar #20983

Closed
mbhave opened this issue Apr 15, 2020 · 7 comments
Closed

Switching default to include layers index for fat jar #20983

mbhave opened this issue Apr 15, 2020 · 7 comments
Assignees
Labels
type: enhancement A general enhancement
Milestone

Comments

@mbhave
Copy link
Contributor

mbhave commented Apr 15, 2020

No description provided.

@mbhave mbhave added status: waiting-for-triage An issue we've not yet triaged for: team-attention An issue we'd like other members of the team to review labels Apr 15, 2020
@philwebb philwebb added type: enhancement A general enhancement and removed for: team-attention An issue we'd like other members of the team to review status: waiting-for-triage An issue we've not yet triaged labels Apr 23, 2020
@philwebb philwebb added this to the 2.4.x milestone Apr 23, 2020
@philwebb
Copy link
Member

We'd need a Gradle API to switch off support. Since this is quite late for 2.3, we'll reassess in 2.4

@mbhave mbhave self-assigned this Jun 29, 2020
@bclozel
Copy link
Member

bclozel commented Jul 24, 2020

We also need to decide whether we should include the layertools classes to the fat jar, or just the idx file.

@mbhave mbhave changed the title Consider switching default to include layers index for fat jar Switching default to include layers index for fat jar Aug 3, 2020
@mbhave mbhave closed this as completed in 41f5ba9 Aug 4, 2020
@wilkinsona wilkinsona modified the milestones: 2.4.x, 2.4.0-M2 Aug 4, 2020
@wilkinsona
Copy link
Member

Looking at #22195 caused me to notice that there are a few places in the Gradle plugin, javadoc for example, where it reads as if layers are disabled by default. Re-opening so that we can address those.

@wilkinsona wilkinsona reopened this Aug 6, 2020
@bclozel bclozel added the for: team-attention An issue we'd like other members of the team to review label Aug 7, 2020
@bclozel
Copy link
Member

bclozel commented Aug 7, 2020

It seems that both layers.idx and layertools are included in the fat jar by default now. You can only exclude the layertools from the jar explicitly.

I know that layertools isn't that big, but in this case should this be included with the launcher? I'm a bit confused by this situation.

@mbhave
Copy link
Contributor Author

mbhave commented Aug 7, 2020

I thought it made sense to include layertools by default if layers.idx was present by default as well. I'm not sure what you mean by

but in this case should this be included with the launcher?

but we can discuss this on the next call.

@snicoll
Copy link
Member

snicoll commented Aug 7, 2020

I'm not sure what you mean by

The use case we want to promote is CNB and our build image build integration. This does not require layertools so that seems a bit strange to include it by default.

This would break users creating docker image and relying on that when upgrading to 2.3 but they'd have to change the enabled flag to includeLayerTools. That doesn't sound too bad to me.

@philwebb philwebb removed the for: team-attention An issue we'd like other members of the team to review label Aug 10, 2020
@snicoll
Copy link
Member

snicoll commented Aug 11, 2020

We've discussed this and we concluded that offering a good out-of-the-box experience for both CNB users (via our build image task in the Maven and Gradle plugins) as well as those who prefer to craft their Dockerfile themselves is important. Advanced users can opt-out for layers, the layertools or both via configuration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

5 participants