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

PropertiesLauncher does not respect classpath.idx when adding jars in BOOT-INF/lib to the classpath #41719

Closed
oldium opened this issue Aug 8, 2024 · 9 comments
Assignees
Labels
type: bug A general bug
Milestone

Comments

@oldium
Copy link

oldium commented Aug 8, 2024

This is about spring-boot-loader module and org.springframework.boot.loader.launch package and is currently in the master Git branch.

The JarLauncher is written in a way that it can be used on the unpacked JARs (used in a layered Docker images) easily, because it reads the existing BOOT-INF/classpath.idx and thus respects the order of libraries. The PropertiesLauncher does not read the classpath.idx nor have an option to specify it, so the expected order of libraries is not respected. There is a possibility to read the index by script and put it into the loader.path system property, but that would be large to specify it via the command line, so this brings another complication (although solvable). Conclusion is that the PropertiesLauncher cannot be used easily on the unpacked JARs - on the layered images.

@oldium oldium changed the title PropertyLauncher cannot be used on layered images, it does not respect classpath.idx PropertiesLauncher cannot be used on layered images, it does not respect classpath.idx Aug 8, 2024
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Aug 8, 2024
@snicoll
Copy link
Member

snicoll commented Aug 8, 2024

Yes, this is the expected behavior. PropertiesLauncher is not an ExecutableArchiveLauncher by design as it is meant to be used with user-configured classpath.

Rather than an explanation of the status quo, it would be more interesting to understand why you care. In other words, why do you need to use PropertiesLauncher in this scenario?

@snicoll snicoll added the status: waiting-for-feedback We need additional information before we can continue label Aug 8, 2024
@oldium
Copy link
Author

oldium commented Aug 8, 2024

We have two entry points within the application, so we need a way to specify the Start-Class during startup. JarLauncher does not offer option to change the main/starting class (it takes it from the manifest), while PropertiesLauncher does have it (via loader.main property). But PropertiesLauncher cannot be used for our application, because it does not respect the classpath.idx and application fails to start because of that.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Aug 8, 2024
@wilkinsona
Copy link
Member

The documentation states that:

loader.path (if empty) defaults to BOOT-INF/lib (meaning a local directory or a nested one if running from an archive). Because of this, PropertiesLauncher behaves the same as JarLauncher when no additional configuration is provided.

PropertiesLauncher doesn't behave the same as JarLauncher because it ignores the classpath.idx file. We should either correct the documentation or change the implementation to match the documentation. Assuming the latter's not too difficult, I think that would be preferable.

@wilkinsona wilkinsona added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged status: feedback-provided Feedback has been provided labels Aug 13, 2024
@wilkinsona wilkinsona added this to the 3.2.x milestone Aug 13, 2024
@wilkinsona wilkinsona changed the title PropertiesLauncher cannot be used on layered images, it does not respect classpath.idx PropertiesLauncher does not respect classpath.idx when adding jars in BOOT-INF/lib to the classpath Aug 13, 2024
@oldium
Copy link
Author

oldium commented Aug 13, 2024

I know that PropertiesLauncher ignores classpath.idx, this is the main point of this issue :-). The PropertiesLauncher is therefore unusable on unpacked/layered JARs (otherwise a very good feature of Spring Boot) and I think it would be great to make it working on them.

This is not about PropertiesLauncher actually, we need a way specify a different Start-Class on an unpacked/layered JAR and with Spring Boot launchers it is not possible.

I would be happy with any solution for the launching problem, it could be a parameter for JarLauncher or any other launcher.

@wilkinsona
Copy link
Member

I know that you know it ignores the index. My comment was explaining why the documentation's incorrect.

we need a way specify a different Start-Class on an unpacked/layered JAR and with Spring Boot launchers it is not possible

You can use PropertiesLauncher and loader.main for that purpose.

I would be happy with any solution for the launching problem, it could be a parameter for JarLauncher or any other launcher.

PropertiesLauncher is the one to use if you want this kind of control. We have some regrets about its complexity and we don't want to start adding similar complexity to the other launchers.

@oldium
Copy link
Author

oldium commented Aug 13, 2024

I know that you know it ignores the index. My comment was explaining why the documentation's incorrect.

I know that you know the I know :-D Lol. Yep, clear.

we need a way specify a different Start-Class on an unpacked/layered JAR and with Spring Boot launchers it is not possible

You can use PropertiesLauncher and loader.main for that purpose.

I have tried that, but different order of libs causes bean initialization failures. The main app does not start because of that.

I would be happy with any solution for the launching problem, it could be a parameter for JarLauncher or any other launcher.

PropertiesLauncher is the one to use if you want this kind of control. We have some regrets about its complexity and we don't want to start adding similar complexity to the other launchers.

Yes, but I would have to transform 498 library entries from classpath.idx into loader.path to make it start.

@wilkinsona
Copy link
Member

Yes, but I would have to transform 498 library entries from classpath.idx into loader.path to make it start.

That won't be necessary once we've (hopefully) fixed this bug.

@scottfrederick
Copy link
Contributor

@oldium This should be fixed now and available in snapshot releases. If you have the ability to test with a snapshot and give feedback, we'd appreciate it.

Note that we decided to only apply this fix to the new loader, not to the classic loader.

@oldium
Copy link
Author

oldium commented Aug 21, 2024

Perfect, thanks. This should simplify our startup script to use single loader for both application entry points.

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

No branches or pull requests

5 participants