[github_pr_destination] Don't require context reference if pr_branch is set

java/com/google/copybara/git/GitHubPrDestination.java

@@ -333,44 +333,68 @@ public Iterable<GitIntegrateChanges> getIntegrates() {
   private String getPullRequestBranchName(
       @Nullable Revision changeRevision, String workflowName, String workflowIdentityUser)
       throws ValidationException {
+    String resolvedPrBranchName;
     if (!Strings.isNullOrEmpty(gitHubDestinationOptions.destinationPrBranch)) {
-      return gitHubDestinationOptions.destinationPrBranch;
+      // command line provided value has highest priority
+      resolvedPrBranchName = gitHubDestinationOptions.destinationPrBranch;
+    } else if (prBranch != null) {
+      // next, the starlark provided value, with resolved labels.
+      resolvedPrBranchName = resolvePrBranchLabels(prBranch, changeRevision);
+    } else {
+      // finally, attempt a sane branch name from the workflow
+      // change revision and context are required to compute a pr branch name.
+      if (changeRevision == null || changeRevision.contextReference() == null) {
+        throw new ValidationException(MISSING_CONTEXT_REFERENCE_MESSAGE);
+      }
+      resolvedPrBranchName =
+          Identity.computeIdentity(
+              "OriginGroupIdentity",
+              changeRevision.contextReference(),
+              workflowName,
+              mainConfigFile.getIdentifier(),
+              workflowIdentityUser);
     }
-    String contextReference = changeRevision.contextReference();
-    // We could do more magic here with the change identity. But this is already complex so we
-    // require  a group identity either provided by the origin or the workflow (Will be implemented
-    // later.
-    checkCondition(contextReference != null,
-        "git.github_pr_destination is incompatible with the current origin. Origin has to be"
-            + " able to provide the contextReference or use '%s' flag",
-        GitHubDestinationOptions.GITHUB_DESTINATION_PR_BRANCH);
-    String branchNameFromUser = getCustomBranchName(contextReference);
-    String branchName =
-        branchNameFromUser != null
-            ? branchNameFromUser
-            : Identity.computeIdentity(
-                "OriginGroupIdentity",
-                contextReference,
-                workflowName,
-                mainConfigFile.getIdentifier(),
-                workflowIdentityUser);
-    return GitHubUtil.getValidBranchName(branchName);
+    return GitHubUtil.getValidBranchName(resolvedPrBranchName);
   }
 
-  @Nullable
-  private String getCustomBranchName(String contextReference) throws ValidationException {
-    if (prBranch == null) {
-      return null;
-    }
+  private static String resolvePrBranchLabels(String prBranch, Revision changeRevision)
+      throws ValidationException {
     try {
       return new LabelTemplate(prBranch)
-          .resolve(e -> e.equals("CONTEXT_REFERENCE") ? contextReference : prBranch);
+          .resolve(
+              e -> {
+                // no labels can be resolved without a revision.
+                if (changeRevision == null) {
+                  return null;
+                }
+                // associatedLabels may contain CONTEXT_REFERENCE
+                if (e.equals("CONTEXT_REFERENCE")) {
+                  return changeRevision.contextReference();
+                }
+                // no other labels are supported, according to the docs.
+                return null;
+              });
     } catch (LabelNotFoundException e) {
+      if ("CONTEXT_REFERENCE".equals(e.getLabel())) {
+        // contextReference was needed to resolve the branch.
+        // This lazily validates the context reference, allowing support for the
+        // case where the Revision is missing or lacks context, but the workflow
+        // does not ask for it.
+        // See GitRepository.fetchSingleRefWithTags for cases where this is true.
+        throw new ValidationException(MISSING_CONTEXT_REFERENCE_MESSAGE);
+      }
+
       throw new ValidationException(
           "Cannot find some labels in the GitHub PR branch name field: " + e.getMessage(), e);
     }
   }
 
+  @VisibleForTesting
+  static final String MISSING_CONTEXT_REFERENCE_MESSAGE =  String.format(
+      "git.github_pr_destination is incompatible with the current origin."
+          + "Origin has to be able to provide the contextReference or use '%s' flag",
+      GitHubDestinationOptions.GITHUB_DESTINATION_PR_BRANCH);
+
   @Override
   public String getLabelNameWhenOrigin() {
     return GitRepository.GIT_ORIGIN_REV_ID;
@@ -400,4 +424,4 @@ private static class GitHubWriterState extends WriterState {
     }
     return resolvedDestinationRef;
   }
-}
+}

javatests/com/google/copybara/git/GitHubPrDestinationTest.java

@@ -128,10 +128,7 @@ public void testWrite_noContextReference() throws ValidationException {
         assertThrows(ValidationException.class, () -> d.newWriter(writerContext));
     assertThat(thrown)
         .hasMessageThat()
-        .contains(
-            "git.github_pr_destination is incompatible with the current origin."
-                + " Origin has to be able to provide the contextReference or use"
-                + " '--github-destination-pr-branch' flag");
+        .contains(GitHubPrDestination.MISSING_CONTEXT_REFERENCE_MESSAGE);
   }
 
   @Test
@@ -301,8 +298,42 @@ public void testWrite_destinationPrBranchFlag()
   }
 
   @Test
-  public void testTrimMessageForPrTitle()
+  public void testWrite_destinationPrBranchHasMissingLabel() {
+    assertThat(
+    assertThrows(
+        ValidationException.class,
+        () -> testBranchNameFromUser(
+            "copybara/import_foo_${MISSING}",
+            null,
+            null))).hasMessageThat().contains("MISSING");
+  }
+
+  @Test
+  public void testWrite_destinationPrBranchNoContextReferenceRequired()
       throws ValidationException, IOException, RepoException {
+    testBranchNameFromUser(
+        "copybara/import_foo",
+        "copybara/import_foo",
+        null
+    );
+  }
+
+  @Test
+  public void testWrite_destinationPrBranchContextExplicitlyReferenceRequired() {
+    assertThat(
+            assertThrows(
+                ValidationException.class,
+                () ->
+                    testBranchNameFromUser(
+                        "copybara/import_foo_${CONTEXT_REFERENCE}",
+                        null,
+                        null)))
+        .hasMessageThat()
+        .contains(GitHubPrDestination.MISSING_CONTEXT_REFERENCE_MESSAGE);
+  }
+
+  @Test
+  public void testTrimMessageForPrTitle() throws ValidationException, IOException, RepoException {
     options.githubDestination.destinationPrBranch = "feature";
 
     mockNoPullRequestsGet("feature");