ConfigEntry: support glob destination branches

Conf only allows a 1:1 mapping or a *:* mapping between manifest and
superproject branches. In setups with one manifest repo feeding
multiple superproject, this produces long and error-prone
configurations, with many entries and long exclude lists.

Support glob destinations in the conf, e.g.
[superproject "superproject:refs/heads/*-release"]
or
[superproject "superproject:refs/heads/a-*-dev"]

Change-Id: Ia1cec87f6db221b7a321941e59421c2972714c56
diff --git a/java/com/googlesource/gerrit/plugins/supermanifest/ConfigEntry.java b/java/com/googlesource/gerrit/plugins/supermanifest/ConfigEntry.java
index 066cfd4..0beaff2 100644
--- a/java/com/googlesource/gerrit/plugins/supermanifest/ConfigEntry.java
+++ b/java/com/googlesource/gerrit/plugins/supermanifest/ConfigEntry.java
@@ -64,9 +64,9 @@
           String.format("invalid destination '%s'. Must specify refs/heads/", destRef));
     }
 
-    if (destRef.contains("*") && !destRef.equals(REFS_HEADS + "*")) {
+    if (destRef.indexOf("*") != destRef.lastIndexOf('*')) {
       throw new ConfigInvalidException(
-          String.format("invalid destination '%s'. Use just '*' for all branches.", destRef));
+          String.format("invalid destination '%s' has more than one '*'", destRef));
     }
 
     String srcRepo = cfg.getString(SECTION_NAME, name, "srcRepo");
@@ -92,7 +92,7 @@
             String.format("entry %s has invalid toolType: %s", name, toolType));
     }
 
-    if (destRef.equals(REFS_HEADS + "*")) {
+    if (destRef.contains("*")) {
       srcRef = "";
     } else {
       if (!Repository.isValidRefName(destRef)) {
@@ -139,8 +139,8 @@
 
   public String src() {
     String src = srcRef;
-    if (destBranch.equals("*")) {
-      src = "*";
+    if (destBranch.contains("*")) {
+      src = destBranch;
     }
     return srcRepoKey + ":" + src + ":" + xmlPath;
   }
@@ -223,7 +223,7 @@
    * @return the short-named destination branch (assumed under refs/heads)
    */
   public String getActualDestBranch(String updatedRef) {
-    if (destBranch.equals("*")) {
+    if (destBranch.contains("*")) {
       // We can take the ref, because we know it matched this conf before
       return updatedRef.substring(REFS_HEADS.length());
     }
@@ -235,12 +235,19 @@
       return false;
     }
 
-    if (!(destBranch.equals("*") || srcRef.equals(refName))) {
+    if (!(destBranch.contains("*") || srcRef.equals(refName))) {
       return false;
     }
 
-    if (destBranch.equals("*") && !refName.startsWith(REFS_HEADS)) {
-      return false;
+    if (destBranch.contains("*")) {
+      if (!refName.startsWith(REFS_HEADS)) {
+        return false;
+      }
+
+      RefSpec destSpec = new RefSpec(REFS_HEADS + destBranch, WildcardMode.ALLOW_MISMATCH);
+      if (!destSpec.matchSource(refName)) {
+        return false;
+      }
     }
 
     if (excludesRef(refName)) {
diff --git a/java/com/googlesource/gerrit/plugins/supermanifest/ConfigParser.java b/java/com/googlesource/gerrit/plugins/supermanifest/ConfigParser.java
new file mode 100644
index 0000000..eebc402
--- /dev/null
+++ b/java/com/googlesource/gerrit/plugins/supermanifest/ConfigParser.java
@@ -0,0 +1,99 @@
+package com.googlesource.gerrit.plugins.supermanifest;
+
+import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.entities.Project.NameKey;
+import com.google.gerrit.extensions.annotations.PluginName;
+import com.google.gerrit.server.config.AllProjectsName;
+import com.google.gerrit.server.config.PluginConfigFactory;
+import com.google.gerrit.server.project.NoSuchProjectException;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+import javax.inject.Inject;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.lib.Config;
+
+public class ConfigParser {
+  private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
+  private final PluginConfigFactory cfgFactory;
+  private final String pluginName;
+  private final AllProjectsName allProjectsName;
+
+  @Inject
+  ConfigParser(
+      AllProjectsName allProjectsName,
+      @PluginName String pluginName,
+      PluginConfigFactory cfgFactory) {
+    this.allProjectsName = allProjectsName;
+    this.pluginName = pluginName;
+    this.cfgFactory = cfgFactory;
+  }
+
+  /*
+     [superproject "submodules:refs/heads/nyc"]
+        srcRepo = platforms/manifest
+        srcRef = refs/heads/nyc
+        srcPath = manifest.xml
+  */
+  Set<ConfigEntry> parseConfiguration() throws NoSuchProjectException {
+    Config cfg = cfgFactory.getProjectPluginConfig(allProjectsName, pluginName);
+
+    Set<ConfigEntry> newConf = new HashSet<>();
+    Set<String> destinations = new HashSet<>();
+    Set<String> wildcardDestinations = new HashSet<>();
+    Map<NameKey, NameKey> globbedMappings = new HashMap<>();
+    Set<String> sources = new HashSet<>();
+
+    for (String sect : cfg.getSections()) {
+      if (!sect.equals(ConfigEntry.SECTION_NAME)) {
+        logger.atWarning().log("%s.config: ignoring invalid section %s", pluginName, sect);
+      }
+    }
+    for (String subsect : cfg.getSubsections(ConfigEntry.SECTION_NAME)) {
+      try {
+        ConfigEntry configEntry = new ConfigEntry(cfg, subsect);
+        if (destinations.contains(configEntry.srcRepoKey.get())
+            || sources.contains(configEntry.destRepoKey.get())) {
+          // Don't want cyclic dependencies.
+          throw new ConfigInvalidException(
+              String.format("repo in entry %s cannot be both source and destination", configEntry));
+        }
+        if (configEntry.destBranch.equals("*")) {
+          if (wildcardDestinations.contains(configEntry.destRepoKey.get())) {
+            throw new ConfigInvalidException(
+                String.format(
+                    "repo %s already has a wildcard destination branch.", configEntry.destRepoKey));
+          }
+          wildcardDestinations.add(configEntry.destRepoKey.get());
+        }
+
+        // Don't allow globbed writes from two different sources to the same destination.
+        // Maybe the globs do not overlap, but we don't have an easy way to find that out, so
+        // better safe than sorry.
+        if (configEntry.destBranch.contains("*")) {
+          NameKey knownSource = globbedMappings.get(configEntry.destRepoKey);
+          if (knownSource != null && !knownSource.equals(configEntry.srcRepoKey)) {
+            throw new ConfigInvalidException(
+                String.format(
+                    "repo %s has globbled destinations from at least two sources %s and %s",
+                    configEntry.destRepoKey,
+                    configEntry.srcRepoKey,
+                    globbedMappings.get(configEntry.srcRepoKey)));
+          }
+          globbedMappings.put(configEntry.destRepoKey, configEntry.srcRepoKey);
+        }
+
+        sources.add(configEntry.srcRepoKey.get());
+        destinations.add(configEntry.destRepoKey.get());
+
+        newConf.add(configEntry);
+      } catch (ConfigInvalidException e) {
+        logger.atSevere().log("invalid configuration: %s", e);
+      }
+    }
+
+    return newConf;
+  }
+}
diff --git a/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestRefUpdatedListener.java b/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestRefUpdatedListener.java
index aa5f94b..b232214 100644
--- a/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestRefUpdatedListener.java
+++ b/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestRefUpdatedListener.java
@@ -23,7 +23,6 @@
 import com.google.errorprone.annotations.FormatString;
 import com.google.gerrit.entities.Project;
 import com.google.gerrit.entities.RefNames;
-import com.google.gerrit.extensions.annotations.PluginName;
 import com.google.gerrit.extensions.api.projects.BranchInput;
 import com.google.gerrit.extensions.config.DownloadScheme;
 import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
@@ -41,7 +40,6 @@
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.config.AllProjectsName;
 import com.google.gerrit.server.config.CanonicalWebUrl;
-import com.google.gerrit.server.config.PluginConfigFactory;
 import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.logging.PluginMetadata;
 import com.google.gerrit.server.permissions.GlobalPermission;
@@ -65,6 +63,7 @@
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
@@ -76,7 +75,6 @@
 import org.eclipse.jgit.errors.RevisionSyntaxException;
 import org.eclipse.jgit.gitrepo.RepoCommand;
 import org.eclipse.jgit.gitrepo.RepoCommand.RemoteFile;
-import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.PersonIdent;
 import org.eclipse.jgit.lib.Ref;
@@ -97,10 +95,9 @@
 
   private final SuperManifestRepoManager.Factory repoManagerFactory;
   private final URI canonicalWebUrl;
-  private final PluginConfigFactory cfgFactory;
-  private final String pluginName;
   private final AllProjectsName allProjectsName;
   private final ProjectCache projectCache;
+  private final ConfigParser configParser;
   private final Provider<PersonIdent> serverIdent;
   private final Provider<IdentifiedUser> identifiedUser;
   private final PermissionBackend permissionBackend;
@@ -112,9 +109,8 @@
   SuperManifestRefUpdatedListener(
       AllProjectsName allProjectsName,
       @CanonicalWebUrl String canonicalWebUrl,
-      @PluginName String pluginName,
       PluginMapContext<DownloadScheme> downloadScheme,
-      PluginConfigFactory cfgFactory,
+      ConfigParser configParser,
       ProjectCache projectCache,
       @GerritPersonIdent Provider<PersonIdent> serverIdent,
       SuperManifestRepoManager.Factory repoManagerFactory,
@@ -122,7 +118,7 @@
       PermissionBackend permissionBackend,
       MetricMaker metrics) {
 
-    this.pluginName = pluginName;
+    this.configParser = configParser;
     this.serverIdent = serverIdent;
     this.allProjectsName = allProjectsName;
     this.repoManagerFactory = repoManagerFactory;
@@ -133,7 +129,6 @@
     }
 
     this.downloadScheme = downloadScheme;
-    this.cfgFactory = cfgFactory;
     this.projectCache = projectCache;
     this.identifiedUser = identifiedUser;
     this.permissionBackend = permissionBackend;
@@ -191,57 +186,6 @@
     logger.atInfo().log("%s: %s", canonicalWebUrl, String.format(formatStr, args));
   }
 
-  /*
-     [superproject "submodules:refs/heads/nyc"]
-        srcRepo = platforms/manifest
-        srcRef = refs/heads/nyc
-        srcPath = manifest.xml
-  */
-  private Set<ConfigEntry> parseConfiguration(PluginConfigFactory cfgFactory, String name)
-      throws NoSuchProjectException {
-    Config cfg = cfgFactory.getProjectPluginConfig(allProjectsName, name);
-
-    Set<ConfigEntry> newConf = new HashSet<>();
-    Set<String> destinations = new HashSet<>();
-    Set<String> wildcardDestinations = new HashSet<>();
-    Set<String> sources = new HashSet<>();
-
-    for (String sect : cfg.getSections()) {
-      if (!sect.equals(ConfigEntry.SECTION_NAME)) {
-        warn("%s.config: ignoring invalid section %s", name, sect);
-      }
-    }
-    for (String subsect : cfg.getSubsections(ConfigEntry.SECTION_NAME)) {
-      try {
-        ConfigEntry configEntry = new ConfigEntry(cfg, subsect);
-        if (destinations.contains(configEntry.srcRepoKey.get())
-            || sources.contains(configEntry.destRepoKey.get())) {
-          // Don't want cyclic dependencies.
-          throw new ConfigInvalidException(
-              String.format("repo in entry %s cannot be both source and destination", configEntry));
-        }
-        if (configEntry.destBranch.equals("*")) {
-          if (wildcardDestinations.contains(configEntry.destRepoKey.get())) {
-            throw new ConfigInvalidException(
-                String.format(
-                    "repo %s already has a wildcard destination branch.", configEntry.destRepoKey));
-          }
-          wildcardDestinations.add(configEntry.destRepoKey.get());
-        }
-
-        sources.add(configEntry.srcRepoKey.get());
-        destinations.add(configEntry.destRepoKey.get());
-
-        newConf.add(configEntry);
-
-      } catch (ConfigInvalidException e) {
-        error("invalid configuration: %s", e);
-      }
-    }
-
-    return newConf;
-  }
-
   private boolean checkRepoExists(Project.NameKey id) {
     return projectCache.get(id) != null;
   }
@@ -277,8 +221,7 @@
   }
 
   private ImmutableSet<ConfigEntry> getConfiguration() throws NoSuchProjectException {
-    Set<ConfigEntry> entries = parseConfiguration(cfgFactory, pluginName);
-
+    Set<ConfigEntry> entries = configParser.parseConfiguration();
     Set<ConfigEntry> filtered = new HashSet<>();
     for (ConfigEntry e : entries) {
       if (!checkRepoExists(e.srcRepoKey)) {
@@ -335,6 +278,10 @@
               relevantConfig.toString(), event.getRefName(), sw);
         }
       }
+    } catch (ConfigInvalidException e) {
+      error(
+          "Failed parsing supermanifest for project %s and ref %s: %s",
+          event.getProjectName(), event.getRefName(), e.getMessage());
     } catch (NoSuchProjectException e) {
       error(
           "Failed to do supermanifest updates for project %s and ref %s because the plugin could"
@@ -370,9 +317,16 @@
               allProjectsName));
     }
 
-    List<ConfigEntry> relevantConfigs =
-        findRelevantConfigs(
-            config, resource.getProjectState().getProject().getName(), resource.getRef());
+    List<ConfigEntry> relevantConfigs;
+    try {
+      relevantConfigs =
+          findRelevantConfigs(
+              config, resource.getProjectState().getProject().getName(), resource.getRef());
+    } catch (ConfigInvalidException e) {
+      error("manual trigger for %s:%s: %s", manifestProject, manifestBranch, e.getMessage());
+      throw new PreconditionFailedException("Invalid configuration");
+    }
+
     if (relevantConfigs.isEmpty()) {
       info(
           "manual trigger for %s:%s: no configs found, nothing to do.",
@@ -394,10 +348,24 @@
   }
 
   private List<ConfigEntry> findRelevantConfigs(
-      ImmutableSet<ConfigEntry> config, String project, String refName) {
-    return config.stream()
-        .filter(c -> c.matchesSource(project, refName))
-        .collect(Collectors.toList());
+      ImmutableSet<ConfigEntry> config, String project, String refName)
+      throws ConfigInvalidException {
+    List<ConfigEntry> relevantConfigs =
+        config.stream().filter(c -> c.matchesSource(project, refName)).collect(Collectors.toList());
+
+    // Don't write twice to same destination (no overlaps)
+    Map<String, ConfigEntry> destinations = new HashMap<>();
+    for (ConfigEntry configEntry : relevantConfigs) {
+      String key = configEntry.getDestRepoKey() + ":" + configEntry.getActualDestBranch(refName);
+      if (destinations.containsKey(key)) {
+        throw new ConfigInvalidException(
+            String.format(
+                "Configuration overlap %s:%s writes to %s twice (confs %s and %s)",
+                project, refName, key, configEntry, destinations.get(key)));
+      }
+      destinations.put(key, configEntry);
+    }
+    return relevantConfigs;
   }
 
   private void updateForConfig(ConfigEntry configEntry, String refName)
diff --git a/javatests/com/googlesource/gerrit/plugins/supermanifest/ConfigEntryTest.java b/javatests/com/googlesource/gerrit/plugins/supermanifest/ConfigEntryTest.java
index c3caec0..80193e6 100644
--- a/javatests/com/googlesource/gerrit/plugins/supermanifest/ConfigEntryTest.java
+++ b/javatests/com/googlesource/gerrit/plugins/supermanifest/ConfigEntryTest.java
@@ -14,6 +14,8 @@
 package com.googlesource.gerrit.plugins.supermanifest;
 
 import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+import static org.junit.Assert.fail;
 
 import com.google.gerrit.entities.Project;
 import org.eclipse.jgit.errors.ConfigInvalidException;
@@ -74,6 +76,89 @@
   }
 
   @Test
+  public void regexBranches_suffix_ok() throws ConfigInvalidException {
+    String conf =
+        getBasicConf(
+            "superproject", "refs/heads/nyc-*", "manifest", "refs/heads/nyc-src", "default.xml");
+
+    Config cfg = new Config();
+    cfg.fromText(conf);
+
+    ConfigEntry entry = new ConfigEntry(cfg, "superproject:refs/heads/nyc-*");
+    assertThat(entry.getDestRepoKey()).isEqualTo(Project.nameKey("superproject"));
+    assertThat(entry.getDestBranch()).isEqualTo("nyc-*");
+    assertThat(entry.getSrcRepoKey()).isEqualTo(Project.nameKey("manifest"));
+    assertThat(entry.getSrcRef()).isEqualTo(""); // Ignored
+    assertThat(entry.getXmlPath()).isEqualTo("default.xml");
+    assertThat(entry.matchesSource("manifest", "refs/heads/a")).isFalse();
+    assertThat(entry.matchesSource("manifest", "refs/heads/nyc-dev")).isTrue();
+    assertThat(entry.matchesSource("otherproject", "refs/heads/nyc-dev")).isFalse();
+    assertThat(entry.getActualDestBranch("refs/heads/nyc-dev")).isEqualTo("nyc-dev");
+  }
+
+  @Test
+  public void regexBranches_prefix_ok() throws ConfigInvalidException {
+    String conf =
+        getBasicConf(
+            "superproject",
+            "refs/heads/*-release",
+            "manifest",
+            "refs/heads/nyc-src",
+            "default.xml");
+
+    Config cfg = new Config();
+    cfg.fromText(conf);
+
+    ConfigEntry entry = new ConfigEntry(cfg, "superproject:refs/heads/*-release");
+    assertThat(entry.getDestRepoKey()).isEqualTo(Project.nameKey("superproject"));
+    assertThat(entry.getDestBranch()).isEqualTo("*-release");
+    assertThat(entry.getSrcRepoKey()).isEqualTo(Project.nameKey("manifest"));
+    assertThat(entry.getSrcRef()).isEqualTo(""); // Ignored
+    assertThat(entry.getXmlPath()).isEqualTo("default.xml");
+    assertThat(entry.matchesSource("manifest", "refs/heads/release")).isFalse();
+    assertThat(entry.matchesSource("manifest", "refs/heads/release-a")).isFalse();
+    assertThat(entry.matchesSource("manifest", "refs/heads/a-release")).isTrue();
+    assertThat(entry.matchesSource("manifest", "refs/heads/-release")).isFalse();
+    assertThat(entry.matchesSource("otherproject", "refs/heads/a-release")).isFalse();
+    assertThat(entry.getActualDestBranch("refs/heads/a-release")).isEqualTo("a-release");
+  }
+
+  @Test
+  public void regexBranches_moreThanOneStar_invalid() throws ConfigInvalidException {
+    String conf =
+        getBasicConf(
+            "superproject",
+            "refs/heads/nyc-*-with-*",
+            "manifest",
+            "refs/heads/nyc-src",
+            "default.xml");
+
+    Config cfg = new Config();
+    cfg.fromText(conf);
+
+    assertThrows(
+        ConfigInvalidException.class,
+        () -> new ConfigEntry(cfg, "superproject:refs/heads/nyc-*-with-*"));
+  }
+
+  @Test
+  public void regexBranches_noRefHeads_invalid() throws ConfigInvalidException {
+    String conf =
+        getBasicConf(
+            "superproject", "refs/tags/nyc-*", "manifest", "refs/heads/nyc-src", "default.xml");
+
+    Config cfg = new Config();
+    cfg.fromText(conf);
+
+    try {
+      ConfigEntry entry = new ConfigEntry(cfg, "superproject:refs/tags/nyc-*");
+      fail("Regex doesn't support more than one *");
+    } catch (ConfigInvalidException e) {
+      // Expected
+    }
+  }
+
+  @Test
   public void repoGroups() throws ConfigInvalidException {
     StringBuilder builder =
         new StringBuilder(
diff --git a/javatests/com/googlesource/gerrit/plugins/supermanifest/RepoSuperManifestIT.java b/javatests/com/googlesource/gerrit/plugins/supermanifest/RepoSuperManifestIT.java
index 67e7410..a0270fd 100644
--- a/javatests/com/googlesource/gerrit/plugins/supermanifest/RepoSuperManifestIT.java
+++ b/javatests/com/googlesource/gerrit/plugins/supermanifest/RepoSuperManifestIT.java
@@ -27,6 +27,7 @@
 import com.google.gerrit.acceptance.TestPlugin;
 import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
 import com.google.gerrit.entities.Project;
+import com.google.gerrit.entities.Project.NameKey;
 import com.google.gerrit.entities.RefNames;
 import com.google.gerrit.extensions.api.projects.BranchApi;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
@@ -489,6 +490,71 @@
   }
 
   @Test
+  public void regexDestBranchWorks() throws Exception {
+    setupTestRepos("project");
+
+    pushConfig(
+        "[superproject \""
+            + superKey.get()
+            + ":refs/heads/nyc-*\"]\n"
+            + "  srcRepo = "
+            + manifestKey.get()
+            + "\n"
+            + "  srcRef = blablabla\n"
+            + "  srcPath = default.xml\n");
+
+    String remoteXml = "  <remote name=\"origin\" fetch=\"" + canonicalWebUrl.get() + "\" />\n";
+    String originXml = "  <default remote=\"origin\" revision=\"refs/heads/master\" />\n";
+
+    // XML change will trigger commit to superproject.
+    String xml =
+        "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
+            + "<manifest>\n"
+            + remoteXml
+            + originXml
+            + "  <project name=\""
+            + testRepoKeys[0].get()
+            + "\" path=\"project1\" />\n"
+            + "</manifest>\n";
+
+    Result src1ManifestPush =
+        pushFactory
+            .create(admin.newIdent(), manifestRepo, "Subject", "default.xml", xml)
+            .to("refs/heads/nyc-1");
+    src1ManifestPush.assertOkStatus();
+
+    xml =
+        "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
+            + "<manifest>\n"
+            + remoteXml
+            + originXml
+            + "  <project name=\""
+            + testRepoKeys[1].get()
+            + "\" path=\"project2\" />\n"
+            + "</manifest>\n";
+
+    Result src2ManifestPush =
+        pushFactory
+            .create(admin.newIdent(), manifestRepo, "Subject", "default.xml", xml)
+            .to("refs/heads/nyc-2");
+    src2ManifestPush.assertOkStatus();
+
+    BranchApi branch1 = gApi.projects().name(superKey.get()).branch("refs/heads/nyc-1");
+    assertThat(branch1.file("project1").getContentType()).isEqualTo("x-git/gitlink; charset=UTF-8");
+    assertThrows(ResourceNotFoundException.class, () -> branch1.file("project2"));
+    assertThat(branch1.file(SUPERMANIFEST_STAMP).asString())
+        .isEqualTo(
+            manifestKey.get() + " refs/heads/nyc-1 " + src1ManifestPush.getCommit().getName());
+
+    BranchApi branch2 = gApi.projects().name(superKey.get()).branch("refs/heads/nyc-2");
+    assertThat(branch2.file("project2").getContentType()).isEqualTo("x-git/gitlink; charset=UTF-8");
+    assertThrows(ResourceNotFoundException.class, () -> branch2.file("project1"));
+    assertThat(branch2.file(SUPERMANIFEST_STAMP).asString())
+        .isEqualTo(
+            manifestKey.get() + " refs/heads/nyc-2 " + src2ManifestPush.getCommit().getName());
+  }
+
+  @Test
   public void manifestIncludesOtherManifest() throws Exception {
     setupTestRepos("project");
 
@@ -659,5 +725,110 @@
     assertThat(branch4.file(SUPERMANIFEST_STAMP).asString())
         .startsWith(manifestKey.get() + " refs/heads/src4");
   }
+
+  @Test
+  public void overlappingRefSpecDestination() throws Exception {
+    setupTestRepos("project");
+
+    // Configure supermanifest to same superproject for main-* and *-release.
+    // An update to main-x-release triggers an overlapping write
+    pushConfig(
+        "[superproject \""
+            + superKey.get()
+            + ":refs/heads/main-*\"]\n"
+            + "  srcRepo = "
+            + manifestKey.get()
+            + "\n"
+            + "  srcRef = blablabla\n"
+            + "  srcPath = default.xml\n"
+            + "\n"
+            + "[superproject \""
+            + superKey.get()
+            + ":refs/heads/*-release\"]\n"
+            + "  srcRepo = "
+            + manifestKey.get()
+            + "\n"
+            + "  srcRef = blablabla\n"
+            + "  srcPath = default.xml\n");
+
+    String remoteXml = "  <remote name=\"origin\" fetch=\"" + canonicalWebUrl.get() + "\" />\n";
+    String originXml = "  <default remote=\"origin\" revision=\"refs/heads/master\" />\n";
+
+    String xml =
+        "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
+            + "<manifest>\n"
+            + remoteXml
+            + originXml
+            + "  <project name=\""
+            + testRepoKeys[0].get()
+            + "\" path=\"project1\" />\n"
+            + "</manifest>\n";
+
+    // This fails but silently. Check there is no destination written.
+    pushFactory
+        .create(admin.newIdent(), manifestRepo, "Subject", "default.xml", xml)
+        .to("refs/heads/main-x-release")
+        .assertOkStatus();
+
+    // This branch should not exist
+    BranchApi branch2 = gApi.projects().name(superKey.get()).branch("refs/heads/main-x-release");
+    assertThrows(ResourceNotFoundException.class, () -> branch2.file(".gitmodules"));
+
+    // Using update_manifest
+    RestResponse r =
+        adminRestSession.post(
+            "/projects/" + manifestKey + "/branches/main-x-release/update_manifest");
+    r.assertPreconditionFailed();
+  }
+
+  @Test
+  public void globsFromDifferentSources() throws Exception {
+    setupTestRepos("project");
+
+    NameKey manifest2Key = projectOperations.newProject().name(name("manifest2")).create();
+    TestRepository<InMemoryRepository> manifest2Repo = cloneProject(manifest2Key, admin);
+
+    pushConfig(
+        "[superproject \""
+            + superKey.get()
+            + ":refs/heads/main-*\"]\n"
+            + "  srcRepo = "
+            + manifestKey.get()
+            + "\n"
+            + "  srcRef = blablabla\n"
+            + "  srcPath = default.xml\n"
+            + "\n"
+            + "[superproject \""
+            + superKey.get()
+            + ":refs/heads/*-release\"]\n"
+            + "  srcRepo = "
+            + manifest2Key.get()
+            + "\n"
+            + "  srcRef = blablabla\n"
+            + "  srcPath = default.xml\n");
+
+    String remoteXml = "  <remote name=\"origin\" fetch=\"" + canonicalWebUrl.get() + "\" />\n";
+    String originXml = "  <default remote=\"origin\" revision=\"refs/heads/master\" />\n";
+
+    String xml =
+        "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
+            + "<manifest>\n"
+            + remoteXml
+            + originXml
+            + "  <project name=\""
+            + testRepoKeys[0].get()
+            + "\" path=\"project1\" />\n"
+            + "</manifest>\n";
+
+    pushFactory
+        .create(admin.newIdent(), manifest2Repo, "Subject", "default.xml", xml)
+        .to("refs/heads/main-x-release")
+        .assertOkStatus();
+
+    // The conf from manifest2 is ignored, so the push above should not create this branch
+    BranchApi secondConfBranch =
+        gApi.projects().name(superKey.get()).branch("refs/heads/main-x-release");
+    assertThrows(ResourceNotFoundException.class, () -> secondConfBranch.file(".gitmodules"));
+  }
   // TODO - should add tests for all the error handling in configuration parsing?
 }