Handle gradle included-builds in quarkusDev task #12046
Conversation
|
That's a tricky one. I am not sure this is the proper way, tbh. AFAIU, included builds should be replacing the matching app dependencies. It's not the same as resolving sets of dependencies separately and then merging them in. E.g. if the included build had a different dependency graph then the dependency it was replacing, I don't think you would get the proper classpath, you would get a mix of both. |
|
@aloubyansky, I wasn't aware of |
glefloch
force-pushed
the
fix/11902
branch
2 times, most recently
from
298070a to
c98fbaa
Compare
gsmet
added
the
triage/needs-rebase
|
This PR needs a rebase and probably another look then :). |
ghost
added
area/devtools
glefloch
removed
the
triage/needs-rebase
|
@aloubyansky I was able to work on this. Actually, resolved dependency are already substituted. |
| Project projectDep = project.getRootProject() | ||
| .findProject(((ProjectComponentIdentifier) a.getId().getComponentIdentifier()).getProjectPath()); | ||
| addDevModePaths(dep, a, projectDep); | ||
| Optional<IncludedBuild> includedBuild = includedBuild(project, a.getName()); |
There was a problem hiding this comment.
Couldn't this be replaced with project.getGradle().includedBuild(name)?
There was a problem hiding this comment.
yes, I just wrapped that in a try/catch because project.getGradle().includedBuild(name) throws an exception if the project is not an included build.
| @@ -402,6 +413,25 @@ private void addDevModePaths(final DependencyImpl dep, ResolvedArtifact a, Proje | |||
| } | |||
| } | |||
|
|
|||
| private void addSubstitutedProject(final DependencyImpl dep, File projectFile, LaunchMode mode) { | |||
There was a problem hiding this comment.
Just curious, have you tried to measure how long this method takes to execute? Is it seconds?
There was a problem hiding this comment.
I just ran some really basics benchmarks, it takes 2 to 4 seconds..
There was a problem hiding this comment.
Sorry, do you mind not using Optional in this case? Making includeBuild return boolean will make it look simpler and perform better.
There was a problem hiding this comment.
Oh yes for sure. I'm trying to understand why the test fails in ci and passes on my machine. I will remove the optional once everything is ok.
There was a problem hiding this comment.
I just ran some really basics benchmarks, it takes 2 to 4 seconds..
Just to clarify, was it a restart/reload that was taking 2 to 4 seconds in dev mode?
There was a problem hiding this comment.
It's for the first quarkusDev start. Reload is not impacted, and thanks to daemons/caches, next restart are not really impacted.
There was a problem hiding this comment.
Right, that's good. Thanks!
Right now, included build were ignored while creating the quarkus model.
This branch add support for included build by:
When looking for included builds, the gradle API only gives us the project name and the project directory. That's why I use the Quarkus model to collect included build infos.
close #11902