Tweak PropertiesLauncher to locate classes as well as nested jars #8486
Conversation
dsyer
changed the title
|
That last commit adds flags that user can set to launch an app with classes in the root of the archive. Some agents may break (anecdotally at least), but the app seems to work (I added a test), and AspectJ weaving also works (tested manually). |
| @@ -489,7 +499,7 @@ private Archive getArchive(File file) throws IOException { | |||
| } | |||
|
|
|||
| private List<Archive> getNestedArchives(String root) throws Exception { | |||
| if (root.startsWith("/") | |||
| if (!root.equals("/") && root.startsWith("/") | |||
| || this.parent.getUrl().equals(this.home.toURI().toURL())) { | |||
| // If home dir is same as parent archive, no need to add it twice. | |||
| return null; | |||
There was a problem hiding this comment.
It's hard to write unit tests for this stuff, so I had already added new integration tests instead. However, I rebased and pushed an extra commit (2ae6a14) with some more unit tests, plus bug fixes for corner cases I found when writing those, and 2 new integration tests. None of the unit tests can get at this code path unfortunately because there is no way to change the "parent" archive for the test. I think the integration tests would fail without this line though.
There was a problem hiding this comment.
Now those changes are in d9d84b6
There was a problem hiding this comment.
Thanks, Dave. We agreed on Slack not to merge 0e8f055 so I've been ignoring those integration tests. The new ones are better, but I don't like them being part of spring-boot-gradle-tests because they're not really anything to do with Gradle. Also, that module doesn't exist any more in 2.0 so they'll be a pain to merge in their current form. Is it not possible to integration tests this in the spring-boot-loader module by creating a jar or two with the required layout in code?
| if (("/".equals(root) || ".".equals(root)) && parent != this.parent) { | ||
| // You can't find the root with an entry filter so it has to be added | ||
| // explicitly. But don't add the root of the parent archive. | ||
| archives.add(parent); |
There was a problem hiding this comment.
Same as above (it's the same method I think).
| i.e. conventional (defaults to `true`) | ||
|
|
||
| |`loader.parent.boot` | ||
| |Boolean flag to indicate that the parent class laoder should be the "boot" loader, not the |
There was a problem hiding this comment.
Good catch, thanks. Fixed in 0e8f05.
dsyer
force-pushed
the
relocate-classes
branch
2 times, most recently
from
d9d84b6 to
80e0752
Compare
Before this change application classes only work in BOOT-INF/classes. With this change you can use a subdirectory, or the root directory of an external jar (but not the parent archive to avoid issues with agents and awkward delegation models). Fixes spring-projectsgh-8480
|
Another rebase (onto 1.5.x), removed the integration tests, added more unit tests, and some slightly enhanced logic for jar paths (e.g. testing that relative paths work, which we always supported but hadn't tested): 6687e24. I also removed the class loader hierarchy options. Still have that code on a branch if anyone is interested. |
This includes better support for properties launcher, which we will use in zipkin-aws and zipkin-azure spring-projects/spring-boot#8486
This includes better support for properties launcher, which we will use in zipkin-aws and zipkin-azure spring-projects/spring-boot#8486
This includes better support for properties launcher, which we will use in zipkin-aws and zipkin-azure spring-projects/spring-boot#8486
Special thanks to @dsyer for spring-projects/spring-boot#8486
Special thanks to @dsyer for spring-projects/spring-boot#8486
Special thanks to @dsyer for spring-projects/spring-boot#8486
Special thanks to @dsyer for spring-projects/spring-boot#8486
Before this change application classes only work in BOOT-INF/classes.
With this change you can use a subdirectory, or the root directory
of an external jar (but not the parent archive to avoid issues
with agents and awkward delegation models).
Fixes gh-8480