RFR: 2454: Skara mistakenly marked a non-clean backport as clean [v3]
Zhao Song
zsong at openjdk.org
Wed Mar 12 22:42:05 UTC 2025
On Wed, 12 Mar 2025 22:38:57 GMT, Zhao Song <zsong at openjdk.org> wrote:
>> A user reported that Skara bot mistakenly marked a non-clean backport as clean. After investigation, I found that the PR was too large for the remote host to return the entire diff. The PR contained 8082 changed files, but when Skara bot requested the diff, only 3000 files were returned. Therefore, Skara bot was unable to compare some patches, so mistakenly mark the pr as clean.
>>
>> In this patch, Skara bot will now check if the PR is too large to evaluate. If it is, Skara bot will not evaluate if this PR is clean and will add a warning comment to the PR instead.
>
> Zhao Song has updated the pull request incrementally with two additional commits since the last revision:
>
> - fix copyright
> - update
forge/src/main/java/org/openjdk/skara/forge/gitlab/GitLabMergeRequest.java line 834:
> 832: var changes = request.get("changes").param("access_raw_diffs", "true").execute();
> 833: boolean diffComplete;
> 834: if (changes.get("overflow").asBoolean()) {
During my experiment, I found we can't fully trust the "overflow" field returned by GitLab.
forge/src/main/java/org/openjdk/skara/forge/gitlab/GitLabMergeRequest.java line 837:
> 835: diffComplete = false;
> 836: } else {
> 837: diffComplete = !changes.get("changes_count").asString().contains("+");
When overflow happens, GitLab returns the changes_count as "number+".
forge/src/main/java/org/openjdk/skara/forge/gitlab/GitLabRepository.java line 758:
> 756: // The diff here is only used to evaluate if a backport PR is clean or not,
> 757: // assuming the diff is complete will not introduce any side effect.
> 758: diffs = List.of(toDiff(metadata.parents().get(0), hash, diff, true));
I thought the diff is only limited by the count of files. But I was wrong.
The diff is limited by 3 factors. So it's hard for us to know if it's limited or not.
https://docs.gitlab.com/administration/diff_limits/#configure-diff-limits
-------------
PR Review Comment: https://git.openjdk.org/skara/pull/1708#discussion_r1992379177
PR Review Comment: https://git.openjdk.org/skara/pull/1708#discussion_r1992381095
PR Review Comment: https://git.openjdk.org/skara/pull/1708#discussion_r1992383526
More information about the skara-dev
mailing list