RFR: 1691: Run Jcheck twice in CheckRun if .jcheck/conf has changed [v3]

[Erik Joelsson]

On Thu, 8 Dec 2022 18:13:39 GMT, Zhao Song <zsong at openjdk.org> wrote:

>> When a pr is created, pr bot will run jcheck to check whether this commit is good. However, currently, we only run jcheck with the configuration from the target branch. Sometimes, user wants to modify jcheck configuration and accidentally messes up the format. In this case, pr bot would not be able to find this problem. After this commit with badly formatted jcheck configuration integrated, all the jcheck will fail and our bot gets stuck into a retry loop.
>> 
>> In this patch, if jcheck configuration file is updated, the bot will run jcheck twice, once with the configuration in the target and once with the new one in the source. So if the new configuration has any problem, it will became an integration blocker.
>
> Zhao Song has updated the pull request incrementally with one additional commit since the last revision:
> 
>   fix problems

bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckRun.java line 1202:

> 1200: 
> 1201:     private boolean isFileUpdated(String filename, Hash hash) throws IOException {
> 1202:         return localRepo.commits(hash.hex()).asList().stream()

Suggestion:

        return localRepo.commits(hash.hex()).stream()

bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckRun.java line 1205:

> 1203:                 .anyMatch(commit -> commit.parentDiffs().stream()
> 1204:                         .anyMatch(diff -> diff.patches().stream()
> 1205:                                 .anyMatch(patch -> patch.toString().contains(filename))));

Suggestion:

                                .anyMatch(patch -> patch.source().path().toString().equals(filename)
                                    || patch.target().path().toString().equals(filename))));

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

> 166:         var localRepo = hostedRepositoryPool.checkout(pr.repository(), pr.headHash().hex(), path);
> 167:         localRepo.fetch(pr.repository().url(), "+" + pr.targetRef() + ":prutils_targetref", false);
> 168:         localRepo.fetch(pr.repository().url(), "+" + pr.sourceRef() + ":prutils_sourceref", false);

I'm curious as to why this is needed. Line 166 should make sure we are fetching the source/head of the pr. I don't see any use of the local branch `prutils_sourceref`.

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

PR: https://git.openjdk.org/skara/pull/1439