[GitHub] [maven] rfscholte commented on a change in pull request #603: [MPH-183] BOM hierarchy in InputSource

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

rfscholte commented on a change in pull request #603:
URL: https://github.com/apache/maven/pull/603#discussion_r745439915



##########
File path: maven-model-builder/src/main/java/org/apache/maven/model/composition/DefaultDependencyManagementImporter.java
##########
@@ -83,4 +90,43 @@ public void importManagement( Model target, List<? extends DependencyManagement>
         }
     }
 
+    static void updateDependencyHierarchy( Dependency dependency, DependencyManagement bom )
+    {
+        // We are only interested in the InputSource, so the location of the <dependency> element is sufficient
+        InputLocation dependencyLocation = dependency.getLocation( "" );
+        InputLocation bomLocation = bom.getLocation( "" );
+
+        if ( dependencyLocation == null || bomLocation == null )
+        {
+            return;
+        }
+
+        InputSource hierarchicalSource = dependencyLocation.getSource();

Review comment:
       This is actually very interesting: it looks like it is possible to even use InputLocation instead of InputSource, whcih would give even more details of the path. How about using that instead?

##########
File path: maven-model/src/main/mdo/maven.mdo
##########
@@ -3137,6 +3137,20 @@
             ]]>
           </description>
         </field>
+        <field>
+          <name>importedBy</name>

Review comment:
       Thinking out loud: there are more ways to define a path to a dependency. Via parent is another one, just like [maven-tiles](https://github.com/repaint-io/maven-tiles) by @talios. There are plans to support import for plugins too. 
   In the end there's only 1 path, so let's use a more generic name.
   Not sure if there is a need to keep track of the type of reference (parent, import-dependency), so let's do that if people ask for it.  

##########
File path: maven-model-builder/src/main/java/org/apache/maven/model/composition/DefaultDependencyManagementImporter.java
##########
@@ -83,4 +90,43 @@ public void importManagement( Model target, List<? extends DependencyManagement>
         }
     }
 
+    static void updateDependencyHierarchy( Dependency dependency, DependencyManagement bom )
+    {
+        // We are only interested in the InputSource, so the location of the <dependency> element is sufficient
+        InputLocation dependencyLocation = dependency.getLocation( "" );
+        InputLocation bomLocation = bom.getLocation( "" );
+
+        if ( dependencyLocation == null || bomLocation == null )
+        {
+            return;
+        }
+
+        InputSource hierarchicalSource = dependencyLocation.getSource();
+        InputSource bomSource = bomLocation.getSource();
+
+        // Skip if the dependency and bom have the same source
+        if ( hierarchicalSource == null || bomSource == null || Objects.equals( hierarchicalSource.getModelId(),
+                bomSource.getModelId() ) )
+        {
+            return;
+        }
+
+        while ( hierarchicalSource.getImportedBy() != null )

Review comment:
       Is this loop really necessary? Would love to see an example that needs it. (maybe as an integration test)




-- 
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: issues-unsubscribe@maven.apache.org

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