RFR: 1424: CheckRun fails to evaluate jol#24

[Erik Joelsson]

On Tue, 3 May 2022 18:34:39 GMT, Erik Joelsson <erikj at openjdk.org> wrote:

> CheckRun, which is part of the PullRequestBot, is currently failing to evaluate https://github.com/openjdk/jol/pull/24. The failure happens in PullRequestUtils::containsForeignMerge where we are looking for merges in the PR branch which bring in commits that aren't descendants of the main merge parent. If this test is positive, the bot adds a comment with a warning.
> 
> The problem is that GitRepository::mergeBase doesn't differentiate between the git command failing with an error and when it just can't find a merge-base. For this check, a non existent merge-base should qualify for a positive test.
> 
> I'm refactoring Repository::mergeBase into two methods, one that returns an Optional, which will be empty when a merge-base can't be found. I've also gone through all callers to identify when the new method should be used to properly tell this situation apart.

forge/src/main/java/org/openjdk/skara/forge/PullRequestUtils.java line 132:

> 130:         // Ensure that the source and the target are related
> 131:         localRepo.mergeBaseOptional(targetHash(localRepo), sourceHead)
> 132:                 .orElseThrow(() -> new CommitFailure("The target and the source branches do not share common history - cannot merge them."));

Here I think a real error should be let through so the whole WorkItem may be retried later.

vcs/src/main/java/org/openjdk/skara/vcs/git/GitRepository.java line 924:

> 922:     public Hash mergeBase(Hash first, Hash second) throws IOException {
> 923:         try (var p = capture("git", "merge-base", first.hex(), second.hex())) {
> 924:             var res = await(p);

await(p) basically does p.await() and then throws IOException if res.status() is non zero.

-------------

PR: https://git.openjdk.java.net/skara/pull/1312