From 669bf8f73ca535a5ceaedb41d579c3ab8e49eb25 Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Thu, 20 Feb 2025 13:15:59 +0100 Subject: [PATCH 01/16] Prioritise TLDR part over the tree diff --- bin/comment_with_dependency_diff | 138 +++++++++++++------------------ 1 file changed, 58 insertions(+), 80 deletions(-) diff --git a/bin/comment_with_dependency_diff b/bin/comment_with_dependency_diff index fe5b5b32..1cbba82f 100755 --- a/bin/comment_with_dependency_diff +++ b/bin/comment_with_dependency_diff @@ -121,90 +121,68 @@ java -jar "$JAR_DIR/dependency-diff-tldr-0.0.6.jar" $BASE_BRANCH_DEPENDENCIES_FI ./dependency-tree-diff.jar $BASE_BRANCH_BUILD_ENV_FILE $HEAD_BRANCH_BUILD_ENV_FILE >$DIFF_BUILD_ENV_FILE java -jar "$JAR_DIR/dependency-diff-tldr-0.0.6.jar" $BASE_BRANCH_BUILD_ENV_FILE $HEAD_BRANCH_BUILD_ENV_FILE >$TLDR_DIFF_BUILD_ENV_FILE -generate_project_dependencies_comment_body() { - cat < Preparing comment for GitHub" -The following changes in project dependencies were detected (configuration \`$CONFIGURATION\`): - -
list - -\`\`\` -$(cat "$TLDR_DIFF_DEPENDENCIES_FILE") -\`\`\` - -
- -
tree - -\`\`\`diff -$(cat "$DIFF_DEPENDENCIES_FILE") -\`\`\` - -
- - -EOF -} - -generate_build_dependencies_comment_body() { - cat <list - -\`\`\` -$(cat "$TLDR_DIFF_BUILD_ENV_FILE") -\`\`\` - - - -
tree - -\`\`\`diff -$(cat "$DIFF_BUILD_ENV_FILE") -\`\`\` +# Check if any dependency changes were found +if [ ! -s "$TLDR_DIFF_DEPENDENCIES_FILE" ] && [ ! -s "$TLDR_DIFF_BUILD_ENV_FILE" ]; then + echo "No dependency changes found" + comment_on_pr --id "dependency-diff" --if-exist delete + exit 0 +fi -
+COMMENT_FILE="$DIFF_DEPENDENCIES_FOLDER/comment_body.txt" +true > $COMMENT_FILE # Clear the file +MAX_COMMENT_SIZE=65400 +current_size=0 +WARNING_MESSAGE="Content exceeds $MAX_COMMENT_SIZE characters. [Navigate to Buildkite build artifacts](${BUILDKITE_BUILD_URL}#${BUILDKITE_JOB_ID}) to see details." + +# Templates for content formatting +SECTION_HEADER="\n## %s changes\n\n" +TLDR_TEMPLATE="
list\n\n\`\`\`\n%s\n\`\`\`\n\n
" +TREE_TEMPLATE="
tree\n\n\`\`\`diff\n%s\n\`\`\`\n\n
" +TREE_TOO_LARGE="\n> ⚠️ %s tree is too large. [View it in Buildkite artifacts](${BUILDKITE_BUILD_URL}#${BUILDKITE_JOB_ID})\n" + +# Calculate total size needed for TLDRs +# This assumes that TLDRs are never too large to fit in the comment +tldr_size=0 +for section in "Project dependencies" "Build environment"; do + tldr_file="$TLDR_DIFF_DEPENDENCIES_FILE" + [[ $section == "Build environment" ]] && tldr_file="$TLDR_DIFF_BUILD_ENV_FILE" + + if [ -s "$tldr_file" ]; then + content=$(printf "$SECTION_HEADER$TLDR_TEMPLATE" "$section" "$(cat "$tldr_file")") + tldr_size=$(( tldr_size + ${#content} )) + fi +done -EOF +# Function to check if content would fit +would_content_fit() { + return $(($1 + tldr_size < MAX_COMMENT_SIZE )) } -if [ -n "$DIFF_DEPENDENCIES_FILE" ] || [ -n "$DIFF_BUILD_ENV_FILE" ]; then - echo "--> Commenting result to GitHub" - - # Create or clear the comment body file - COMMENT_FILE="$DIFF_DEPENDENCIES_FOLDER/comment_body.txt" - true > $COMMENT_FILE # 'true' used to clear the file as a no-op - - append_content() { - local header="$1" - local content="$2" - local github_comment_max_size=65400 # GitHub constraint is 65536, but we decrease this value slightly to reserve for hidden comment_on_pr content and line breaks - local warning_message="Content exceeds $github_comment_max_size characters. [Navigate to Buildkite build artifacts](${BUILDKITE_BUILD_URL}#${BUILDKITE_JOB_ID}) to see details." - - local current_size - current_size=$(wc -c < "$COMMENT_FILE") - - # Calculate if there's enough space for the content and header - if (( current_size + ${#header} + ${#content} > github_comment_max_size - ${#header} - ${#warning_message})); then - printf "\n%s\n%s\n" "$header" "$warning_message" > $COMMENT_FILE - return +# Print content in order +for section in "Project dependencies" "Build environment"; do + tldr_file="$TLDR_DIFF_DEPENDENCIES_FILE" + tree_file="$DIFF_DEPENDENCIES_FILE" + [[ $section == "Build environment" ]] && tldr_file="$TLDR_DIFF_BUILD_ENV_FILE" && tree_file="$DIFF_BUILD_ENV_FILE" + + if [ -s "$tldr_file" ]; then + # Print section header and TLDR + printf "$SECTION_HEADER" "$section" >> $COMMENT_FILE + printf "$TLDR_TEMPLATE" "$(cat "$tldr_file")" >> $COMMENT_FILE + current_size=$(wc -c < "$COMMENT_FILE") + + # Try to add tree if it exists + if [ -s "$tree_file" ]; then + content=$(printf "$TREE_TEMPLATE" "$(cat "$tree_file")") + if would_content_fit ${#content}; then + echo -e "$content" >> $COMMENT_FILE + current_size=$(wc -c < "$COMMENT_FILE") + else + printf "$TREE_TOO_LARGE" "$section" >> $COMMENT_FILE + fi + fi fi +done - printf "\n%s\n" "$header" >> $COMMENT_FILE - printf "%s\n" "$content" >> $COMMENT_FILE - } - - if [ -s "$DIFF_DEPENDENCIES_FILE" ]; then - append_content "## Project dependencies changes" "$(generate_project_dependencies_comment_body)" - fi - - if [ -s "$DIFF_BUILD_ENV_FILE" ]; then - append_content "## Build environment changes" "$(generate_build_dependencies_comment_body)" - fi - - comment_on_pr --id "dependency-diff" --if-exist update "$COMMENT_FILE" -else - comment_on_pr --id "dependency-diff" --if-exist delete -fi +comment_on_pr --id "dependency-diff" --if-exist update "$COMMENT_FILE" From f97a4b7fde88f0b39e000d51e4ac1db9c445dce2 Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Thu, 20 Feb 2025 15:14:21 +0100 Subject: [PATCH 02/16] add debug information --- bin/comment_with_dependency_diff | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/bin/comment_with_dependency_diff b/bin/comment_with_dependency_diff index 1cbba82f..c3f0ca8d 100755 --- a/bin/comment_with_dependency_diff +++ b/bin/comment_with_dependency_diff @@ -134,13 +134,12 @@ COMMENT_FILE="$DIFF_DEPENDENCIES_FOLDER/comment_body.txt" true > $COMMENT_FILE # Clear the file MAX_COMMENT_SIZE=65400 current_size=0 -WARNING_MESSAGE="Content exceeds $MAX_COMMENT_SIZE characters. [Navigate to Buildkite build artifacts](${BUILDKITE_BUILD_URL}#${BUILDKITE_JOB_ID}) to see details." # Templates for content formatting SECTION_HEADER="\n## %s changes\n\n" TLDR_TEMPLATE="
list\n\n\`\`\`\n%s\n\`\`\`\n\n
" TREE_TEMPLATE="
tree\n\n\`\`\`diff\n%s\n\`\`\`\n\n
" -TREE_TOO_LARGE="\n> ⚠️ %s tree is too large. [View it in Buildkite artifacts](${BUILDKITE_BUILD_URL}#${BUILDKITE_JOB_ID})\n" +TREE_TOO_LARGE="\n ⚠️ %s tree is too large. [View it in Buildkite artifacts](${BUILDKITE_BUILD_URL}#${BUILDKITE_JOB_ID})\n" # Calculate total size needed for TLDRs # This assumes that TLDRs are never too large to fit in the comment @@ -157,7 +156,10 @@ done # Function to check if content would fit would_content_fit() { - return $(($1 + tldr_size < MAX_COMMENT_SIZE )) + echo "Debug: New content size: $1" + echo "Debug: TLDR total size: $tldr_size" + echo "Debug: Max size: $MAX_COMMENT_SIZE" + return $(( $1 + tldr_size < MAX_COMMENT_SIZE )) } # Print content in order From bd653549fc0c34c5deb1bcb5dc5ad4b2f53f4edd Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Thu, 20 Feb 2025 15:24:36 +0100 Subject: [PATCH 03/16] Add more debug logs --- bin/comment_with_dependency_diff | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/bin/comment_with_dependency_diff b/bin/comment_with_dependency_diff index c3f0ca8d..22f1a4ce 100755 --- a/bin/comment_with_dependency_diff +++ b/bin/comment_with_dependency_diff @@ -158,8 +158,9 @@ done would_content_fit() { echo "Debug: New content size: $1" echo "Debug: TLDR total size: $tldr_size" + echo "Debug: Current size: $current_size" echo "Debug: Max size: $MAX_COMMENT_SIZE" - return $(( $1 + tldr_size < MAX_COMMENT_SIZE )) + return $(( current_size + $1 < MAX_COMMENT_SIZE )) } # Print content in order @@ -176,7 +177,10 @@ for section in "Project dependencies" "Build environment"; do # Try to add tree if it exists if [ -s "$tree_file" ]; then - content=$(printf "$TREE_TEMPLATE" "$(cat "$tree_file")") + tree_content=$(cat "$tree_file") + echo "Debug: Raw tree size: ${#tree_content}" + content=$(printf "$TREE_TEMPLATE" "$tree_content") + echo "Debug: Formatted tree size: ${#content}" if would_content_fit ${#content}; then echo -e "$content" >> $COMMENT_FILE current_size=$(wc -c < "$COMMENT_FILE") From c174d5bfcb9e4fe17ed5733ab7cedda6c22bd0a0 Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Thu, 20 Feb 2025 18:18:59 +0100 Subject: [PATCH 04/16] invert the would_content_fit condition --- bin/comment_with_dependency_diff | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/bin/comment_with_dependency_diff b/bin/comment_with_dependency_diff index 22f1a4ce..a1cd7fd5 100755 --- a/bin/comment_with_dependency_diff +++ b/bin/comment_with_dependency_diff @@ -160,7 +160,11 @@ would_content_fit() { echo "Debug: TLDR total size: $tldr_size" echo "Debug: Current size: $current_size" echo "Debug: Max size: $MAX_COMMENT_SIZE" - return $(( current_size + $1 < MAX_COMMENT_SIZE )) + if (( current_size + $1 < MAX_COMMENT_SIZE )); then + return 0 # Success/true - content will fit + else + return 1 # Failure/false - content won't fit + fi } # Print content in order From 48220b441a4fa85879c2f9f2bb7cd23bb2d47a05 Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Thu, 20 Feb 2025 18:20:34 +0100 Subject: [PATCH 05/16] Add empty lines at the end of templates to make 'tree too large' link work --- bin/comment_with_dependency_diff | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bin/comment_with_dependency_diff b/bin/comment_with_dependency_diff index a1cd7fd5..1ebcbe11 100755 --- a/bin/comment_with_dependency_diff +++ b/bin/comment_with_dependency_diff @@ -137,8 +137,8 @@ current_size=0 # Templates for content formatting SECTION_HEADER="\n## %s changes\n\n" -TLDR_TEMPLATE="
list\n\n\`\`\`\n%s\n\`\`\`\n\n
" -TREE_TEMPLATE="
tree\n\n\`\`\`diff\n%s\n\`\`\`\n\n
" +TLDR_TEMPLATE="
list\n\n\`\`\`\n%s\n\`\`\`\n\n
\n" +TREE_TEMPLATE="
tree\n\n\`\`\`diff\n%s\n\`\`\`\n\n
\n" TREE_TOO_LARGE="\n ⚠️ %s tree is too large. [View it in Buildkite artifacts](${BUILDKITE_BUILD_URL}#${BUILDKITE_JOB_ID})\n" # Calculate total size needed for TLDRs From ec017ad737676dc3db4d3976e0c608aa50f90528 Mon Sep 17 00:00:00 2001 From: Dependency Tree Diff Tool Date: Thu, 20 Feb 2025 18:50:37 +0100 Subject: [PATCH 06/16] extract comment crafting to separate script --- bin/comment_with_dependency_diff | 66 +------------------------- bin/craft_comment_with_dependency_diff | 65 +++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 64 deletions(-) create mode 100755 bin/craft_comment_with_dependency_diff diff --git a/bin/comment_with_dependency_diff b/bin/comment_with_dependency_diff index 1ebcbe11..1499a795 100755 --- a/bin/comment_with_dependency_diff +++ b/bin/comment_with_dependency_diff @@ -30,6 +30,8 @@ set -euo pipefail +craft_comment_with_dependency_diff + MODULE="${1:-}" CONFIGURATION="${2:-}" @@ -130,69 +132,5 @@ if [ ! -s "$TLDR_DIFF_DEPENDENCIES_FILE" ] && [ ! -s "$TLDR_DIFF_BUILD_ENV_FILE" exit 0 fi -COMMENT_FILE="$DIFF_DEPENDENCIES_FOLDER/comment_body.txt" -true > $COMMENT_FILE # Clear the file -MAX_COMMENT_SIZE=65400 -current_size=0 - -# Templates for content formatting -SECTION_HEADER="\n## %s changes\n\n" -TLDR_TEMPLATE="
list\n\n\`\`\`\n%s\n\`\`\`\n\n
\n" -TREE_TEMPLATE="
tree\n\n\`\`\`diff\n%s\n\`\`\`\n\n
\n" -TREE_TOO_LARGE="\n ⚠️ %s tree is too large. [View it in Buildkite artifacts](${BUILDKITE_BUILD_URL}#${BUILDKITE_JOB_ID})\n" - -# Calculate total size needed for TLDRs -# This assumes that TLDRs are never too large to fit in the comment -tldr_size=0 -for section in "Project dependencies" "Build environment"; do - tldr_file="$TLDR_DIFF_DEPENDENCIES_FILE" - [[ $section == "Build environment" ]] && tldr_file="$TLDR_DIFF_BUILD_ENV_FILE" - - if [ -s "$tldr_file" ]; then - content=$(printf "$SECTION_HEADER$TLDR_TEMPLATE" "$section" "$(cat "$tldr_file")") - tldr_size=$(( tldr_size + ${#content} )) - fi -done - -# Function to check if content would fit -would_content_fit() { - echo "Debug: New content size: $1" - echo "Debug: TLDR total size: $tldr_size" - echo "Debug: Current size: $current_size" - echo "Debug: Max size: $MAX_COMMENT_SIZE" - if (( current_size + $1 < MAX_COMMENT_SIZE )); then - return 0 # Success/true - content will fit - else - return 1 # Failure/false - content won't fit - fi -} - -# Print content in order -for section in "Project dependencies" "Build environment"; do - tldr_file="$TLDR_DIFF_DEPENDENCIES_FILE" - tree_file="$DIFF_DEPENDENCIES_FILE" - [[ $section == "Build environment" ]] && tldr_file="$TLDR_DIFF_BUILD_ENV_FILE" && tree_file="$DIFF_BUILD_ENV_FILE" - - if [ -s "$tldr_file" ]; then - # Print section header and TLDR - printf "$SECTION_HEADER" "$section" >> $COMMENT_FILE - printf "$TLDR_TEMPLATE" "$(cat "$tldr_file")" >> $COMMENT_FILE - current_size=$(wc -c < "$COMMENT_FILE") - - # Try to add tree if it exists - if [ -s "$tree_file" ]; then - tree_content=$(cat "$tree_file") - echo "Debug: Raw tree size: ${#tree_content}" - content=$(printf "$TREE_TEMPLATE" "$tree_content") - echo "Debug: Formatted tree size: ${#content}" - if would_content_fit ${#content}; then - echo -e "$content" >> $COMMENT_FILE - current_size=$(wc -c < "$COMMENT_FILE") - else - printf "$TREE_TOO_LARGE" "$section" >> $COMMENT_FILE - fi - fi - fi -done comment_on_pr --id "dependency-diff" --if-exist update "$COMMENT_FILE" diff --git a/bin/craft_comment_with_dependency_diff b/bin/craft_comment_with_dependency_diff new file mode 100755 index 00000000..b189a565 --- /dev/null +++ b/bin/craft_comment_with_dependency_diff @@ -0,0 +1,65 @@ + +COMMENT_FILE="$DIFF_DEPENDENCIES_FOLDER/comment_body.txt" +true > $COMMENT_FILE # Clear the file +MAX_COMMENT_SIZE=65400 +current_size=0 + +# Templates for content formatting +SECTION_HEADER="\n## %s changes\n\n" +TLDR_TEMPLATE="
list\n\n\`\`\`\n%s\n\`\`\`\n\n
\n" +TREE_TEMPLATE="
tree\n\n\`\`\`diff\n%s\n\`\`\`\n\n
" +TREE_TOO_LARGE="\n ⚠️ %s tree is too large. [View it in Buildkite artifacts](${BUILDKITE_BUILD_URL}#${BUILDKITE_JOB_ID})\n" + +# Calculate total size needed for TLDRs +# This assumes that TLDRs are never too large to fit in the comment +tldr_size=0 +for section in "Project dependencies" "Build environment"; do + tldr_file="$TLDR_DIFF_DEPENDENCIES_FILE" + [[ $section == "Build environment" ]] && tldr_file="$TLDR_DIFF_BUILD_ENV_FILE" + + if [ -s "$tldr_file" ]; then + content=$(printf "$SECTION_HEADER$TLDR_TEMPLATE" "$section" "$(cat "$tldr_file")") + tldr_size=$(( tldr_size + ${#content} )) + fi +done + +# Function to check if content would fit +would_content_fit() { + echo "Debug: New content size: $1" + echo "Debug: TLDR total size: $tldr_size" + echo "Debug: Current size: $current_size" + echo "Debug: Max size: $MAX_COMMENT_SIZE" + if (( current_size + $1 < MAX_COMMENT_SIZE )); then + return 0 # Success/true - content will fit + else + return 1 # Failure/false - content won't fit + fi +} + +# Print content in order +for section in "Project dependencies" "Build environment"; do + tldr_file="$TLDR_DIFF_DEPENDENCIES_FILE" + tree_file="$DIFF_DEPENDENCIES_FILE" + [[ $section == "Build environment" ]] && tldr_file="$TLDR_DIFF_BUILD_ENV_FILE" && tree_file="$DIFF_BUILD_ENV_FILE" + + if [ -s "$tldr_file" ]; then + # Print section header and TLDR + printf "$SECTION_HEADER" "$section" >> $COMMENT_FILE + printf "$TLDR_TEMPLATE" "$(cat "$tldr_file")" >> $COMMENT_FILE + current_size=$(wc -c < "$COMMENT_FILE") + + # Try to add tree if it exists + if [ -s "$tree_file" ]; then + tree_content=$(cat "$tree_file") + echo "Debug: Raw tree size: ${#tree_content}" + content=$(printf "$TREE_TEMPLATE" "$tree_content") + echo "Debug: Formatted tree size: ${#content}" + if would_content_fit ${#content}; then + echo -e "$content" >> $COMMENT_FILE + current_size=$(wc -c < "$COMMENT_FILE") + else + printf "$TREE_TOO_LARGE" "$section" >> $COMMENT_FILE + fi + fi + fi +done \ No newline at end of file From 4b768a1d4e6774c39701834b139cf03d3b81b25f Mon Sep 17 00:00:00 2001 From: Dependency Tree Diff Tool Date: Thu, 20 Feb 2025 18:58:45 +0100 Subject: [PATCH 07/16] Add necessary bash declarations --- bin/craft_comment_with_dependency_diff | 3 +++ 1 file changed, 3 insertions(+) diff --git a/bin/craft_comment_with_dependency_diff b/bin/craft_comment_with_dependency_diff index b189a565..404dd1b1 100755 --- a/bin/craft_comment_with_dependency_diff +++ b/bin/craft_comment_with_dependency_diff @@ -1,3 +1,6 @@ +#!/bin/bash + +set -euo pipefail COMMENT_FILE="$DIFF_DEPENDENCIES_FOLDER/comment_body.txt" true > $COMMENT_FILE # Clear the file From 7e1023c2dd5d392666994aa4cddf0a209287ec86 Mon Sep 17 00:00:00 2001 From: Dependency Tree Diff Tool Date: Thu, 20 Feb 2025 19:03:09 +0100 Subject: [PATCH 08/16] Export variables shared in another script --- bin/comment_with_dependency_diff | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/bin/comment_with_dependency_diff b/bin/comment_with_dependency_diff index 1499a795..8db036bd 100755 --- a/bin/comment_with_dependency_diff +++ b/bin/comment_with_dependency_diff @@ -30,8 +30,6 @@ set -euo pipefail -craft_comment_with_dependency_diff - MODULE="${1:-}" CONFIGURATION="${2:-}" @@ -45,19 +43,19 @@ if [ "${BUILDKITE_PULL_REQUEST:-false}" == "false" ]; then exit 2 fi -DIFF_DEPENDENCIES_FOLDER="./build/reports/diff" +export DIFF_DEPENDENCIES_FOLDER="./build/reports/diff" BASE_BRANCH_DEPENDENCIES_FILE="$DIFF_DEPENDENCIES_FOLDER/base_branch_dependencies.txt" HEAD_BRANCH_DEPENDENCIES_FILE="$DIFF_DEPENDENCIES_FOLDER/head_branch_dependencies.txt" BASE_BRANCH_BUILD_ENV_FILE="$DIFF_DEPENDENCIES_FOLDER/base_branch_build_env.txt" HEAD_BRANCH_BUILD_ENV_FILE="$DIFF_DEPENDENCIES_FOLDER/head_branch_build_env.txt" -DIFF_DEPENDENCIES_FILE="$DIFF_DEPENDENCIES_FOLDER/diff_dependencies.txt" -DIFF_BUILD_ENV_FILE="$DIFF_DEPENDENCIES_FOLDER/diff_build_env.txt" +export DIFF_DEPENDENCIES_FILE="$DIFF_DEPENDENCIES_FOLDER/diff_dependencies.txt" +export DIFF_BUILD_ENV_FILE="$DIFF_DEPENDENCIES_FOLDER/diff_build_env.txt" DEPENDENCY_TREE_VERSION="1.2.1" DEPENDENCY_TREE_CHECKSUM="39c50f22cd4211a3e52afec43120b7fdf90f9890" -TLDR_DIFF_DEPENDENCIES_FILE="$DIFF_DEPENDENCIES_FOLDER/tldr_diff_dependencies.txt" -TLDR_DIFF_BUILD_ENV_FILE="$DIFF_DEPENDENCIES_FOLDER/tldr_diff_build_env.txt" +export TLDR_DIFF_DEPENDENCIES_FILE="$DIFF_DEPENDENCIES_FOLDER/tldr_diff_dependencies.txt" +export TLDR_DIFF_BUILD_ENV_FILE="$DIFF_DEPENDENCIES_FOLDER/tldr_diff_build_env.txt" git config --global user.email "mobile@automattic.com" git config --global user.name "Dependency Tree Diff Tool" @@ -132,5 +130,6 @@ if [ ! -s "$TLDR_DIFF_DEPENDENCIES_FILE" ] && [ ! -s "$TLDR_DIFF_BUILD_ENV_FILE" exit 0 fi +craft_comment_with_dependency_diff comment_on_pr --id "dependency-diff" --if-exist update "$COMMENT_FILE" From 6ea0801f088baa4c1ee45ceb967cfe2243764a27 Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Thu, 20 Feb 2025 19:10:18 +0100 Subject: [PATCH 09/16] Export variables shared in another script --- bin/craft_comment_with_dependency_diff | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/craft_comment_with_dependency_diff b/bin/craft_comment_with_dependency_diff index 404dd1b1..b322ee4b 100755 --- a/bin/craft_comment_with_dependency_diff +++ b/bin/craft_comment_with_dependency_diff @@ -2,7 +2,7 @@ set -euo pipefail -COMMENT_FILE="$DIFF_DEPENDENCIES_FOLDER/comment_body.txt" +export COMMENT_FILE="$DIFF_DEPENDENCIES_FOLDER/comment_body.txt" true > $COMMENT_FILE # Clear the file MAX_COMMENT_SIZE=65400 current_size=0 From 47e9e93d91c016107e9d6daabe5ec9891a7f8590 Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Thu, 20 Feb 2025 19:26:42 +0100 Subject: [PATCH 10/16] move COMMENT_FILE to comment_with_dependency_diff --- bin/comment_with_dependency_diff | 1 + bin/craft_comment_with_dependency_diff | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/comment_with_dependency_diff b/bin/comment_with_dependency_diff index 8db036bd..d0c3dfa2 100755 --- a/bin/comment_with_dependency_diff +++ b/bin/comment_with_dependency_diff @@ -44,6 +44,7 @@ if [ "${BUILDKITE_PULL_REQUEST:-false}" == "false" ]; then fi export DIFF_DEPENDENCIES_FOLDER="./build/reports/diff" +export COMMENT_FILE="$DIFF_DEPENDENCIES_FOLDER/comment_body.txt" BASE_BRANCH_DEPENDENCIES_FILE="$DIFF_DEPENDENCIES_FOLDER/base_branch_dependencies.txt" HEAD_BRANCH_DEPENDENCIES_FILE="$DIFF_DEPENDENCIES_FOLDER/head_branch_dependencies.txt" BASE_BRANCH_BUILD_ENV_FILE="$DIFF_DEPENDENCIES_FOLDER/base_branch_build_env.txt" diff --git a/bin/craft_comment_with_dependency_diff b/bin/craft_comment_with_dependency_diff index b322ee4b..c593c0e0 100755 --- a/bin/craft_comment_with_dependency_diff +++ b/bin/craft_comment_with_dependency_diff @@ -2,7 +2,6 @@ set -euo pipefail -export COMMENT_FILE="$DIFF_DEPENDENCIES_FOLDER/comment_body.txt" true > $COMMENT_FILE # Clear the file MAX_COMMENT_SIZE=65400 current_size=0 From 18bfbf9498272f2758f2fbe004ae0c4db8af2a23 Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Thu, 20 Feb 2025 19:27:42 +0100 Subject: [PATCH 11/16] add test for crafting the dependency diff comment --- ...st_craft_comment_with_dependency_diff.bats | 108 ++++++++++++++++++ 1 file changed, 108 insertions(+) create mode 100644 tests/test_craft_comment_with_dependency_diff.bats diff --git a/tests/test_craft_comment_with_dependency_diff.bats b/tests/test_craft_comment_with_dependency_diff.bats new file mode 100644 index 00000000..3c72108e --- /dev/null +++ b/tests/test_craft_comment_with_dependency_diff.bats @@ -0,0 +1,108 @@ +#!/usr/bin/env bats + +setup() { + # Get the directory of the test file + DIR="$( cd "$( dirname "$BATS_TEST_FILENAME" )" >/dev/null 2>&1 && pwd )" + + # Create a temporary directory for test artifacts + export TEMP_DIR="$(mktemp -d)" + export DIFF_DEPENDENCIES_FOLDER="$TEMP_DIR" + export COMMENT_FILE="$DIFF_DEPENDENCIES_FOLDER/comment_body.txt" + export TLDR_DIFF_DEPENDENCIES_FILE="$TEMP_DIR/tldr_dependencies.txt" + export TLDR_DIFF_BUILD_ENV_FILE="$TEMP_DIR/tldr_build_env.txt" + export DIFF_DEPENDENCIES_FILE="$TEMP_DIR/dependencies.txt" + export DIFF_BUILD_ENV_FILE="$TEMP_DIR/build_env.txt" + + # Mock Buildkite environment variables + export BUILDKITE_BUILD_URL="https://buildkite.com/test/build" + export BUILDKITE_JOB_ID="job123" +} + +teardown() { + rm -rf "$TEMP_DIR" +} + +@test "should create comment with both TLDRs and trees when content is small" { + # Create test dependency files with small content + echo "org.test:library:2.0.0 (from 1.0.0)" > "$TLDR_DIFF_DEPENDENCIES_FILE" + echo "org.test:build-tool:3.0.0 (from 2.0.0)" > "$TLDR_DIFF_BUILD_ENV_FILE" + + echo "+org.test:library:2.0.0 +-org.test:library:1.0.0" > "$DIFF_DEPENDENCIES_FILE" + + echo "+org.test:build-tool:3.0.0 +-org.test:build-tool:2.0.0" > "$DIFF_BUILD_ENV_FILE" + + # Run the script + run "$DIR/../bin/craft_comment_with_dependency_diff" + + # Assert success + [ "$status" -eq 0 ] + + # Read generated comment + local comment_content=$(<"$COMMENT_FILE") + + # Assert both sections are present + [[ "$comment_content" =~ "## Project dependencies changes" ]] + [[ "$comment_content" =~ "## Build environment changes" ]] + + # Assert both TLDRs are present + [[ "$comment_content" =~ "org.test:library:2.0.0 (from 1.0.0)" ]] + [[ "$comment_content" =~ "org.test:build-tool:3.0.0 (from 2.0.0)" ]] + + # Assert both trees are present + [[ "$comment_content" =~ "+org.test:library:2.0.0" ]] + [[ "$comment_content" =~ "+org.test:build-tool:3.0.0" ]] + + # Assert no "too large" warnings + [[ ! "$comment_content" =~ "tree is too large" ]] +} + +@test "should handle mixed tree sizes with large project dependencies" { + # Create large project dependencies tree + echo "org.test:library:2.0.0 (from 1.0.0)" > "$TLDR_DIFF_DEPENDENCIES_FILE" + + # Generate a large tree for project dependencies + local large_tree="" + for i in {1..1100}; do + large_tree+="+org.test:large-lib-${i}:2.0.0 +-org.test:large-lib-${i}:1.0.0 +" + done + echo "$large_tree" > "$DIFF_DEPENDENCIES_FILE" + + # Create small build environment changes + echo "org.test:build-tool:3.0.0 (from 2.0.0)" > "$TLDR_DIFF_BUILD_ENV_FILE" + echo "+org.test:build-tool:3.0.0 +-org.test:build-tool:2.0.0" > "$DIFF_BUILD_ENV_FILE" + + # Run the script + run "$DIR/../bin/craft_comment_with_dependency_diff" + + # Assert success + [ "$status" -eq 0 ] + + # Read generated comment + local comment_content=$(<"$COMMENT_FILE") + + echo "$comment_content"; + + # Assert both sections are present + [[ "$comment_content" =~ "## Project dependencies changes" ]] + [[ "$comment_content" =~ "## Build environment changes" ]] + + # Assert both TLDRs are present + [[ "$comment_content" =~ "org.test:library:2.0.0 (from 1.0.0)" ]] + [[ "$comment_content" =~ "org.test:build-tool:3.0.0 (from 2.0.0)" ]] + + # Assert small build environment tree is present + [[ "$comment_content" =~ "+org.test:build-tool:3.0.0" ]] + [[ "$comment_content" =~ "-org.test:build-tool:2.0.0" ]] + + # Assert large project dependencies tree is replaced with warning + [[ "$comment_content" =~ "Project dependencies tree is too large" ]] + [[ "$comment_content" =~ "View it in Buildkite artifacts" ]] + + # Assert only one "too large" warning + [[ ! "$comment_content" =~ "Build environment tree is too large" ]] +} From 83ed3c885caa494070abb600c662b498c44683d6 Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Thu, 20 Feb 2025 19:45:53 +0100 Subject: [PATCH 12/16] apply lint improvements --- bin/craft_comment_with_dependency_diff | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/bin/craft_comment_with_dependency_diff b/bin/craft_comment_with_dependency_diff index c593c0e0..ab6c9850 100755 --- a/bin/craft_comment_with_dependency_diff +++ b/bin/craft_comment_with_dependency_diff @@ -2,15 +2,15 @@ set -euo pipefail -true > $COMMENT_FILE # Clear the file +true > "$COMMENT_FILE" # Clear the file MAX_COMMENT_SIZE=65400 current_size=0 # Templates for content formatting -SECTION_HEADER="\n## %s changes\n\n" -TLDR_TEMPLATE="
list\n\n\`\`\`\n%s\n\`\`\`\n\n
\n" -TREE_TEMPLATE="
tree\n\n\`\`\`diff\n%s\n\`\`\`\n\n
" -TREE_TOO_LARGE="\n ⚠️ %s tree is too large. [View it in Buildkite artifacts](${BUILDKITE_BUILD_URL}#${BUILDKITE_JOB_ID})\n" +SECTION_HEADER='## %s changes\n\n' +TLDR_TEMPLATE='
list\n\n```\n%s\n```\n\n
\n' +TREE_TEMPLATE='
tree\n\n```diff\n%s\n```\n\n
' +TREE_TOO_LARGE='\n ⚠️ %s tree is too large. [View it in Buildkite artifacts](%s#%s)\n' # Calculate total size needed for TLDRs # This assumes that TLDRs are never too large to fit in the comment @@ -20,7 +20,7 @@ for section in "Project dependencies" "Build environment"; do [[ $section == "Build environment" ]] && tldr_file="$TLDR_DIFF_BUILD_ENV_FILE" if [ -s "$tldr_file" ]; then - content=$(printf "$SECTION_HEADER$TLDR_TEMPLATE" "$section" "$(cat "$tldr_file")") + content=$(printf '%s%s' "$(printf "$SECTION_HEADER" "$section")" "$(printf "$TLDR_TEMPLATE" "$(cat "$tldr_file")")") tldr_size=$(( tldr_size + ${#content} )) fi done @@ -46,21 +46,21 @@ for section in "Project dependencies" "Build environment"; do if [ -s "$tldr_file" ]; then # Print section header and TLDR - printf "$SECTION_HEADER" "$section" >> $COMMENT_FILE - printf "$TLDR_TEMPLATE" "$(cat "$tldr_file")" >> $COMMENT_FILE + printf '%s' "$(printf "$SECTION_HEADER" "$section")" >> "$COMMENT_FILE" + printf '%s' "$(printf "$TLDR_TEMPLATE" "$(cat "$tldr_file")")" >> "$COMMENT_FILE" current_size=$(wc -c < "$COMMENT_FILE") # Try to add tree if it exists if [ -s "$tree_file" ]; then tree_content=$(cat "$tree_file") echo "Debug: Raw tree size: ${#tree_content}" - content=$(printf "$TREE_TEMPLATE" "$tree_content") + content=$(printf '%s' "$(printf "$TREE_TEMPLATE" "$tree_content")") echo "Debug: Formatted tree size: ${#content}" if would_content_fit ${#content}; then - echo -e "$content" >> $COMMENT_FILE + echo -e "$content" >> "$COMMENT_FILE" current_size=$(wc -c < "$COMMENT_FILE") else - printf "$TREE_TOO_LARGE" "$section" >> $COMMENT_FILE + printf '%s' "$(printf "$TREE_TOO_LARGE" "$section" "$BUILDKITE_BUILD_URL" "$BUILDKITE_JOB_ID")" >> "$COMMENT_FILE" fi fi fi From ecd839361074769ccbc36f7c8448dc8a0b59f9c5 Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Fri, 21 Feb 2025 13:21:49 +0100 Subject: [PATCH 13/16] Simplify printfs and formatting of templates. Ignore `SC2059` --- bin/craft_comment_with_dependency_diff | 32 ++++++++++++++++++-------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/bin/craft_comment_with_dependency_diff b/bin/craft_comment_with_dependency_diff index ab6c9850..add8a46a 100755 --- a/bin/craft_comment_with_dependency_diff +++ b/bin/craft_comment_with_dependency_diff @@ -1,4 +1,5 @@ #!/bin/bash +# shellcheck disable=SC2059 set -euo pipefail @@ -7,10 +8,23 @@ MAX_COMMENT_SIZE=65400 current_size=0 # Templates for content formatting -SECTION_HEADER='## %s changes\n\n' -TLDR_TEMPLATE='
list\n\n```\n%s\n```\n\n
\n' -TREE_TEMPLATE='
tree\n\n```diff\n%s\n```\n\n
' -TREE_TOO_LARGE='\n ⚠️ %s tree is too large. [View it in Buildkite artifacts](%s#%s)\n' +SECTION_HEADER="## %s changes\n\n" +TLDR_TEMPLATE="
list + +\`\`\` +%s +\`\`\` + +
+" +TREE_TEMPLATE="
tree + +\`\`\`diff +%s +\`\`\` + +
" +TREE_TOO_LARGE="\n ⚠️ %s tree is too large. [View it in Buildkite artifacts](%s#%s)\n" # Calculate total size needed for TLDRs # This assumes that TLDRs are never too large to fit in the comment @@ -20,7 +34,7 @@ for section in "Project dependencies" "Build environment"; do [[ $section == "Build environment" ]] && tldr_file="$TLDR_DIFF_BUILD_ENV_FILE" if [ -s "$tldr_file" ]; then - content=$(printf '%s%s' "$(printf "$SECTION_HEADER" "$section")" "$(printf "$TLDR_TEMPLATE" "$(cat "$tldr_file")")") + content=$(printf "$SECTION_HEADER$TLDR_TEMPLATE" "$section" "$(cat "$tldr_file")") tldr_size=$(( tldr_size + ${#content} )) fi done @@ -46,21 +60,21 @@ for section in "Project dependencies" "Build environment"; do if [ -s "$tldr_file" ]; then # Print section header and TLDR - printf '%s' "$(printf "$SECTION_HEADER" "$section")" >> "$COMMENT_FILE" - printf '%s' "$(printf "$TLDR_TEMPLATE" "$(cat "$tldr_file")")" >> "$COMMENT_FILE" + printf "$SECTION_HEADER" "$section" >> "$COMMENT_FILE" + printf "$TLDR_TEMPLATE" "$(cat "$tldr_file")" >> "$COMMENT_FILE" current_size=$(wc -c < "$COMMENT_FILE") # Try to add tree if it exists if [ -s "$tree_file" ]; then tree_content=$(cat "$tree_file") echo "Debug: Raw tree size: ${#tree_content}" - content=$(printf '%s' "$(printf "$TREE_TEMPLATE" "$tree_content")") + content=$(printf "$TREE_TEMPLATE" "$tree_content") echo "Debug: Formatted tree size: ${#content}" if would_content_fit ${#content}; then echo -e "$content" >> "$COMMENT_FILE" current_size=$(wc -c < "$COMMENT_FILE") else - printf '%s' "$(printf "$TREE_TOO_LARGE" "$section" "$BUILDKITE_BUILD_URL" "$BUILDKITE_JOB_ID")" >> "$COMMENT_FILE" + printf "$TREE_TOO_LARGE" "$section" "$BUILDKITE_BUILD_URL" "$BUILDKITE_JOB_ID" >> "$COMMENT_FILE" fi fi fi From 0d519df8930ffee6ef0c632562a9d8df359b83ef Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Fri, 21 Feb 2025 13:22:04 +0100 Subject: [PATCH 14/16] Remove debug logs --- bin/craft_comment_with_dependency_diff | 6 ------ 1 file changed, 6 deletions(-) diff --git a/bin/craft_comment_with_dependency_diff b/bin/craft_comment_with_dependency_diff index add8a46a..3f3c0412 100755 --- a/bin/craft_comment_with_dependency_diff +++ b/bin/craft_comment_with_dependency_diff @@ -41,10 +41,6 @@ done # Function to check if content would fit would_content_fit() { - echo "Debug: New content size: $1" - echo "Debug: TLDR total size: $tldr_size" - echo "Debug: Current size: $current_size" - echo "Debug: Max size: $MAX_COMMENT_SIZE" if (( current_size + $1 < MAX_COMMENT_SIZE )); then return 0 # Success/true - content will fit else @@ -67,9 +63,7 @@ for section in "Project dependencies" "Build environment"; do # Try to add tree if it exists if [ -s "$tree_file" ]; then tree_content=$(cat "$tree_file") - echo "Debug: Raw tree size: ${#tree_content}" content=$(printf "$TREE_TEMPLATE" "$tree_content") - echo "Debug: Formatted tree size: ${#content}" if would_content_fit ${#content}; then echo -e "$content" >> "$COMMENT_FILE" current_size=$(wc -c < "$COMMENT_FILE") From 2175c0f81edf8f4b8e4aa93caab0d153e436e4ad Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Fri, 21 Feb 2025 13:23:55 +0100 Subject: [PATCH 15/16] Add comment to craft_comment_with_dependency_diff --- bin/craft_comment_with_dependency_diff | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/bin/craft_comment_with_dependency_diff b/bin/craft_comment_with_dependency_diff index 3f3c0412..d1d4777a 100755 --- a/bin/craft_comment_with_dependency_diff +++ b/bin/craft_comment_with_dependency_diff @@ -1,6 +1,9 @@ #!/bin/bash # shellcheck disable=SC2059 +# This script is used to craft a comment with a dependency diff and a build environment diff. +# It's extracted from comment_with_dependency_diff to be tested in isolation. + set -euo pipefail true > "$COMMENT_FILE" # Clear the file @@ -72,4 +75,4 @@ for section in "Project dependencies" "Build environment"; do fi fi fi -done \ No newline at end of file +done From 3c0700582d42f438bb1f63692c93819d11decae7 Mon Sep 17 00:00:00 2001 From: Olivier Halligon Date: Fri, 21 Feb 2025 16:17:56 +0100 Subject: [PATCH 16/16] Refactor templating to use methods. Addresses SC2059 --- bin/craft_comment_with_dependency_diff | 34 +++++++++++++++++--------- 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/bin/craft_comment_with_dependency_diff b/bin/craft_comment_with_dependency_diff index d1d4777a..0a71ef35 100755 --- a/bin/craft_comment_with_dependency_diff +++ b/bin/craft_comment_with_dependency_diff @@ -1,5 +1,4 @@ #!/bin/bash -# shellcheck disable=SC2059 # This script is used to craft a comment with a dependency diff and a build environment diff. # It's extracted from comment_with_dependency_diff to be tested in isolation. @@ -11,23 +10,34 @@ MAX_COMMENT_SIZE=65400 current_size=0 # Templates for content formatting -SECTION_HEADER="## %s changes\n\n" -TLDR_TEMPLATE="
list +print_header() { + printf "## %s changes\n\n" "$1" +} + +print_tldr() { + printf "
list \`\`\` %s \`\`\`
-" -TREE_TEMPLATE="
tree +" "$1" +} + +print_tree() { + printf "
tree \`\`\`diff %s \`\`\` -
" -TREE_TOO_LARGE="\n ⚠️ %s tree is too large. [View it in Buildkite artifacts](%s#%s)\n" +
" "$1" +} + +print_tree_too_large() { + printf "\n ⚠️ %s tree is too large. [View it in Buildkite artifacts](%s#%s)\n" "$1" "$2" "$3" +} # Calculate total size needed for TLDRs # This assumes that TLDRs are never too large to fit in the comment @@ -37,7 +47,7 @@ for section in "Project dependencies" "Build environment"; do [[ $section == "Build environment" ]] && tldr_file="$TLDR_DIFF_BUILD_ENV_FILE" if [ -s "$tldr_file" ]; then - content=$(printf "$SECTION_HEADER$TLDR_TEMPLATE" "$section" "$(cat "$tldr_file")") + content=$(print_header "$section"; print_tldr "$(cat "$tldr_file")") tldr_size=$(( tldr_size + ${#content} )) fi done @@ -59,19 +69,19 @@ for section in "Project dependencies" "Build environment"; do if [ -s "$tldr_file" ]; then # Print section header and TLDR - printf "$SECTION_HEADER" "$section" >> "$COMMENT_FILE" - printf "$TLDR_TEMPLATE" "$(cat "$tldr_file")" >> "$COMMENT_FILE" + print_header "$section" >> "$COMMENT_FILE" + print_tldr "$(cat "$tldr_file")" >> "$COMMENT_FILE" current_size=$(wc -c < "$COMMENT_FILE") # Try to add tree if it exists if [ -s "$tree_file" ]; then tree_content=$(cat "$tree_file") - content=$(printf "$TREE_TEMPLATE" "$tree_content") + content=$(print_tree "$tree_content") if would_content_fit ${#content}; then echo -e "$content" >> "$COMMENT_FILE" current_size=$(wc -c < "$COMMENT_FILE") else - printf "$TREE_TOO_LARGE" "$section" "$BUILDKITE_BUILD_URL" "$BUILDKITE_JOB_ID" >> "$COMMENT_FILE" + print_tree_too_large "$section" "$BUILDKITE_BUILD_URL" "$BUILDKITE_JOB_ID" >> "$COMMENT_FILE" fi fi fi