From 856792f93ef48454c154a728000fe01e2b27f5fd Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Sat, 31 May 2025 10:56:29 +0200 Subject: [PATCH] workflows/check-cherry-picks: truncate long diffs after 10k characters GitHub comments have a length limit, so we can't just dump everything. The 10k limit is arbitrary, but the assumption is that reviewing the range-diff is not the sensible thing to do once it becomes a certain size - reviewing the regular diff and treating the commit as "new" is easier to do in that case. Thus, truncating should work out fine, especially when the full range-diff is still available in the runner log. This could still end up in with an error, if a PR has multiple commits, which all hit the limit. Let's get there first, before we try to fix that hypothetical case, too. --- .github/workflows/check-cherry-picks.yml | 5 ++++- ci/check-cherry-picks.sh | 13 ++++++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/.github/workflows/check-cherry-picks.yml b/.github/workflows/check-cherry-picks.yml index b44027e326d2..84ed5604443d 100644 --- a/.github/workflows/check-cherry-picks.yml +++ b/.github/workflows/check-cherry-picks.yml @@ -49,7 +49,7 @@ jobs: const header = await readFile('trusted/ci/check-cherry-picks.md') const body = await readFile('checked-cherry-picks.md') const footer = - `\n_Hint: The diffs are also available in the [runner logs](${job_url}) with slightly better highlighting._` + `\n_Hint: The full diffs are also available in the [runner logs](${job_url}) with slightly better highlighting._` const review = header + body + footer await writeFile('review.md', review) @@ -79,6 +79,9 @@ jobs: ) ) + // Either of those two requests could fail for very long comments. This can only happen + // with multiple commits all hitting the truncation limit for the diff. If you ever hit + // this case, consider just splitting up those commits into multiple PRs. if (pendingReview) { await github.rest.pulls.updateReview({ owner: context.repo.owner, diff --git a/ci/check-cherry-picks.sh b/ci/check-cherry-picks.sh index e0b278207875..6036d5a1529a 100755 --- a/ci/check-cherry-picks.sh +++ b/ci/check-cherry-picks.sh @@ -112,7 +112,18 @@ while read -r new_commit_sha ; do echo '> ```diff' >> $markdown_file # The output of `git range-diff` is indented with 4 spaces, which we need to match with the # code blocks indent to get proper syntax highlighting on GitHub. - $range_diff_common | tail -n +2 | sed -Ee 's/^ {4}/> /g' >> $markdown_file + diff="$($range_diff_common | tail -n +2 | sed -Ee 's/^ {4}/> /g')" + # Also limit the output to 10k bytes (and remove the last, potentially incomplete line), because + # GitHub comments are limited in length. The value of 10k is arbitrary with the assumption, that + # after the range-diff becomes a certain size, a reviewer is better off reviewing the regular diff + # in GitHub's UI anyway, thus treating the commit as "new" and not cherry-picked. + # Note: This could still lead to a too lengthy comment with multiple commits touching the limit. We + # consider this too unlikely to happen, to deal with explicitly. + max_length=10000 + if [ "${#diff}" -gt $max_length ]; then + printf -v diff "%s\n\n[...truncated...]" "$(echo "$diff" | head -c $max_length | head -n-1)" + fi + echo "$diff" >> $markdown_file echo '> ```' >> $markdown_file echo "> " >> $markdown_file