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

Reverse order of classpath to allow patching #777

Closed
tobad357 opened this issue Aug 2, 2018 · 9 comments
Closed

Reverse order of classpath to allow patching #777

tobad357 opened this issue Aug 2, 2018 · 9 comments
Assignees
Milestone

Comments

@tobad357
Copy link

tobad357 commented Aug 2, 2018

Description of the issue:
Overriding dependency classes is impossible due to classpath order when generating
startup command.

Sometimes it is necessary to patch a class in the dependencies.
To get this to work the classpath would need to load application classes first and then
lib classpath

Current Entrypoint
ENTRYPOINT ["java","-Xms512m","-Xdebug","--spring.config.name","${APP_NAME}","--spring.config.location","${CONFIG_FILES}","-cp","/app/libs/*:/app/resources/:/app/classes/","com.app.AppStart"]

Expected behavior:
The app classes dir would be first in the classpath
ENTRYPOINT ["java","-Xms512m","-Xdebug","--spring.config.name","${APP_NAME}","--spring.config.location","${CONFIG_FILES}","-cp","/app/resources/:/app/classes/:/app/libs/*","com.app.AppStart"]

Steps to reproduce:

  1. Build any project with gradle jibExportDockerContext
  2. View the generated Dockerfile

Environment: gradle

@coollog
Copy link
Contributor

coollog commented Aug 2, 2018

Hi @tobad357 , thanks for the suggestion! This definitely sounds like a useful feature and we will have this patched for the next version release (0.9.9).

@coollog
Copy link
Contributor

coollog commented Aug 22, 2018

Hi @tobad357 , we have reversed the classpath order in version 0.9.9 - let us know if this works for you.

@anuraaga
Copy link
Contributor

Thanks for fixing this issue - I can confirm the change fixed a similar problem I was having (I guess there are many jars out there specifying a file tls.crt...).

However, I notice that now the order is resources -> classes -> libs. While there's no official standard, from what I understand, it's more conventional to do classes -> resources -> libs. I've confirmed that this is the order the Gradle application plugin's run task uses, so seems like it might be better to match that behavior?

@coollog
Copy link
Contributor

coollog commented Aug 27, 2018

Hi @anuraaga thanks for the tip! We were thinking to keep resources before classes in case one wished to replace some class file in classes with a file in resources, whereas the reverse does not seem to be of any use (replacing resource file with file in classes). However, mimicking the behavior of the run task may be the right way to go @GoogleContainerTools/java-tools

@coollog coollog reopened this Aug 27, 2018
@briandealwis
Copy link
Member

I think your reasoning makes sense @coollog.

@TadCordle TadCordle modified the milestones: v0.9.9, v0.9.10 Aug 27, 2018
@TadCordle
Copy link
Contributor

Agreed, I think we gain more here the way we currently have it vs following convention.

@coollog
Copy link
Contributor

coollog commented Aug 27, 2018

Closing unless any objections to keeping it the way it is.

@coollog coollog closed this as completed Aug 27, 2018
@anuraaga
Copy link
Contributor

anuraaga commented Aug 28, 2018

Feel free to keep this closed :) Just want to ask for one more thought though. Currently, this means that users could see different behavior when using gradle's run (which is commonly used from normal IntelliJ running with the Gradle runner) and docker run of a jib image. It's also different than gradle's distribution plugin which runs a jar - the jar packages classes before resources so the JRE will prefer classes. Basically, any Gradle user is highly likely to currently rely on behavior that prioritizes classes over resources regardless of their running / deployment strategy.

I think it can be very surprising to users to see different behavior for run and docker run and might reduce trust in the plugin. I'd recommend giving one more look from a user's perspective - if this Gradle plugin decides not to match Gradle's behavior, I would at least think that should be mentioned in the README.

@briandealwis
Copy link
Member

Although the Maven exec:java doesn't add resources to the classpath by default, when configured to do so (addResourcesToClasspath=true) then it adds resources, then classes, then dependencies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants