Skip to content
Permalink
Browse files
344: PR should warn when source branch is an upstream branch
Reviewed-by: rwestberg
  • Loading branch information
edvbld committed Mar 31, 2020
1 parent 32bba61 commit c231092fdabbb41f39347495e29f048216ff81f7
Showing 7 changed files with 147 additions and 7 deletions.
@@ -51,6 +51,7 @@ class CheckRun {
private final String progressMarker = "<!-- Anything below this marker will be automatically updated, please do not edit manually! -->";
private final String mergeReadyMarker = "<!-- PullRequestBot merge is ready comment -->";
private final String outdatedHelpMarker = "<!-- PullRequestBot outdated help comment -->";
private final String sourceBranchWarningMarker = "<!-- PullRequestBot source branch warning comment -->";
private final Pattern mergeSourceFullPattern = Pattern.compile("^Merge ([-/\\w]+):([-\\w]+)$");
private final Pattern mergeSourceBranchOnlyPattern = Pattern.compile("^Merge ([-\\w]+)$");
private final Set<String> newLabels;
@@ -83,13 +84,14 @@ private boolean isTargetBranchAllowed() {
}

private List<String> allowedTargetBranches() {
var remoteBranches = prInstance.remoteBranches().stream()
.map(Reference::name)
.map(name -> workItem.bot.allowedTargetBranches().matcher(name))
.filter(Matcher::matches)
.map(Matcher::group)
.collect(Collectors.toList());
return remoteBranches;
return pr.repository()
.branches()
.stream()
.map(HostedBranch::name)
.map(name -> workItem.bot.allowedTargetBranches().matcher(name))
.filter(Matcher::matches)
.map(Matcher::group)
.collect(Collectors.toList());
}

// For unknown contributors, check that all commits have the same name and email
@@ -575,6 +577,37 @@ private void updateMergeReadyComment(boolean isReady, String commitMessage, List
}
}

private void addSourceBranchWarningComment(List<Comment> comments) {
var existing = findComment(comments, sourceBranchWarningMarker);
if (existing.isPresent()) {
// Only add the comment once per PR
return;
}
var branch = pr.sourceRef();
var message = ":warning: @" + pr.author().userName() + " " +
"a branch with the same name as the source branch for this pull request (`" + branch + "`) " +
"is present in the [target repository](" + pr.repository().nonTransformedWebUrl() + "). " +
"If you eventually integrate this pull request then the branch `" + branch + "` " +
"in your [personal fork](" + pr.sourceRepository().nonTransformedWebUrl() + ") will diverge once you sync " +
"your personal fork with the upstream repository.\n" +
"\n" +
"To avoid this situation, create a new branch for your changes and reset the `" + branch + "` branch. " +
"You can do this by running the following commands in a local repository for your personal fork. " +
"_Note_: you do *not* have to name the new branch `NEW-BRANCH-NAME`." +
"\n" +
"```" +
"$ git checkout " + branch + "\n" +
"$ git checkout -b NEW-BRANCH-NAME\n" +
"$ git branch -f " + branch + " " + prInstance.baseHash().hex() + "\n" +
"$ git push -f origin " + branch + "\n" +
"```\n" +
"\n" +
"Then proceed to create a new pull request with `NEW-BRANCH-NAME` as the source branch and " +
"close this one.\n" +
sourceBranchWarningMarker;
pr.addComment(message);
}

private void addOutdatedComment(List<Comment> comments) {
var existing = findComment(comments, outdatedHelpMarker);
if (existing.isPresent()) {
@@ -662,6 +695,11 @@ private void checkStatus() {
newLabels.remove("merge-conflict");
}

var branchNames = pr.repository().branches().stream().map(HostedBranch::name).collect(Collectors.toSet());
if (!pr.repository().url().equals(pr.sourceRepository().url()) && branchNames.contains(pr.sourceRef())) {
addSourceBranchWarningComment(comments);
}

// Ensure that the ready for sponsor label is up to date
newLabels.remove("sponsor");
var readyHash = ReadyForSponsorTracker.latestReadyForSponsor(pr.repository().forge().currentUser(), comments);
@@ -140,4 +140,9 @@ public Hash branchHash(String ref) {
public List<PullRequest> findPullRequestsWithComment(String author, String body) {
return null;
}

@Override
public List<HostedBranch> branches() {
return List.of();
}
}
@@ -0,0 +1,65 @@
/*
* Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
package org.openjdk.skara.forge;

import org.openjdk.skara.vcs.Hash;

import java.util.Objects;

public class HostedBranch {
private final String name;
private final Hash hash;

public HostedBranch(String name, Hash hash) {
this.name = name;
this.hash = hash;
}

public String name() {
return name;
}

public Hash hash() {
return hash;
}

@Override
public String toString() {
return name + "@" + hash.hex();
}

@Override
public int hashCode() {
return Objects.hash(name, hash);
}

@Override
public boolean equals(Object other) {
if (!(other instanceof HostedBranch)) {
return false;
}

var o = (HostedBranch) other;
return Objects.equals(name, o.name) && Objects.equals(hash, o.hash);
}
}
@@ -65,6 +65,7 @@ PullRequest createPullRequest(HostedRepository target,
HostedRepository fork();
long id();
Hash branchHash(String ref);
List<HostedBranch> branches();

default PullRequest createPullRequest(HostedRepository target,
String targetRef,
@@ -242,4 +242,13 @@ public Hash branchHash(String ref) {
var branch = request.get("branches/" + ref).execute();
return new Hash(branch.get("commit").get("sha").asString());
}

@Override
public List<HostedBranch> branches() {
var branches = request.get("branches").execute();
return branches.stream()
.map(b -> new HostedBranch(b.get("name").asString(),
new Hash(b.get("commit").get("sha").asString())))
.collect(Collectors.toList());
}
}
@@ -264,4 +264,13 @@ public Hash branchHash(String ref) {
var branch = request.get("repository/branches/" + ref).execute();
return new Hash(branch.get("commit").get("id").asString());
}

@Override
public List<HostedBranch> branches() {
var branches = request.get("branches").execute();
return branches.stream()
.map(b -> new HostedBranch(b.get("name").asString(),
new Hash(b.get("commit").get("id").asString())))
.collect(Collectors.toList());
}
}
@@ -185,6 +185,19 @@ public Hash branchHash(String ref) {
}
}

@Override
public List<HostedBranch> branches() {
try {
var result = new ArrayList<HostedBranch>();
for (var b : localRepository.branches()) {
result.add(new HostedBranch(b.name(), localRepository.resolve(b).orElseThrow()));
}
return result;
} catch (IOException e) {
throw new RuntimeException(e);
}
}

Repository localRepository() {
return localRepository;
}

0 comments on commit c231092

Please sign in to comment.