Find parent and submodule definitions before fully resolving a pom

The Maven API has two options for parsing pom files: getting the "raw model"
(which parses the pom to ensure it is correct, but doesn't do any resolution nor
variable substitution) and getting the "effective model," which does all
resolution. Before this CL, generate_workspace was just getting the effective
model immediately, which is easily foiled by having an artifact that depends on
a "sibling" (the parent cannot be resolved until the siblings are and the
siblings can only be resolved if they happen to arrive in the right order).

This changes the code to get the raw models first to get locations of all
artifacts, then fully resolve them.

This fixes #383.

--
MOS_MIGRATED_REVID=101129094

6 files changed, 110 insertions, 42 deletions

diff --git a/src/main/java/com/google/devtools/build/workspace/GenerateWorkspace.java b/src/main/java/com/google/devtools/build/workspace/GenerateWorkspace.java
index 6473cc5550..93313da713 100644
--- a/src/main/java/com/google/devtools/build/workspace/GenerateWorkspace.java
+++ b/src/main/java/com/google/devtools/build/workspace/GenerateWorkspace.java
@@ -39,7 +39,7 @@ import java.util.List;
  */
 public class GenerateWorkspace {
 
-  private final StoredEventHandler handler;
+  private final EventHandler handler;
   private final FileSystem fileSystem;
   private final com.google.devtools.build.workspace.maven.Resolver resolver;
   private final Path outputDir;
@@ -76,7 +76,7 @@ public class GenerateWorkspace {
   }
 
   private GenerateWorkspace(String outputDir) {
-    this.handler = new StoredEventHandler();
+    this.handler = new EventHandler();
     this.fileSystem = getFileSystem();
     this.resolver = new com.google.devtools.build.workspace.maven.Resolver(handler);
     if (outputDir.isEmpty()) {
@@ -148,4 +148,11 @@ public class GenerateWorkspace {
       System.err.println(event);
     }
   }
+
+  private class EventHandler extends StoredEventHandler {
+    @Override
+    public void handle(Event event) {
+      System.err.println(event.getKind() + ": " + event.getMessage());
+    }
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/workspace/Resolver.java b/src/main/java/com/google/devtools/build/workspace/Resolver.java
index a7bce54a39..595f11fd2a 100644
--- a/src/main/java/com/google/devtools/build/workspace/Resolver.java
+++ b/src/main/java/com/google/devtools/build/workspace/Resolver.java
@@ -36,7 +36,6 @@ import com.google.devtools.build.workspace.maven.DefaultModelResolver;
 import com.google.devtools.build.workspace.maven.Rule;
 
 import org.apache.maven.model.building.ModelSource;
-import org.apache.maven.model.resolution.InvalidRepositoryException;
 import org.apache.maven.model.resolution.UnresolvableModelException;
 
 import java.io.IOException;
@@ -116,9 +115,9 @@ public class Resolver {
           if (attributeMap.has("repository", Type.STRING)
               && !attributeMap.get("repository", Type.STRING).isEmpty()) {
             modelResolver.addUserRepository(attributeMap.get("repository", Type.STRING));
-            rule.setRepository(attributeMap.get("repository", Type.STRING));
+            rule.setRepository(attributeMap.get("repository", Type.STRING), handler);
           }
-        } catch (Rule.InvalidRuleException | InvalidRepositoryException e) {
+        } catch (Rule.InvalidRuleException e) {
           handler.handle(Event.error(location, "Couldn't get attribute: " + e.getMessage()));
           return;
         }
@@ -132,7 +131,7 @@ public class Resolver {
               "Could not resolve model for " + target + ": " + e.getMessage()));
           continue;
         }
-        resolver.resolveModelSource(modelSource);
+        resolver.resolveEffectiveModel(modelSource);
       } else {
         handler.handle(Event.warn(location, "Cannot fetch transitive dependencies for " + target
             + " yet, skipping"));
diff --git a/src/main/java/com/google/devtools/build/workspace/maven/BUILD b/src/main/java/com/google/devtools/build/workspace/maven/BUILD
index cb68b8bcd8..8844f937a1 100644
--- a/src/main/java/com/google/devtools/build/workspace/maven/BUILD
+++ b/src/main/java/com/google/devtools/build/workspace/maven/BUILD
@@ -31,6 +31,7 @@ java_library(
         "//src/main/java/com/google/devtools/build:__subpackages__",
     ],
     deps = [
+        "//src/main/java:events",
         "//src/main/java:maven-connector",
         "//third_party:aether",
         "//third_party:guava",
diff --git a/src/main/java/com/google/devtools/build/workspace/maven/DefaultModelResolver.java b/src/main/java/com/google/devtools/build/workspace/maven/DefaultModelResolver.java
index 800b9c6c46..c903c02368 100644
--- a/src/main/java/com/google/devtools/build/workspace/maven/DefaultModelResolver.java
+++ b/src/main/java/com/google/devtools/build/workspace/maven/DefaultModelResolver.java
@@ -24,7 +24,6 @@ import org.apache.maven.model.Parent;
 import org.apache.maven.model.Repository;
 import org.apache.maven.model.building.ModelSource;
 import org.apache.maven.model.building.UrlModelSource;
-import org.apache.maven.model.resolution.InvalidRepositoryException;
 import org.apache.maven.model.resolution.ModelResolver;
 import org.apache.maven.model.resolution.UnresolvableModelException;
 
@@ -60,6 +59,10 @@ public class DefaultModelResolver implements ModelResolver {
   @Override
   public ModelSource resolveModel(String groupId, String artifactId, String version)
       throws UnresolvableModelException {
+    String artifact = Rule.name(groupId, artifactId);
+    if (artifactToUrl.containsKey(artifact)) {
+      return artifactToUrl.get(artifact);
+    }
     for (Repository repository : repositories) {
       UrlModelSource modelSource = getModelSource(
           repository.getUrl(), groupId, artifactId, version);
@@ -124,13 +127,12 @@ public class DefaultModelResolver implements ModelResolver {
   }
 
   @Override
-  public void addRepository(Repository repository) throws InvalidRepositoryException {
+  public void addRepository(Repository repository) {
     repositories.add(repository);
   }
 
   @Override
-  public void addRepository(Repository repository, boolean replace)
-      throws InvalidRepositoryException {
+  public void addRepository(Repository repository, boolean replace) {
     addRepository(repository);
   }
 
@@ -142,7 +144,7 @@ public class DefaultModelResolver implements ModelResolver {
   /**
    * Adds a user-specified repository to the list.
    */
-  public void addUserRepository(String url) throws InvalidRepositoryException {
+  public void addUserRepository(String url) {
     Repository repository = new Repository();
     repository.setUrl(url);
     repository.setId("user-defined repository");
@@ -150,4 +152,12 @@ public class DefaultModelResolver implements ModelResolver {
     addRepository(repository);
   }
 
+  public boolean putModelSource(String groupId, String artifactId, ModelSource modelSource) {
+    String key = Rule.name(groupId, artifactId);
+    if (!artifactToUrl.containsKey(key)) {
+      artifactToUrl.put(key, modelSource);
+      return true;
+    }
+    return false;
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/workspace/maven/Resolver.java b/src/main/java/com/google/devtools/build/workspace/maven/Resolver.java
index 35fba17a8a..955a74be59 100644
--- a/src/main/java/com/google/devtools/build/workspace/maven/Resolver.java
+++ b/src/main/java/com/google/devtools/build/workspace/maven/Resolver.java
@@ -20,6 +20,7 @@ import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.EventHandler;
 
 import org.apache.maven.model.Model;
+import org.apache.maven.model.Parent;
 import org.apache.maven.model.Repository;
 import org.apache.maven.model.building.DefaultModelBuilder;
 import org.apache.maven.model.building.DefaultModelBuilderFactory;
@@ -29,13 +30,17 @@ import org.apache.maven.model.building.FileModelSource;
 import org.apache.maven.model.building.ModelBuildingException;
 import org.apache.maven.model.building.ModelBuildingResult;
 import org.apache.maven.model.building.ModelSource;
+import org.apache.maven.model.composition.DefaultDependencyManagementImporter;
 import org.apache.maven.model.io.DefaultModelReader;
 import org.apache.maven.model.locator.DefaultModelLocator;
+import org.apache.maven.model.management.DefaultDependencyManagementInjector;
+import org.apache.maven.model.management.DefaultPluginManagementInjector;
+import org.apache.maven.model.plugin.DefaultPluginConfigurationExpander;
 import org.apache.maven.model.profile.DefaultProfileSelector;
-import org.apache.maven.model.resolution.InvalidRepositoryException;
 import org.apache.maven.model.resolution.UnresolvableModelException;
 
 import java.io.File;
+import java.io.IOException;
 import java.io.PrintStream;
 import java.util.List;
 import java.util.Map;
@@ -61,7 +66,11 @@ public class Resolver {
     this.headers = Lists.newArrayList();
     this.deps = Maps.newHashMap();
     this.modelBuilder = new DefaultModelBuilderFactory().newInstance()
-        .setProfileSelector(new DefaultProfileSelector());
+        .setProfileSelector(new DefaultProfileSelector())
+        .setPluginConfigurationExpander(new DefaultPluginConfigurationExpander())
+        .setPluginManagementInjector(new DefaultPluginManagementInjector())
+        .setDependencyManagementImporter(new DefaultDependencyManagementImporter())
+        .setDependencyManagementInjector(new DefaultDependencyManagementInjector());
     this.modelResolver = new DefaultModelResolver();
   }
 
@@ -118,20 +127,11 @@ public class Resolver {
     processor.setModelReader(new DefaultModelReader());
     File pom = processor.locatePom(new File(projectPath));
     addHeader(pom.getAbsolutePath());
-    Model model = resolveModelSource(new FileModelSource(pom));
-
-    // For the top-level pom _only_, resolve all of its submodules.
-    resolveSubmodules(model, pom);
-  }
-
-  /**
-   * This calls resolvePomDependencies on each submodule of the model, thus filling in the
-   * transitive submodules' dependencies as well as the main project's.
-   */
-  private void resolveSubmodules(Model model, File pom) {
-    for (String module : model.getModules()) {
-      resolvePomDependencies(pom.getParent() + "/" + module);
-    }
+    FileModelSource pomSource = new FileModelSource(pom);
+    // First resolve the model source locations.
+    resolveSourceLocations(pomSource);
+    // Next, fully resolve the models.
+    resolveEffectiveModel(pomSource);
   }
 
   /**
@@ -140,7 +140,7 @@ public class Resolver {
    * @return the model.
    */
   @Nullable
-  public Model resolveModelSource(ModelSource modelSource) {
+  public Model resolveEffectiveModel(ModelSource modelSource) {
     DefaultModelBuildingRequest request = new DefaultModelBuildingRequest();
     request.setModelResolver(modelResolver);
     request.setModelSource(modelSource);
@@ -154,15 +154,8 @@ public class Resolver {
           + ": " + e.getMessage()));
       return null;
     }
-
     for (Repository repo : model.getRepositories()) {
-      try {
-        modelResolver.addRepository(repo);
-      } catch (InvalidRepositoryException e) {
-        handler.handle(Event.error("Unable to add repository " + repo.getName()
-            + " (" + repo.getId() + "," + repo.getUrl() + ")"));
-        return model;
-      }
+      modelResolver.addRepository(repo);
     }
 
     for (org.apache.maven.model.Dependency dependency : model.getDependencies()) {
@@ -176,8 +169,8 @@ public class Resolver {
           ModelSource depModelSource = modelResolver.resolveModel(
               dependency.getGroupId(), dependency.getArtifactId(), dependency.getVersion());
           if (depModelSource != null) {
-            artifactRule.setRepository(depModelSource.getLocation());
-            resolveModelSource(depModelSource);
+            artifactRule.setRepository(depModelSource.getLocation(), handler);
+            resolveEffectiveModel(depModelSource);
           } else {
             handler.handle(Event.error("Could not get a model for " + dependency));
           }
@@ -192,6 +185,56 @@ public class Resolver {
   }
 
   /**
+   * Find the POM files for a given pom's parent(s) and submodules.
+   */
+  private void resolveSourceLocations(FileModelSource fileModelSource) {
+    DefaultModelBuildingRequest request = new DefaultModelBuildingRequest();
+    request.setModelResolver(modelResolver);
+    request.setModelSource(fileModelSource);
+    Model model;
+    try {
+      ModelBuildingResult result = modelBuilder.build(request);
+      model = result.getRawModel();
+    } catch (ModelBuildingException | IllegalArgumentException e) {
+      // IllegalArg can be thrown if the parent POM cannot be resolved.
+      handler.handle(Event.error("Unable to resolve raw Maven model from "
+          + fileModelSource.getLocation() + ": " + e.getMessage()));
+      return;
+    }
+
+    // Self.
+    Parent parent = model.getParent();
+    if (model.getGroupId() == null) {
+      model.setGroupId(parent.getGroupId());
+    }
+    if (!modelResolver.putModelSource(
+        model.getGroupId(), model.getArtifactId(), fileModelSource)) {
+      return;
+    }
+
+    // Parent.
+    File pomDirectory = new File(fileModelSource.getLocation()).getParentFile();
+    if (parent != null && !parent.getArtifactId().equals(model.getArtifactId())) {
+      File parentPom;
+      try {
+        parentPom = new File(pomDirectory, parent.getRelativePath()).getCanonicalFile();
+      } catch (IOException e) {
+        handler.handle(Event.error("Unable to get canonical path of " + pomDirectory + " and "
+            + parent.getRelativePath()));
+        return;
+      }
+      if (parentPom.exists()) {
+        resolveSourceLocations(new FileModelSource(parentPom));
+      }
+    }
+
+    // Submodules.
+    for (String module : model.getModules()) {
+      resolveSourceLocations(new FileModelSource(new File(pomDirectory, module + "/pom.xml")));
+    }
+  }
+
+  /**
    * Adds the artifact to the map of deps, if it is not already there. Returns if the artifact
    * was newly added. If the artifact was in the list at a different version, adds an annotation
    * about the desired version.
diff --git a/src/main/java/com/google/devtools/build/workspace/maven/Rule.java b/src/main/java/com/google/devtools/build/workspace/maven/Rule.java
index 379bad7821..fb7675d21a 100644
--- a/src/main/java/com/google/devtools/build/workspace/maven/Rule.java
+++ b/src/main/java/com/google/devtools/build/workspace/maven/Rule.java
@@ -17,6 +17,8 @@ package com.google.devtools.build.workspace.maven;
 import com.google.common.collect.Sets;
 
 import com.google.devtools.build.lib.bazel.repository.MavenConnector;
+import com.google.devtools.build.lib.events.Event;
+import com.google.devtools.build.lib.events.EventHandler;
 import org.apache.maven.model.Dependency;
 import org.eclipse.aether.artifact.Artifact;
 import org.eclipse.aether.artifact.DefaultArtifact;
@@ -83,15 +85,21 @@ public final class Rule {
     return groupId() + ":" + artifactId() + ":" + version();
   }
 
-  public void setRepository(String url) throws InvalidRuleException {
+  public void setRepository(String url, EventHandler handler) {
     // url is of the form repository/group/artifact/version/artifact-version.pom. Strip off
     // everything after repository/.
     int uriStart = url.indexOf(getUri());
     if (uriStart == -1) {
-      throw new InvalidRuleException("Cannot find expected URI (" + getUri()
-          + ") in URL (" + url + ")");
+      // If url is actually a path to a file, it won't match the URL pattern described above.
+      // However, in that case we also have no way of fetching the artifact, so we'll print a
+      // warning.
+      handler.handle(Event.warn(name() + " was defined in " + url
+          + " which isn't a repository URL, so we couldn't figure out how to fetch "
+          + toMavenArtifactString() + " in a general way. You will need to set the \"repository\""
+          + " attribute manually"));
+    } else {
+      this.repository = url.substring(0, uriStart);
     }
-    this.repository = url.substring(0, uriStart);
   }
 
   private String getUri() {