[GitHub] [netbeans] sdedic commented on a diff in pull request #3942: Fix project load caching

[GitBox <gi...@apache.org>]

sdedic commented on code in PR #3942:
URL: https://github.com/apache/netbeans/pull/3942#discussion_r845782669


##########
extide/gradle/netbeans-gradle-tooling/src/main/java/org/netbeans/modules/gradle/tooling/NbProjectInfoBuilder.java:
##########
@@ -433,7 +442,12 @@ private void detectDependencies(NbProjectInfoModel model) {
                             ResolvedDependencyResult rdr = (ResolvedDependencyResult) it2;
                             if (rdr.getRequested() instanceof ModuleComponentSelector) {
                                 ids.add(rdr.getSelected().getId());
-                                componentIds.add(rdr.getSelected().getId().toString());
+                                // do not bother with components that only select a variant, which is itself a component
+                                // TODO: represent as a special component type so the IDE shows it, but the IDE knows it is an abstract
+                                // intermediate with no artifact(s).
+                                if (!rdr.getResolvedVariant().getExternalVariant().isPresent()) {

Review Comment:
   I found out that regular JAR dependencies do not have `externalVariant` populated.



##########
extide/gradle/netbeans-gradle-tooling/src/main/java/org/netbeans/modules/gradle/tooling/NbProjectInfoBuilder.java:
##########
@@ -627,7 +650,7 @@ private void collectModuleDependencies(final NbProjectInfoModel model, String co
     private Set<Configuration> configurationsToSave() {
         return project
                 .getConfigurations()
-                .matching(c -> !CONFIG_EXCLUDES.contains(c.getName()))
+                .matching(c -> !CONFIG_EXCLUDES_PATTERN.matcher(c.getName()).matches())

Review Comment:
   The added regexp excludes all `*DependencyMetadata` configurations like `compileOnlyDependencyMetadata` used by Kotlin plugin. Useless for the IDE, but since the module has no JVM artifacts, it confuses the caching / validation mechanism.



##########
extide/gradle/netbeans-gradle-tooling/src/main/java/org/netbeans/modules/gradle/tooling/NbProjectInfoBuilder.java:
##########
@@ -172,7 +180,8 @@ private void detectProjectMetadata(NbProjectInfoModel model) {
         model.getInfo().put("project_buildDir", project.getBuildDir());
         model.getInfo().put("project_projectDir", project.getProjectDir());
         model.getInfo().put("project_rootDir", project.getRootDir());
-        model.getInfo().put("gradle_user_home", project.getGradle().getGradleHomeDir());
+        model.getInfo().put("gradle_user_home", project.getGradle().getGradleUserHomeDir());

Review Comment:
   Passing `gradleHomeDir` instead of userdir caused the `GradleModuleFileCache21` to work over **gradle distribution** directory rather than **user cache** directory, effectively disabling the accessor.



##########
extide/gradle/netbeans-gradle-tooling/src/main/java/org/netbeans/modules/gradle/tooling/NbProjectInfoBuilder.java:
##########
@@ -487,6 +501,15 @@ private void detectDependencies(NbProjectInfoModel model) {
 
             if (resolvable(it)) {
                 try {
+                    Set<ResolvedArtifact> arts = it.getResolvedConfiguration()

Review Comment:
   The `collectArtifacts` [at line 516](https://github.com/apache/netbeans/pull/3942/files#diff-9fd2231c8ce99fda4d6d8295c69707bf67f91b6b492ee886a736f8a4a6d55a08R516) fetches modules recursively, but JARs are not. Therefore the `jvm_resolved_artifacts` map was incomplete: did not assign artifacts for all components turned into `ModuleDependency` for the `micronaut-core` project.
   Adding this provides as much JAR locations as possible. Checked that does not need much additional time compared to original.



##########
extide/gradle/src/org/netbeans/modules/gradle/NbGradleProjectImpl.java:
##########
@@ -477,9 +477,10 @@ public boolean satisifes(LoadingCF other) {
                     this.loading = null;
                 }
             }
+            f.complete(prj);

Review Comment:
   This is tricky. I've encountered a situation when:
   * the project was loading
   * fired the [event at line 481](https://github.com/apache/netbeans/pull/3942/files#diff-e833b1ad0623c45c9af375ded2ba2f854c529638f4334c624969e3ea40a6e05eL481),
   * it went from the listener through a series of other listeners (classpath changes), 
   * then eventually attempted to obtain the GradleProject - in the same execution thread
   * the future was NOT completed yet (as it would have been completed after the event dispatch ended)
   * the loader was blocked
   * all other gradle reloads were blocked, as the RP has throughput 1
   
   An opinion on the fix is needed. This proposed version is simple: fire event only after the future completes. A code that does `NbGradleProject.toQuality(...).thenApply()` will progress with the loaded project, BUT then it gets a change event ... and may perform additional processing / updates, although the state is clean.
   It could be possible to introduce a `ThreadLocal` that would return completed future just for the event firing thread; event will be delivered **before** the (public) Future actually completes. But this does not prevent a deadlock between more threads: firing thread waiting on X, while X is blocked on Future.get()
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists