-
Notifications
You must be signed in to change notification settings - Fork 3k
Use production dependency model for remote-dev #48331
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
Conversation
@mkouba you might be interested in this failure
|
@aloubyansky I've never seen this before and this particular test didn't fail yet. Also I wasn't able to reproduce it locally (using your branch). Could you pls apply the following patch so that we could identify the problematic piece of code on CI? diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Injection.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Injection.java
index 25337632031..b539608784e 100644
--- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Injection.java
+++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Injection.java
@@ -457,6 +457,9 @@ static Injection forInvokerArgumentLookups(ClassInfo targetBeanClass, MethodInfo
public Injection(AnnotationTarget target, List<InjectionPointInfo> injectionPoints) {
this.target = target;
this.injectionPoints = injectionPoints;
+ if (injectionPoints.stream().anyMatch(ip -> ip == null)) {
+ throw new IllegalArgumentException("Null injection point detected for " + target);
+ }
}
boolean isMethod() { |
@mkouba would you like this change to be merged into the main branch? |
I'd like to find the problem first but yes, it might be useful. |
I suspect it might just be a rare glitch |
c4e1257
to
9abbd09
Compare
Ok, let's keep the assert then... |
This comment has been minimized.
This comment has been minimized.
9abbd09
to
2fc6d8f
Compare
This comment has been minimized.
This comment has been minimized.
2fc6d8f
to
98e326b
Compare
This comment has been minimized.
This comment has been minimized.
@aloubyansky from what I can see, the failures are in the added test. |
Yes, they are. And I don't think they are exactly caused by the changes but I'm still looking into it. |
The reason the new test is failing on Windows is due to a a failure to delete a JAR due to it being locked (something else has it "open"). |
98e326b
to
21cb2fe
Compare
This comment has been minimized.
This comment has been minimized.
The CI looks good. This is ready for review. It fixes more issues than originally reported in the issue referenced in the description. |
Status for workflow
|
Fixes #48198
It fixes a few issues:
mutable-jar
is a production packaging);classes
) merging content of all the local dependencies in a single JAR and linking to it from multipleResolvedDependency
s;appmodel.dat
update to trigger a restart instead of thedeployment-class-path.dat
, since theappmodel.dat
includes information about the reloadable dependencies that would need to be updated on the remote side by the client requiring a restart.