Only include bootstrap dependencies in quarkus:dev ClassPath #12156
Conversation
|
We had that for a reason though. I don't remember exactly the issues it solved but wondering whether this will lead to regressions. Not yet sure if that's related but this test DevMojoIT.testThatAddingConfigFileWorksCorrectly:530 failed on linux and windows. |
|
That probably is related, I will need to look into why (I tested dev mode locally and it worked, but there must be a problem somewhere). |
It was this #7521. Looking at the name of that failing test, the failure indeed is related to the test change that was introduced in the link PR to catch any regression in this area. |
|
Ok, so I think I need to figure out the parent first artifacts and include them. |
|
I have changed this to still add parent first artifacts. Lets see what CI has to say. |
| @@ -93,6 +95,7 @@ | |||
| */ | |||
| @Mojo(name = "dev", defaultPhase = LifecyclePhase.PREPARE_PACKAGE, requiresDependencyResolution = ResolutionScope.COMPILE_PLUS_RUNTIME) | |||
| public class DevMojo extends AbstractMojo { | |||
| private static final String EXT_PROPERTIES_PATH = "META-INF/quarkus-extension.properties"; | |||
There was a problem hiding this comment.
That's io.quarkus.bootstrap.BootstrapConstants.DESCRIPTOR_PATH, btw.
| @@ -70,7 +70,7 @@ public static AppArtifactKey fromString(String str) { | |||
| protected final String classifier; | |||
| protected final String type; | |||
|
|
|||
| protected AppArtifactKey(String[] parts) { | |||
| public AppArtifactKey(String[] parts) { | |||
There was a problem hiding this comment.
It doesn't look like this change is necessary. There is already public AppArtifactKey(String groupId, String artifactId, String classifier, String type)
There was a problem hiding this comment.
It makes the code a lot simpler. This is the ctor that is used by the existing code that reads from quarkus-extensions.properties, I would need to copy the logic.
|
Looks good, the JDK 11 CI has failed, I've restarted the CI, just in case. |
| if (parentFirst != null) { | ||
| String[] artifacts = parentFirst.split(","); | ||
| for (String artifact : artifacts) { | ||
| parentFirstArtifacts.add(new AppArtifactKey(artifact.split(":"))); |
There was a problem hiding this comment.
There is AppArtifactKey.fromString(str) actually.
|
It looks like it fails on |
|
in dev mode |
The rest of the depenencies are resolved in the bootstrap resolver, having them on the ClassPath only causes problems. A similar fix could be applied to Gradle, however I am not sure how to filter the dependencies to only include those required for bootstrap. Fixes quarkusio#12136
The rest of the depenencies are resolved in the bootstrap resolver,
having them on the ClassPath only causes problems.
A similar fix could be applied to Gradle, however I am not sure
how to filter the dependencies to only include those required
for bootstrap.
Fixes #12136