Merge changes from topic 'superproject_fixes'

* changes:
  Submodule subscriptions: accept allowed but unsubscribed subscriptions
  Correct inheriting superproject subscription test
  submodule subscriptions: ensure nested relative projects work

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractSubmoduleSubscription.java

@@ -71,13 +71,6 @@ public abstract class AbstractSubmoduleSubscription extends AbstractDaemonTest {
     return pushChangeTo(repo, "refs/heads/" + branch, "some change", "");
   }
 
-  protected void createSubmoduleSubscription(TestRepository<?> repo, String branch,
-      String subscribeToRepo, String subscribeToBranch) throws Exception {
-    Config config = new Config();
-    prepareSubmoduleConfigEntry(config, subscribeToRepo, subscribeToBranch);
-    pushSubmoduleConfig(repo, branch, config);
-  }
-
   protected void allowSubmoduleSubscription(String submodule, String subBranch,
       String superproject, String superBranch) throws Exception {
     Project.NameKey sub = new Project.NameKey(name(submodule));
@@ -99,6 +92,34 @@ public abstract class AbstractSubmoduleSubscription extends AbstractDaemonTest {
     }
   }
 
+  protected void createSubmoduleSubscription(TestRepository<?> repo, String branch,
+      String subscribeToRepo, String subscribeToBranch) throws Exception {
+    Config config = new Config();
+    prepareSubmoduleConfigEntry(config, subscribeToRepo, subscribeToBranch);
+    pushSubmoduleConfig(repo, branch, config);
+  }
+
+  protected void createRelativeSubmoduleSubscription(TestRepository<?> repo,
+      String branch, String subscribeToRepoPrefix, String subscribeToRepo,
+      String subscribeToBranch) throws Exception {
+    Config config = new Config();
+    prepareRelativeSubmoduleConfigEntry(config, subscribeToRepoPrefix,
+        subscribeToRepo, subscribeToBranch);
+    pushSubmoduleConfig(repo, branch, config);
+  }
+
+  protected void prepareRelativeSubmoduleConfigEntry(Config config,
+      String subscribeToRepoPrefix, String subscribeToRepo,
+      String subscribeToBranch) {
+    subscribeToRepo = name(subscribeToRepo);
+    String url = subscribeToRepoPrefix + subscribeToRepo;
+    config.setString("submodule", subscribeToRepo, "path", subscribeToRepo);
+    config.setString("submodule", subscribeToRepo, "url", url);
+    if (subscribeToBranch != null) {
+      config.setString("submodule", subscribeToRepo, "branch", subscribeToBranch);
+    }
+  }
+
   protected void prepareSubmoduleConfigEntry(Config config,
       String subscribeToRepo, String subscribeToBranch) {
     subscribeToRepo = name(subscribeToRepo);

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmoduleSubscriptionsIT.java

@@ -16,6 +16,7 @@ package com.google.gerrit.acceptance.git;
 
 import static com.google.common.truth.Truth.assertThat;
 
+import com.google.common.collect.Iterables;
 import com.google.gerrit.acceptance.GerritConfig;
 import com.google.gerrit.acceptance.NoHttpd;
 import com.google.gerrit.reviewdb.client.Project;
@@ -27,7 +28,9 @@ import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevTree;
 import org.eclipse.jgit.revwalk.RevWalk;
+import org.eclipse.jgit.transport.PushResult;
 import org.eclipse.jgit.transport.RefSpec;
+import org.eclipse.jgit.transport.RemoteRefUpdate;
 import org.junit.Test;
 
 @NoHttpd
@@ -339,20 +342,66 @@ public class SubmoduleSubscriptionsIT extends AbstractSubmoduleSubscription {
   @Test
   public void testSubscriptionInheritACL() throws Exception {
     createProjectWithPush("config-repo");
-    TestRepository<?> superRepo = createProjectWithPush("super-project",
+    createProjectWithPush("config-repo2",
         new Project.NameKey(name("config-repo")));
-    TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
-    allowSubmoduleSubscription("config-repo", "refs/heads/master",
-        "super-project", "refs/heads/wrong-branch");
+    TestRepository<?> superRepo = createProjectWithPush("super-project");
+    TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project",
+        new Project.NameKey(name("config-repo2")));
+    allowSubmoduleSubscription("config-repo", "refs/heads/*",
+        "super-project", "refs/heads/*");
 
     pushChangeTo(subRepo, "master");
     createSubmoduleSubscription(superRepo, "master",
         "subscribed-to-project", "master");
+    ObjectId subHEAD = pushChangeTo(subRepo, "master");
+    expectToHaveSubmoduleState(superRepo, "master",
+        "subscribed-to-project", subHEAD);
+  }
+
+  @Test
+  public void testAllowedButNotSubscribed() throws Exception {
+    TestRepository<?> superRepo = createProjectWithPush("super-project");
+    TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
+    allowSubmoduleSubscription("subscribed-to-project", "refs/heads/master",
+        "super-project", "refs/heads/master");
+
     pushChangeTo(subRepo, "master");
+    subRepo.branch("HEAD").commit().insertChangeId()
+        .message("some change")
+        .add("b.txt", "b contents for testing")
+        .create();
+    String refspec = "HEAD:refs/heads/master";
+    PushResult r = Iterables.getOnlyElement(subRepo.git().push()
+        .setRemote("origin")
+        .setRefSpecs(new RefSpec(refspec))
+        .call());
+    assertThat(r.getMessages()).doesNotContain("error");
+    assertThat(r.getRemoteUpdate("refs/heads/master").getStatus())
+    .isEqualTo(RemoteRefUpdate.Status.OK);
+
     assertThat(hasSubmodule(superRepo, "master",
         "subscribed-to-project")).isFalse();
   }
 
+  @Test
+  public void testSubscriptionDeepRelative() throws Exception {
+    TestRepository<?> superRepo = createProjectWithPush("super-project");
+    TestRepository<?> subRepo = createProjectWithPush(
+        "nested/subscribed-to-project");
+    // master is allowed to be subscribed to any superprojects branch:
+    allowSubmoduleSubscription("nested/subscribed-to-project",
+        "refs/heads/master", "super-project", null);
+
+    pushChangeTo(subRepo, "master");
+    createRelativeSubmoduleSubscription(superRepo, "master",
+        "../", "nested/subscribed-to-project", "master");
+
+    ObjectId subHEAD = pushChangeTo(subRepo, "master");
+
+    expectToHaveSubmoduleState(superRepo, "master",
+        "nested/subscribed-to-project", subHEAD);
+  }
+
   private void deleteAllSubscriptions(TestRepository<?> repo, String branch)
       throws Exception {
     repo.git().fetch().setRemote("origin").call();

gerrit-server/src/main/java/com/google/gerrit/server/git/GitModules.java

@@ -37,6 +37,7 @@ import org.slf4j.LoggerFactory;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.Set;
 
 /**
@@ -53,11 +54,7 @@ public class GitModules {
 
   private static final String GIT_MODULES = ".gitmodules";
 
-  private final String canonicalWebUrl;
-  private final Branch.NameKey branch;
   private final String submissionId;
-  private final MergeOpRepoManager orm;
-
   Set<SubmoduleSubscription> subscriptions;
 
   @AssistedInject
@@ -65,14 +62,9 @@ public class GitModules {
       @CanonicalWebUrl @Nullable String canonicalWebUrl,
       @Assisted Branch.NameKey branch,
       @Assisted String submissionId,
-      @Assisted MergeOpRepoManager orm) {
-    this.orm = orm;
-    this.branch = branch;
+      @Assisted MergeOpRepoManager orm) throws IOException {
     this.submissionId = submissionId;
-    this.canonicalWebUrl = canonicalWebUrl;
-  }
 
-  void load() throws IOException {
     Project.NameKey project = branch.getParentKey();
     logDebug("Loading .gitmodules of {} for project {}", branch, project);
     try {
@@ -91,18 +83,18 @@ public class GitModules {
     TreeWalk tw = TreeWalk.forPath(or.repo, GIT_MODULES, commit.getTree());
     if (tw == null
         || (tw.getRawMode(0) & FileMode.TYPE_MASK) != FileMode.TYPE_FILE) {
+      subscriptions = Collections.emptySet();
       return;
     }
+    BlobBasedConfig bbc;
     try {
-      BlobBasedConfig bbc =
-          new BlobBasedConfig(null, or.repo, commit, GIT_MODULES);
-      subscriptions = new SubmoduleSectionParser(bbc, canonicalWebUrl,
-          branch).parseAllSections();
+      bbc = new BlobBasedConfig(null, or.repo, commit, GIT_MODULES);
     } catch (ConfigInvalidException e) {
-      throw new IOException(
-          "Could not read .gitmodule file of super project: " +
+      throw new IOException("Could not read .gitmodules of super project: " +
               branch.getParentKey(), e);
     }
+    subscriptions = new SubmoduleSectionParser(bbc, canonicalWebUrl,
+          branch).parseAllSections();
   }
 
   public Collection<SubmoduleSubscription> subscribedTo(Branch.NameKey src) {

gerrit-server/src/main/java/com/google/gerrit/server/git/SubmoduleOp.java

@@ -150,7 +150,6 @@ public class SubmoduleOp {
           getDestinationBranches(branch, s, orm);
       for (Branch.NameKey targetBranch : branches) {
         GitModules m = gitmodulesFactory.create(targetBranch, updateId, orm);
-        m.load();
         for (SubmoduleSubscription ss : m.subscribedTo(branch)) {
           logDebug("Checking SubmoduleSubscription " + ss);
           if (projectCache.get(ss.getSubmodule().getParentKey()) != null) {