You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "rluvaton (via GitHub)" <gi...@apache.org> on 2026/04/09 09:06:02 UTC
[PR] ci: add breaking change detector [datafusion]
rluvaton opened a new pull request, #21499:
URL: https://github.com/apache/datafusion/pull/21499
## Which issue does this PR close?
Partially closes:
- #21406
## Rationale for this change
detect breaking changes
## What changes are included in this PR?
add new github workflow
## Are these changes tested?
testing now
## Are there any user-facing changes?
no
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: add breaking change detector [datafusion]
Posted by "rluvaton (via GitHub)" <gi...@apache.org>.
rluvaton commented on code in PR #21499:
URL: https://github.com/apache/datafusion/pull/21499#discussion_r3060506173
##########
.github/workflows/breaking_changes_detector.yml:
##########
@@ -0,0 +1,143 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Detect semver-incompatible (breaking) API changes in crates modified by a PR.
+#
+# Only public workspace crates that have file changes are checked.
+# Internal crates (benchmarks, test-utils, sqllogictest, doc) are excluded.
+#
+# If breaking changes are found, a sticky comment is posted on the PR.
+# The comment is removed automatically once the issues are resolved.
+
+name: "Detect breaking changes"
+
+on:
+ pull_request:
+ branches:
+ - main
+
+permissions:
+ contents: read
+
+jobs:
+ check-semver:
+ name: Check semver
+ runs-on: ubuntu-latest
+ outputs:
+ logs: ${{ steps.check_semver.outputs.logs }}
+ steps:
+ - name: Checkout
+ uses: actions/checkout@v4
+ with:
+ fetch-depth: 0
+
+ # For fork PRs, `origin` points to the fork, not the upstream repo.
+ # Explicitly fetch the base branch from the upstream repo so we have
+ # a valid baseline ref for both diff and semver-checks.
+ - name: Fetch base branch
+ run: git fetch https://github.com/${{ github.repository }}.git ${{ github.base_ref }}:refs/remotes/origin/${{ github.base_ref }}
+
+ - name: Determine changed crates
+ id: changed_crates
+ run: |
+ # Parse workspace members from root Cargo.toml, excluding internal crates
+ # that are not published / not part of the public API.
+ MEMBERS=$(sed -n '/^members = \[/,/\]/p' Cargo.toml | grep '"' | sed 's/.*"\(.*\)".*/\1/' \
+ | grep -v -e '^benchmarks$' -e '^test-utils$' -e '^datafusion/sqllogictest$' -e '^datafusion/doc$')
+
+ # Diff against the base branch to find which files changed in this PR
+ CHANGED_FILES=$(git diff --name-only origin/${{ github.base_ref }}...HEAD)
+
+ # For each workspace member, check if any of its files were modified.
+ # If so, extract the crate name from its Cargo.toml.
+ PACKAGES=""
+ for member in $MEMBERS; do
+ if echo "$CHANGED_FILES" | grep -q "^${member}/"; then
+ pkg=$(grep '^name\s*=' "$member/Cargo.toml" | head -1 | sed 's/.*=\s*"\(.*\)"/\1/')
+ if [ -n "$pkg" ]; then
+ PACKAGES="$PACKAGES $pkg"
+ fi
+ fi
+ done
+
+ PACKAGES=$(echo "$PACKAGES" | xargs)
+ echo "packages=$PACKAGES" >> "$GITHUB_OUTPUT"
+ echo "Changed crates: $PACKAGES"
+
+ # Only install toolchain and cargo-semver-checks if there are crates to check
+ - name: Install Rust toolchain
+ if: steps.changed_crates.outputs.packages != ''
+ uses: dtolnay/rust-toolchain@stable
+
+ - name: Install cargo-semver-checks
+ if: steps.changed_crates.outputs.packages != ''
+ run: cargo install cargo-semver-checks
+
+ - name: Run cargo-semver-checks
+ id: check_semver
+ if: steps.changed_crates.outputs.packages != ''
+ run: |
+ set +e
+ ARGS=""
+ for pkg in ${{ steps.changed_crates.outputs.packages }}; do
+ ARGS="$ARGS --package $pkg"
+ done
+
+ # Compare the PR's code against the base branch to detect breaking changes.
+ # Use tee to show output in the Actions log while also capturing it.
+ # Strip ANSI escape codes from the captured output for the PR comment.
+ cargo semver-checks --baseline-rev origin/${{ github.base_ref }} $ARGS 2>&1 | tee /tmp/semver-output.txt
+ EXIT_CODE=${PIPESTATUS[0]}
+ OUTPUT=$(sed 's/\x1b\[[0-9;]*m//g' /tmp/semver-output.txt)
+ echo "logs<<EOF" >> "$GITHUB_OUTPUT"
+ echo "$OUTPUT" >> "$GITHUB_OUTPUT"
+ echo "EOF" >> "$GITHUB_OUTPUT"
+ exit $EXIT_CODE
+
+ # Post or remove a sticky comment on the PR based on the semver check result.
+ comment-on-pr:
+ name: Comment on pull request
+ runs-on: ubuntu-latest
+ needs: check-semver
+ if: always()
+ permissions:
+ contents: read
+ pull-requests: write
+ steps:
+ - name: Comment
+ if: ${{ needs.check-semver.result != 'success' }}
+ uses: marocchino/sticky-pull-request-comment@v2
Review Comment:
did not test the sticky pull request itself, just tested the breaking change detector
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: add breaking change detector [datafusion]
Posted by "rluvaton (via GitHub)" <gi...@apache.org>.
rluvaton commented on PR #21499:
URL: https://github.com/apache/datafusion/pull/21499#issuecomment-4214304633
@alamb, @comphead , would love your review
once merge I will iterate over this, but this works from what I tested
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: add breaking change detector [datafusion]
Posted by "rluvaton (via GitHub)" <gi...@apache.org>.
rluvaton commented on PR #21499:
URL: https://github.com/apache/datafusion/pull/21499#issuecomment-4337150487
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: add breaking change detector [datafusion]
Posted by "rluvaton (via GitHub)" <gi...@apache.org>.
rluvaton commented on PR #21499:
URL: https://github.com/apache/datafusion/pull/21499#issuecomment-4302522930
Hey @mbutrovich, any update?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: add breaking change detector [datafusion]
Posted by "rluvaton (via GitHub)" <gi...@apache.org>.
rluvaton commented on PR #21499:
URL: https://github.com/apache/datafusion/pull/21499#issuecomment-4257327133
And @comphead
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: add breaking change detector [datafusion]
Posted by "rluvaton (via GitHub)" <gi...@apache.org>.
rluvaton commented on code in PR #21499:
URL: https://github.com/apache/datafusion/pull/21499#discussion_r3060592200
##########
.github/workflows/breaking_changes_detector.yml:
##########
@@ -0,0 +1,143 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Detect semver-incompatible (breaking) API changes in crates modified by a PR.
+#
+# Only public workspace crates that have file changes are checked.
+# Internal crates (benchmarks, test-utils, sqllogictest, doc) are excluded.
+#
+# If breaking changes are found, a sticky comment is posted on the PR.
+# The comment is removed automatically once the issues are resolved.
+
+name: "Detect breaking changes"
+
+on:
+ pull_request:
+ branches:
+ - main
+
+permissions:
+ contents: read
+
+jobs:
+ check-semver:
+ name: Check semver
+ runs-on: ubuntu-latest
+ outputs:
+ logs: ${{ steps.check_semver.outputs.logs }}
+ steps:
+ - name: Checkout
+ uses: actions/checkout@v4
+ with:
+ fetch-depth: 0
+
+ # For fork PRs, `origin` points to the fork, not the upstream repo.
+ # Explicitly fetch the base branch from the upstream repo so we have
+ # a valid baseline ref for both diff and semver-checks.
+ - name: Fetch base branch
+ run: git fetch https://github.com/${{ github.repository }}.git ${{ github.base_ref }}:refs/remotes/origin/${{ github.base_ref }}
+
+ - name: Determine changed crates
+ id: changed_crates
+ run: |
+ # Parse workspace members from root Cargo.toml, excluding internal crates
Review Comment:
extracted along with creating comments
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: add breaking change detector [datafusion]
Posted by "rluvaton (via GitHub)" <gi...@apache.org>.
rluvaton merged PR #21499:
URL: https://github.com/apache/datafusion/pull/21499
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: add breaking change detector [datafusion]
Posted by "rluvaton (via GitHub)" <gi...@apache.org>.
rluvaton commented on PR #21499:
URL: https://github.com/apache/datafusion/pull/21499#issuecomment-4237088772
@alamb is this a blocker? since I don't know how to processed from here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: add breaking change detector [datafusion]
Posted by "kumarUjjawal (via GitHub)" <gi...@apache.org>.
kumarUjjawal commented on code in PR #21499:
URL: https://github.com/apache/datafusion/pull/21499#discussion_r3143771182
##########
ci/scripts/changed_crates.sh:
##########
@@ -0,0 +1,137 @@
+#!/usr/bin/env bash
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Helper script for the breaking-changes-detector workflow.
+#
+# Subcommands:
+# changed-crates <base_ref>
+# Print space-separated list of crate names whose files changed vs base_ref.
+#
+# semver-check <base_ref> <packages...>
+# Run cargo-semver-checks for the given packages against base_ref.
+# Prints the (ANSI-stripped) log output to stdout.
+# Exit code matches cargo-semver-checks (0 = pass, non-zero = breaking).
+#
+# comment <repo> <pr_number> <check_result> [logs]
+# Upsert or delete a sticky PR comment based on check_result.
+# check_result: "success" deletes any existing comment,
+# anything else upserts the comment with the provided logs.
+# Requires GH_TOKEN to be set.
+
+set -euo pipefail
+
+MARKER="<!-- semver-check-comment -->"
+
+# ── changed-crates ──────────────────────────────────────────────────
+cmd_changed_crates() {
+ local base_ref="${1:?Usage: changed_crates.sh changed-crates <base_ref>}"
+
+ # Parse workspace members from root Cargo.toml, excluding internal crates
+ # that are not published / not part of the public API.
+ local members
+ members=$(sed -n '/^members = \[/,/\]/p' Cargo.toml | grep '"' | sed 's/.*"\(.*\)".*/\1/' \
+ | grep -v -e '^benchmarks$' -e '^test-utils$' -e '^datafusion/sqllogictest$' -e '^datafusion/doc$')
+
+ local changed_files
+ changed_files=$(git diff --name-only "${base_ref}...HEAD")
+
+ local packages=""
+ for member in $members; do
+ if echo "$changed_files" | grep -q "^${member}/"; then
+ local pkg
+ pkg=$(grep '^name\s*=' "$member/Cargo.toml" | head -1 | sed 's/.*=\s*"\(.*\)"/\1/')
+ if [ -n "$pkg" ]; then
+ packages="$packages $pkg"
+ fi
+ fi
+ done
+
+ echo "$packages" | xargs
+}
+
+# ── semver-check ────────────────────────────────────────────────────
+cmd_semver_check() {
+ local base_ref="${1:?Usage: changed_crates.sh semver-check <base_ref> <packages...>}"
+ shift
+
+ local args=""
+ for pkg in "$@"; do
+ args="$args --package $pkg"
+ done
+
+ set +e
+ # Compare the PR's code against the base branch to detect breaking changes.
+ # Use tee to show output in the Actions log while also capturing it.
+ cargo semver-checks --baseline-rev "$base_ref" $args 2>&1 | tee /tmp/semver-output.txt
+ local exit_code=${PIPESTATUS[0]}
+ set -e
+
+ # Strip ANSI escape codes from the captured output for the PR comment.
+ sed 's/\x1b\[[0-9;]*m//g' /tmp/semver-output.txt
+ return "$exit_code"
+}
Review Comment:
`$()` captures ALL stdout from the subshell. tee's passthrough goes into OUTPUT, not to the Actions log, so the "keep it visible in logs" benefit doesn't actually happen and actions log sees nothing live, and OUTPUT gets the text twice (raw ANSI from tee + clean from sed).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: add breaking change detector [datafusion]
Posted by "rluvaton (via GitHub)" <gi...@apache.org>.
rluvaton commented on PR #21499:
URL: https://github.com/apache/datafusion/pull/21499#issuecomment-4335761752
Yes
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: add breaking change detector [datafusion]
Posted by "rluvaton (via GitHub)" <gi...@apache.org>.
rluvaton commented on code in PR #21499:
URL: https://github.com/apache/datafusion/pull/21499#discussion_r3154558919
##########
ci/scripts/changed_crates.sh:
##########
@@ -0,0 +1,138 @@
+#!/usr/bin/env bash
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Helper script for the breaking-changes-detector workflow.
+#
+# Subcommands:
+# changed-crates <base_ref>
+# Print space-separated list of crate names whose files changed vs base_ref.
+# Only published workspace members (those without `publish = false`) are
+# considered.
+#
+# semver-check <base_ref> <packages...>
+# Run cargo-semver-checks for the given packages against base_ref.
+# Output and exit code are passed through unchanged; the caller is
+# responsible for capturing/formatting them.
+#
+# comment <repo> <pr_number> <check_result> [logs]
+# Upsert or delete a sticky PR comment based on check_result.
+# check_result: "success" deletes any existing comment,
+# anything else upserts the comment with the provided logs.
+# Requires GH_TOKEN to be set.
+
+set -euo pipefail
+
+MARKER="<!-- semver-check-comment -->"
+
+# ── changed-crates ──────────────────────────────────────────────────
+cmd_changed_crates() {
+ local base_ref="${1:?Usage: changed_crates.sh changed-crates <base_ref>}"
+
+ # 1. Files changed between the PR and the base branch.
+ local changed_files
+ changed_files=$(git diff --name-only "${base_ref}...HEAD")
+
+ # 2. Every publishable workspace member, one per line as
+ # "<crate-name> <crate-dir>". `publish = false` in Cargo.toml shows
+ # up as `"publish": []` in cargo metadata, so filtering on that
+ # excludes internal crates without a manual exclusion list.
+ local crates
+ crates=$(cargo metadata --no-deps --format-version 1 | jq -r '
+ (.workspace_root + "/") as $root
+ | .packages[]
+ | select(.publish != [])
+ | "\(.name) \(.manifest_path | ltrimstr($root) | rtrimstr("/Cargo.toml"))"
+ ')
+
+ # 3. Keep crates whose directory contains a changed file.
+ while read -r name dir; do
+ if grep -q "^${dir}/" <<<"$changed_files"; then
+ echo "$name"
+ fi
+ done <<<"$crates" | xargs
+}
+
+# ── semver-check ────────────────────────────────────────────────────
+cmd_semver_check() {
+ local base_ref="${1:?Usage: changed_crates.sh semver-check <base_ref> <packages...>}"
+ shift
+
+ local args=()
+ for pkg in "$@"; do
+ args+=(--package "$pkg")
+ done
+
+ cargo semver-checks --baseline-rev "$base_ref" "${args[@]}"
+}
+
+# ── comment ─────────────────────────────────────────────────────────
+cmd_comment() {
+ local repo="${1:?Usage: changed_crates.sh comment <repo> <pr_number> <check_result> [logs]}"
+ local pr_number="${2:?}"
+ local check_result="${3:?}"
+ local logs="${4:-}"
+
+ # Find existing comment with our marker
+ local comment_id
+ comment_id=$(gh api "repos/${repo}/issues/${pr_number}/comments" \
+ --jq ".[] | select(.body | contains(\"${MARKER}\")) | .id" | head -1)
+
+ echo "existing breaking change comment id $comment_id"
+
+ if [ "$check_result" = "success" ]; then
+ # Delete the comment if one exists
+ if [ -n "$comment_id" ]; then
+ echo "result is success, so deleting breaking change comment"
+ gh api "repos/${repo}/issues/comments/${comment_id}" --method DELETE
+ else
+ echo "result is success and no previous comment to delete"
+ fi
+ else
+ local body="${MARKER}
+Thank you for opening this pull request!
+
+Reviewer note: [cargo-semver-checks](https://github.com/obi1kenobi/cargo-semver-checks) reported the current version number is not SemVer-compatible with the changes made since the last release.
Review Comment:
Compared against base branch since it might depend on another breaking change earlier made
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: add breaking change detector (WIP) [datafusion]
Posted by "github-advanced-security[bot] (via GitHub)" <gi...@apache.org>.
github-advanced-security[bot] commented on code in PR #21499:
URL: https://github.com/apache/datafusion/pull/21499#discussion_r3056727377
##########
.github/workflows/large_files.yml:
##########
@@ -26,6 +26,67 @@
merge_group:
jobs:
+
+ check-semver:
+ name: Check semver
+ runs-on: ubuntu-latest
+ outputs:
+ logs: ${{ steps.check_semver.outputs.logs }}
+ steps:
+ - name: Checkout
+ uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
+ with:
+ fetch-depth: 0
+
+ - name: Install Rust toolchain
+ uses: dtolnay/rust-toolchain@stable
+
+ - name: Install cargo-semver-checks
+ run: cargo install cargo-semver-checks
+
+ - name: Run cargo-semver-checks
+ id: check_semver
+ run: |
+ set +e
+ cargo semver-checks --baseline-rev origin/main 2>&1 | tee /tmp/semver-output.txt
+ EXIT_CODE=${PIPESTATUS[0]}
+ OUTPUT=$(sed 's/\x1b\[[0-9;]*m//g' /tmp/semver-output.txt)
+ echo "logs<<EOF" >> "$GITHUB_OUTPUT"
+ echo "$OUTPUT" >> "$GITHUB_OUTPUT"
+ echo "EOF" >> "$GITHUB_OUTPUT"
+ exit $EXIT_CODE
+
+ comment-on-pr:
Review Comment:
## CodeQL / Workflow does not contain permissions
Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {{contents: read}}
[Show more details](https://github.com/apache/datafusion/security/code-scanning/42)
##########
.github/workflows/breaking_changes_detector.yml:
##########
@@ -0,0 +1,67 @@
+name: "Detect breaking changes"
+
+on:
+ pull_request:
+ branches:
+ - main
+
+jobs:
+ check-semver:
+ name: Check semver
+ runs-on: ubuntu-latest
+ outputs:
+ logs: ${{ steps.check_semver.outputs.logs }}
+ steps:
+ - name: Checkout
+ uses: actions/checkout@v4
+ with:
+ fetch-depth: 0
+
+ - name: Install Rust toolchain
+ uses: dtolnay/rust-toolchain@stable
+
+ - name: Install cargo-semver-checks
+ run: cargo install cargo-semver-checks
+
+ - name: Run cargo-semver-checks
+ id: check_semver
+ run: |
+ set +e
+ cargo semver-checks --baseline-rev origin/main 2>&1 | tee /tmp/semver-output.txt
+ EXIT_CODE=${PIPESTATUS[0]}
+ OUTPUT=$(sed 's/\x1b\[[0-9;]*m//g' /tmp/semver-output.txt)
+ echo "logs<<EOF" >> "$GITHUB_OUTPUT"
+ echo "$OUTPUT" >> "$GITHUB_OUTPUT"
+ echo "EOF" >> "$GITHUB_OUTPUT"
+ exit $EXIT_CODE
+
+ comment-on-pr:
Review Comment:
## CodeQL / Workflow does not contain permissions
Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {{contents: read}}
[Show more details](https://github.com/apache/datafusion/security/code-scanning/41)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: add breaking change detector [datafusion]
Posted by "mbutrovich (via GitHub)" <gi...@apache.org>.
mbutrovich commented on PR #21499:
URL: https://github.com/apache/datafusion/pull/21499#issuecomment-4307022282
Thanks for working on this @rluvaton. As a downstream consumer maintaining [Comet's DataFusion upgrade branch](https://github.com/apache/datafusion-comet/pull/3916), I can confirm this would be valuable. The crates that generate the most upgrade churn for us are `physical_plan`, `physical_expr`, and `logical_expr` (trait changes cascade across dozens of impl sites), so catching those automatically would help a lot.
A few things I noticed:
**Workspace member parsing is fragile.** The `sed`-based TOML parsing in `changed_crates.sh` will break if the format drifts, and the hardcoded exclusion list misses several unpublished crates (`datafusion/wasmtest`, `datafusion-examples`, `datafusion/proto/gen`, `datafusion/proto-common/gen`). `cargo metadata --no-deps --format-version 1 | jq` can give you workspace members with their `publish` field, which handles this correctly and doesn't need a manual exclusion list.
**Double output in `semver-check`.** The `tee` writes to stdout *and* the file, then the `sed` strip also writes to stdout, so the workflow captures both concatenated. Either `tee /dev/stderr > /tmp/semver-output.txt` or just write to the file without tee.
**`cargo install cargo-semver-checks` on every PR run** will be slow (3-5 min compile). Consider caching the cargo install dir or using a pre-built binary.
**Unquoted `$args`** (line 83) works by accident via word-splitting. A bash array would be more robust.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: add breaking change detector [datafusion]
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on PR #21499:
URL: https://github.com/apache/datafusion/pull/21499#issuecomment-4349789267
The CI checks of https://github.com/apache/datafusion/pull/21854 started failing due to this new workflow but the error looks like missing permissions:
```
existing breaking change comment id
no comment with breaking changes, creating a new one
gh: Resource not accessible by integration (HTTP 403)
{"message":"Resource not accessible by integration","documentation_url":"https://docs.github.com/rest/issues/comments#create-an-issue-comment","status":"403"}
Error: Process completed with exit code 1.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: add breaking change detector [datafusion]
Posted by "rluvaton (via GitHub)" <gi...@apache.org>.
rluvaton commented on PR #21499:
URL: https://github.com/apache/datafusion/pull/21499#issuecomment-4217348007
> Thanks @rluvaton -- I left some ideas
>
> > once merge I will iterate over this, but this works from what I tested
>
> How did you test?
locally
> My biggest concern for all these CI jobs / improvements is the bandwidth to maintain them . We have such limited maintainer bandwidth in general.
Usually ci workflows do not really change
> Have you considered running this check on one of your own machines (rather than as a github action), polling for PRs to test? We have had good luck with that model and the `run benchmark` commands.
I don't have one
> I think external runners are better because:
>
> 1. They are easier to debug
> 2. They are maintained outside of the normal DataFusion code repo
> 3. They aren't limited to the github workflow syntax / logic (which I find very hard to debug)
>
> What do you think?
Unfortunately I don't have my own machines and maintaining a script rather than using a github action sounds harder to maintain
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: add breaking change detector [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #21499:
URL: https://github.com/apache/datafusion/pull/21499#discussion_r3060488526
##########
.github/workflows/breaking_changes_detector.yml:
##########
@@ -0,0 +1,143 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Detect semver-incompatible (breaking) API changes in crates modified by a PR.
+#
+# Only public workspace crates that have file changes are checked.
+# Internal crates (benchmarks, test-utils, sqllogictest, doc) are excluded.
+#
+# If breaking changes are found, a sticky comment is posted on the PR.
+# The comment is removed automatically once the issues are resolved.
+
+name: "Detect breaking changes"
+
+on:
+ pull_request:
+ branches:
+ - main
+
+permissions:
+ contents: read
+
+jobs:
+ check-semver:
+ name: Check semver
+ runs-on: ubuntu-latest
+ outputs:
+ logs: ${{ steps.check_semver.outputs.logs }}
+ steps:
+ - name: Checkout
+ uses: actions/checkout@v4
+ with:
+ fetch-depth: 0
+
+ # For fork PRs, `origin` points to the fork, not the upstream repo.
+ # Explicitly fetch the base branch from the upstream repo so we have
+ # a valid baseline ref for both diff and semver-checks.
+ - name: Fetch base branch
+ run: git fetch https://github.com/${{ github.repository }}.git ${{ github.base_ref }}:refs/remotes/origin/${{ github.base_ref }}
+
+ - name: Determine changed crates
+ id: changed_crates
+ run: |
+ # Parse workspace members from root Cargo.toml, excluding internal crates
+ # that are not published / not part of the public API.
+ MEMBERS=$(sed -n '/^members = \[/,/\]/p' Cargo.toml | grep '"' | sed 's/.*"\(.*\)".*/\1/' \
+ | grep -v -e '^benchmarks$' -e '^test-utils$' -e '^datafusion/sqllogictest$' -e '^datafusion/doc$')
+
+ # Diff against the base branch to find which files changed in this PR
+ CHANGED_FILES=$(git diff --name-only origin/${{ github.base_ref }}...HEAD)
+
+ # For each workspace member, check if any of its files were modified.
+ # If so, extract the crate name from its Cargo.toml.
+ PACKAGES=""
+ for member in $MEMBERS; do
+ if echo "$CHANGED_FILES" | grep -q "^${member}/"; then
+ pkg=$(grep '^name\s*=' "$member/Cargo.toml" | head -1 | sed 's/.*=\s*"\(.*\)"/\1/')
+ if [ -n "$pkg" ]; then
+ PACKAGES="$PACKAGES $pkg"
+ fi
+ fi
+ done
+
+ PACKAGES=$(echo "$PACKAGES" | xargs)
+ echo "packages=$PACKAGES" >> "$GITHUB_OUTPUT"
+ echo "Changed crates: $PACKAGES"
+
+ # Only install toolchain and cargo-semver-checks if there are crates to check
+ - name: Install Rust toolchain
+ if: steps.changed_crates.outputs.packages != ''
+ uses: dtolnay/rust-toolchain@stable
+
+ - name: Install cargo-semver-checks
+ if: steps.changed_crates.outputs.packages != ''
+ run: cargo install cargo-semver-checks
+
+ - name: Run cargo-semver-checks
+ id: check_semver
+ if: steps.changed_crates.outputs.packages != ''
+ run: |
+ set +e
+ ARGS=""
+ for pkg in ${{ steps.changed_crates.outputs.packages }}; do
+ ARGS="$ARGS --package $pkg"
+ done
+
+ # Compare the PR's code against the base branch to detect breaking changes.
+ # Use tee to show output in the Actions log while also capturing it.
+ # Strip ANSI escape codes from the captured output for the PR comment.
+ cargo semver-checks --baseline-rev origin/${{ github.base_ref }} $ARGS 2>&1 | tee /tmp/semver-output.txt
+ EXIT_CODE=${PIPESTATUS[0]}
+ OUTPUT=$(sed 's/\x1b\[[0-9;]*m//g' /tmp/semver-output.txt)
+ echo "logs<<EOF" >> "$GITHUB_OUTPUT"
+ echo "$OUTPUT" >> "$GITHUB_OUTPUT"
+ echo "EOF" >> "$GITHUB_OUTPUT"
+ exit $EXIT_CODE
+
+ # Post or remove a sticky comment on the PR based on the semver check result.
+ comment-on-pr:
+ name: Comment on pull request
+ runs-on: ubuntu-latest
+ needs: check-semver
+ if: always()
+ permissions:
+ contents: read
+ pull-requests: write
+ steps:
+ - name: Comment
+ if: ${{ needs.check-semver.result != 'success' }}
+ uses: marocchino/sticky-pull-request-comment@v2
Review Comment:
I thought in general the ASF CI jobs are supposed to avoid using non github actions (unless it is on the whitelist) -- so I am surprised this works.
##########
.github/workflows/breaking_changes_detector.yml:
##########
@@ -0,0 +1,143 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Detect semver-incompatible (breaking) API changes in crates modified by a PR.
+#
+# Only public workspace crates that have file changes are checked.
+# Internal crates (benchmarks, test-utils, sqllogictest, doc) are excluded.
+#
+# If breaking changes are found, a sticky comment is posted on the PR.
+# The comment is removed automatically once the issues are resolved.
+
+name: "Detect breaking changes"
+
+on:
+ pull_request:
+ branches:
+ - main
+
+permissions:
+ contents: read
+
+jobs:
+ check-semver:
+ name: Check semver
+ runs-on: ubuntu-latest
+ outputs:
+ logs: ${{ steps.check_semver.outputs.logs }}
+ steps:
+ - name: Checkout
+ uses: actions/checkout@v4
+ with:
+ fetch-depth: 0
+
+ # For fork PRs, `origin` points to the fork, not the upstream repo.
+ # Explicitly fetch the base branch from the upstream repo so we have
+ # a valid baseline ref for both diff and semver-checks.
+ - name: Fetch base branch
+ run: git fetch https://github.com/${{ github.repository }}.git ${{ github.base_ref }}:refs/remotes/origin/${{ github.base_ref }}
+
+ - name: Determine changed crates
+ id: changed_crates
+ run: |
+ # Parse workspace members from root Cargo.toml, excluding internal crates
Review Comment:
I am not sure how to debug scripts like this -- can you please put this logic into an existing script (like maybe `ci/scripts/changed_packages.sh`?
##########
.github/workflows/breaking_changes_detector.yml:
##########
@@ -0,0 +1,143 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Detect semver-incompatible (breaking) API changes in crates modified by a PR.
+#
+# Only public workspace crates that have file changes are checked.
+# Internal crates (benchmarks, test-utils, sqllogictest, doc) are excluded.
+#
+# If breaking changes are found, a sticky comment is posted on the PR.
+# The comment is removed automatically once the issues are resolved.
+
+name: "Detect breaking changes"
+
+on:
+ pull_request:
+ branches:
+ - main
+
+permissions:
+ contents: read
+
+jobs:
+ check-semver:
+ name: Check semver
+ runs-on: ubuntu-latest
+ outputs:
+ logs: ${{ steps.check_semver.outputs.logs }}
+ steps:
+ - name: Checkout
+ uses: actions/checkout@v4
+ with:
+ fetch-depth: 0
+
+ # For fork PRs, `origin` points to the fork, not the upstream repo.
+ # Explicitly fetch the base branch from the upstream repo so we have
+ # a valid baseline ref for both diff and semver-checks.
+ - name: Fetch base branch
+ run: git fetch https://github.com/${{ github.repository }}.git ${{ github.base_ref }}:refs/remotes/origin/${{ github.base_ref }}
+
+ - name: Determine changed crates
+ id: changed_crates
+ run: |
+ # Parse workspace members from root Cargo.toml, excluding internal crates
+ # that are not published / not part of the public API.
+ MEMBERS=$(sed -n '/^members = \[/,/\]/p' Cargo.toml | grep '"' | sed 's/.*"\(.*\)".*/\1/' \
+ | grep -v -e '^benchmarks$' -e '^test-utils$' -e '^datafusion/sqllogictest$' -e '^datafusion/doc$')
+
+ # Diff against the base branch to find which files changed in this PR
+ CHANGED_FILES=$(git diff --name-only origin/${{ github.base_ref }}...HEAD)
+
+ # For each workspace member, check if any of its files were modified.
+ # If so, extract the crate name from its Cargo.toml.
+ PACKAGES=""
+ for member in $MEMBERS; do
+ if echo "$CHANGED_FILES" | grep -q "^${member}/"; then
+ pkg=$(grep '^name\s*=' "$member/Cargo.toml" | head -1 | sed 's/.*=\s*"\(.*\)"/\1/')
+ if [ -n "$pkg" ]; then
+ PACKAGES="$PACKAGES $pkg"
+ fi
+ fi
+ done
+
+ PACKAGES=$(echo "$PACKAGES" | xargs)
+ echo "packages=$PACKAGES" >> "$GITHUB_OUTPUT"
+ echo "Changed crates: $PACKAGES"
+
+ # Only install toolchain and cargo-semver-checks if there are crates to check
+ - name: Install Rust toolchain
+ if: steps.changed_crates.outputs.packages != ''
Review Comment:
I think the github runner already has rust installed so this step is unecessary probably?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: add breaking change detector [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #21499:
URL: https://github.com/apache/datafusion/pull/21499#issuecomment-4346978039
This looks cool -- thank you
This new check is now failing on https://github.com/apache/datafusion/actions/runs/25101909134/job/73651896105?pr=21828
It is not clear from the failure how to resolve the problem (aka how to get the CI to pass after acknowledging the API change)
Is is possible to put some instructions for what to do on failure, for example similar to this:
https://github.com/apache/datafusion/blob/9a1ed57859398e36bb09b0af07ac20422f86b151/.github/workflows/rust.yml#L66-L69
?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: add breaking change detector [datafusion]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on code in PR #21499:
URL: https://github.com/apache/datafusion/pull/21499#discussion_r3154485476
##########
.github/workflows/breaking_changes_detector.yml:
##########
@@ -0,0 +1,116 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Detect semver-incompatible (breaking) API changes in crates modified by a PR.
+#
+# Only public workspace crates that have file changes are checked.
+# Internal crates (benchmarks, test-utils, sqllogictest, doc) are excluded.
+#
+# If breaking changes are found, a sticky comment is posted on the PR.
+# The comment is removed automatically once the issues are resolved.
+
+name: "Detect breaking changes"
+
+on:
+ pull_request:
+ branches:
+ - main
+
+permissions:
+ contents: read
+
+jobs:
+ check-semver:
+ name: Check semver
+ runs-on: ubuntu-latest
+ outputs:
+ logs: ${{ steps.check_semver.outputs.logs }}
+ steps:
+ - name: Checkout
+ uses: actions/checkout@v4
+ with:
+ fetch-depth: 0
+
+ # For fork PRs, `origin` points to the fork, not the upstream repo.
+ # Explicitly fetch the base branch from the upstream repo so we have
+ # a valid baseline ref for both diff and semver-checks.
+ - name: Fetch base branch
+ env:
+ BASE_REF: ${{ github.base_ref }}
+ REPO: ${{ github.repository }}
+ run: git fetch "https://github.com/${REPO}.git" "${BASE_REF}:refs/remotes/origin/${BASE_REF}"
+
+ - name: Determine changed crates
+ id: changed_crates
+ env:
+ BASE_REF: ${{ github.base_ref }}
+ run: |
+ PACKAGES=$(ci/scripts/changed_crates.sh changed-crates "origin/${BASE_REF}")
+ echo "packages=$PACKAGES" >> "$GITHUB_OUTPUT"
+ echo "Changed crates: $PACKAGES"
+
+ - name: Install cargo-semver-checks
+ if: steps.changed_crates.outputs.packages != ''
+ uses: taiki-e/install-action@94cb46f8d6e437890146ffbd78a778b78e623fb2 # v2.74.0
+ with:
+ tool: cargo-semver-checks
+
+ - name: Run cargo-semver-checks
+ id: check_semver
+ if: steps.changed_crates.outputs.packages != ''
+ env:
+ BASE_REF: ${{ github.base_ref }}
+ PACKAGES: ${{ steps.changed_crates.outputs.packages }}
+ run: |
+ set +e
+ # `tee` lets cargo's output stream live into the Actions log
+ # while we also keep a copy for the PR comment.
+ ci/scripts/changed_crates.sh semver-check "origin/${BASE_REF}" $PACKAGES \
+ 2>&1 | tee /tmp/semver-output.txt
+ EXIT_CODE=${PIPESTATUS[0]}
+ {
+ echo "logs<<EOF"
+ sed 's/\x1b\[[0-9;]*m//g' /tmp/semver-output.txt
+ echo "EOF"
+ } >> "$GITHUB_OUTPUT"
+ exit $EXIT_CODE
+
+ # Post or remove a sticky comment on the PR based on the semver check result.
+ comment-on-pr:
+ name: Comment on pull request
+ runs-on: ubuntu-latest
+ needs: check-semver
+ if: always()
+ permissions:
+ contents: read
+ pull-requests: write
+ steps:
+ - name: Checkout
+ uses: actions/checkout@v4
Review Comment:
Ditto
##########
.github/workflows/breaking_changes_detector.yml:
##########
@@ -0,0 +1,116 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Detect semver-incompatible (breaking) API changes in crates modified by a PR.
+#
+# Only public workspace crates that have file changes are checked.
+# Internal crates (benchmarks, test-utils, sqllogictest, doc) are excluded.
+#
+# If breaking changes are found, a sticky comment is posted on the PR.
+# The comment is removed automatically once the issues are resolved.
+
+name: "Detect breaking changes"
+
+on:
+ pull_request:
+ branches:
+ - main
+
+permissions:
+ contents: read
+
+jobs:
+ check-semver:
+ name: Check semver
+ runs-on: ubuntu-latest
+ outputs:
+ logs: ${{ steps.check_semver.outputs.logs }}
+ steps:
+ - name: Checkout
+ uses: actions/checkout@v4
Review Comment:
Actions should be pinned to hashes
##########
ci/scripts/changed_crates.sh:
##########
@@ -0,0 +1,138 @@
+#!/usr/bin/env bash
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Helper script for the breaking-changes-detector workflow.
+#
+# Subcommands:
+# changed-crates <base_ref>
+# Print space-separated list of crate names whose files changed vs base_ref.
+# Only published workspace members (those without `publish = false`) are
+# considered.
+#
+# semver-check <base_ref> <packages...>
+# Run cargo-semver-checks for the given packages against base_ref.
+# Output and exit code are passed through unchanged; the caller is
+# responsible for capturing/formatting them.
+#
+# comment <repo> <pr_number> <check_result> [logs]
+# Upsert or delete a sticky PR comment based on check_result.
+# check_result: "success" deletes any existing comment,
+# anything else upserts the comment with the provided logs.
+# Requires GH_TOKEN to be set.
+
+set -euo pipefail
+
+MARKER="<!-- semver-check-comment -->"
+
+# ── changed-crates ──────────────────────────────────────────────────
+cmd_changed_crates() {
+ local base_ref="${1:?Usage: changed_crates.sh changed-crates <base_ref>}"
+
+ # 1. Files changed between the PR and the base branch.
+ local changed_files
+ changed_files=$(git diff --name-only "${base_ref}...HEAD")
+
+ # 2. Every publishable workspace member, one per line as
+ # "<crate-name> <crate-dir>". `publish = false` in Cargo.toml shows
+ # up as `"publish": []` in cargo metadata, so filtering on that
+ # excludes internal crates without a manual exclusion list.
+ local crates
+ crates=$(cargo metadata --no-deps --format-version 1 | jq -r '
+ (.workspace_root + "/") as $root
+ | .packages[]
+ | select(.publish != [])
+ | "\(.name) \(.manifest_path | ltrimstr($root) | rtrimstr("/Cargo.toml"))"
+ ')
+
+ # 3. Keep crates whose directory contains a changed file.
+ while read -r name dir; do
+ if grep -q "^${dir}/" <<<"$changed_files"; then
+ echo "$name"
+ fi
+ done <<<"$crates" | xargs
+}
+
+# ── semver-check ────────────────────────────────────────────────────
+cmd_semver_check() {
+ local base_ref="${1:?Usage: changed_crates.sh semver-check <base_ref> <packages...>}"
+ shift
+
+ local args=()
+ for pkg in "$@"; do
+ args+=(--package "$pkg")
+ done
+
+ cargo semver-checks --baseline-rev "$base_ref" "${args[@]}"
+}
+
+# ── comment ─────────────────────────────────────────────────────────
+cmd_comment() {
+ local repo="${1:?Usage: changed_crates.sh comment <repo> <pr_number> <check_result> [logs]}"
+ local pr_number="${2:?}"
+ local check_result="${3:?}"
+ local logs="${4:-}"
+
+ # Find existing comment with our marker
+ local comment_id
+ comment_id=$(gh api "repos/${repo}/issues/${pr_number}/comments" \
+ --jq ".[] | select(.body | contains(\"${MARKER}\")) | .id" | head -1)
+
+ echo "existing breaking change comment id $comment_id"
+
+ if [ "$check_result" = "success" ]; then
+ # Delete the comment if one exists
+ if [ -n "$comment_id" ]; then
+ echo "result is success, so deleting breaking change comment"
+ gh api "repos/${repo}/issues/comments/${comment_id}" --method DELETE
+ else
+ echo "result is success and no previous comment to delete"
+ fi
+ else
+ local body="${MARKER}
+Thank you for opening this pull request!
+
+Reviewer note: [cargo-semver-checks](https://github.com/obi1kenobi/cargo-semver-checks) reported the current version number is not SemVer-compatible with the changes made since the last release.
+
+Details:
+
+\`\`\`
Review Comment:
Could we make this collapsed by default?
##########
ci/scripts/changed_crates.sh:
##########
@@ -0,0 +1,138 @@
+#!/usr/bin/env bash
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Helper script for the breaking-changes-detector workflow.
+#
+# Subcommands:
+# changed-crates <base_ref>
+# Print space-separated list of crate names whose files changed vs base_ref.
+# Only published workspace members (those without `publish = false`) are
+# considered.
+#
+# semver-check <base_ref> <packages...>
+# Run cargo-semver-checks for the given packages against base_ref.
+# Output and exit code are passed through unchanged; the caller is
+# responsible for capturing/formatting them.
+#
+# comment <repo> <pr_number> <check_result> [logs]
+# Upsert or delete a sticky PR comment based on check_result.
+# check_result: "success" deletes any existing comment,
+# anything else upserts the comment with the provided logs.
+# Requires GH_TOKEN to be set.
+
+set -euo pipefail
+
+MARKER="<!-- semver-check-comment -->"
+
+# ── changed-crates ──────────────────────────────────────────────────
+cmd_changed_crates() {
+ local base_ref="${1:?Usage: changed_crates.sh changed-crates <base_ref>}"
+
+ # 1. Files changed between the PR and the base branch.
+ local changed_files
+ changed_files=$(git diff --name-only "${base_ref}...HEAD")
+
+ # 2. Every publishable workspace member, one per line as
+ # "<crate-name> <crate-dir>". `publish = false` in Cargo.toml shows
+ # up as `"publish": []` in cargo metadata, so filtering on that
+ # excludes internal crates without a manual exclusion list.
+ local crates
+ crates=$(cargo metadata --no-deps --format-version 1 | jq -r '
+ (.workspace_root + "/") as $root
+ | .packages[]
+ | select(.publish != [])
+ | "\(.name) \(.manifest_path | ltrimstr($root) | rtrimstr("/Cargo.toml"))"
+ ')
+
+ # 3. Keep crates whose directory contains a changed file.
+ while read -r name dir; do
+ if grep -q "^${dir}/" <<<"$changed_files"; then
+ echo "$name"
+ fi
+ done <<<"$crates" | xargs
+}
+
+# ── semver-check ────────────────────────────────────────────────────
+cmd_semver_check() {
+ local base_ref="${1:?Usage: changed_crates.sh semver-check <base_ref> <packages...>}"
+ shift
+
+ local args=()
+ for pkg in "$@"; do
+ args+=(--package "$pkg")
+ done
+
+ cargo semver-checks --baseline-rev "$base_ref" "${args[@]}"
+}
+
+# ── comment ─────────────────────────────────────────────────────────
+cmd_comment() {
+ local repo="${1:?Usage: changed_crates.sh comment <repo> <pr_number> <check_result> [logs]}"
+ local pr_number="${2:?}"
+ local check_result="${3:?}"
+ local logs="${4:-}"
+
+ # Find existing comment with our marker
+ local comment_id
+ comment_id=$(gh api "repos/${repo}/issues/${pr_number}/comments" \
+ --jq ".[] | select(.body | contains(\"${MARKER}\")) | .id" | head -1)
+
+ echo "existing breaking change comment id $comment_id"
+
+ if [ "$check_result" = "success" ]; then
+ # Delete the comment if one exists
+ if [ -n "$comment_id" ]; then
+ echo "result is success, so deleting breaking change comment"
+ gh api "repos/${repo}/issues/comments/${comment_id}" --method DELETE
+ else
+ echo "result is success and no previous comment to delete"
+ fi
+ else
+ local body="${MARKER}
+Thank you for opening this pull request!
+
+Reviewer note: [cargo-semver-checks](https://github.com/obi1kenobi/cargo-semver-checks) reported the current version number is not SemVer-compatible with the changes made since the last release.
Review Comment:
since the last release, or compared against main?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: add breaking change detector [datafusion]
Posted by "rluvaton (via GitHub)" <gi...@apache.org>.
rluvaton commented on code in PR #21499:
URL: https://github.com/apache/datafusion/pull/21499#discussion_r3143759203
##########
ci/scripts/changed_crates.sh:
##########
@@ -0,0 +1,137 @@
+#!/usr/bin/env bash
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Helper script for the breaking-changes-detector workflow.
+#
+# Subcommands:
+# changed-crates <base_ref>
+# Print space-separated list of crate names whose files changed vs base_ref.
+#
+# semver-check <base_ref> <packages...>
+# Run cargo-semver-checks for the given packages against base_ref.
+# Prints the (ANSI-stripped) log output to stdout.
+# Exit code matches cargo-semver-checks (0 = pass, non-zero = breaking).
+#
+# comment <repo> <pr_number> <check_result> [logs]
+# Upsert or delete a sticky PR comment based on check_result.
+# check_result: "success" deletes any existing comment,
+# anything else upserts the comment with the provided logs.
+# Requires GH_TOKEN to be set.
+
+set -euo pipefail
+
+MARKER="<!-- semver-check-comment -->"
+
+# ── changed-crates ──────────────────────────────────────────────────
+cmd_changed_crates() {
+ local base_ref="${1:?Usage: changed_crates.sh changed-crates <base_ref>}"
+
+ # Parse workspace members from root Cargo.toml, excluding internal crates
+ # that are not published / not part of the public API.
+ local members
+ members=$(sed -n '/^members = \[/,/\]/p' Cargo.toml | grep '"' | sed 's/.*"\(.*\)".*/\1/' \
+ | grep -v -e '^benchmarks$' -e '^test-utils$' -e '^datafusion/sqllogictest$' -e '^datafusion/doc$')
+
+ local changed_files
+ changed_files=$(git diff --name-only "${base_ref}...HEAD")
+
+ local packages=""
+ for member in $members; do
+ if echo "$changed_files" | grep -q "^${member}/"; then
+ local pkg
+ pkg=$(grep '^name\s*=' "$member/Cargo.toml" | head -1 | sed 's/.*=\s*"\(.*\)"/\1/')
+ if [ -n "$pkg" ]; then
+ packages="$packages $pkg"
+ fi
+ fi
+ done
+
+ echo "$packages" | xargs
+}
+
+# ── semver-check ────────────────────────────────────────────────────
+cmd_semver_check() {
+ local base_ref="${1:?Usage: changed_crates.sh semver-check <base_ref> <packages...>}"
+ shift
+
+ local args=""
+ for pkg in "$@"; do
+ args="$args --package $pkg"
+ done
+
+ set +e
+ # Compare the PR's code against the base branch to detect breaking changes.
+ # Use tee to show output in the Actions log while also capturing it.
+ cargo semver-checks --baseline-rev "$base_ref" $args 2>&1 | tee /tmp/semver-output.txt
+ local exit_code=${PIPESTATUS[0]}
+ set -e
+
+ # Strip ANSI escape codes from the captured output for the PR comment.
+ sed 's/\x1b\[[0-9;]*m//g' /tmp/semver-output.txt
+ return "$exit_code"
+}
Review Comment:
duplicate where? it is meant to keep the action output logs while also capturing for adding comment
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: add breaking change detector [datafusion]
Posted by "rluvaton (via GitHub)" <gi...@apache.org>.
rluvaton commented on PR #21499:
URL: https://github.com/apache/datafusion/pull/21499#issuecomment-4327380634
@mbutrovich can you please re-review
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: add breaking change detector [datafusion]
Posted by "kumarUjjawal (via GitHub)" <gi...@apache.org>.
kumarUjjawal commented on code in PR #21499:
URL: https://github.com/apache/datafusion/pull/21499#discussion_r3142996235
##########
ci/scripts/changed_crates.sh:
##########
@@ -0,0 +1,137 @@
+#!/usr/bin/env bash
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Helper script for the breaking-changes-detector workflow.
+#
+# Subcommands:
+# changed-crates <base_ref>
+# Print space-separated list of crate names whose files changed vs base_ref.
+#
+# semver-check <base_ref> <packages...>
+# Run cargo-semver-checks for the given packages against base_ref.
+# Prints the (ANSI-stripped) log output to stdout.
+# Exit code matches cargo-semver-checks (0 = pass, non-zero = breaking).
+#
+# comment <repo> <pr_number> <check_result> [logs]
+# Upsert or delete a sticky PR comment based on check_result.
+# check_result: "success" deletes any existing comment,
+# anything else upserts the comment with the provided logs.
+# Requires GH_TOKEN to be set.
+
+set -euo pipefail
+
+MARKER="<!-- semver-check-comment -->"
+
+# ── changed-crates ──────────────────────────────────────────────────
+cmd_changed_crates() {
+ local base_ref="${1:?Usage: changed_crates.sh changed-crates <base_ref>}"
+
+ # Parse workspace members from root Cargo.toml, excluding internal crates
+ # that are not published / not part of the public API.
+ local members
+ members=$(sed -n '/^members = \[/,/\]/p' Cargo.toml | grep '"' | sed 's/.*"\(.*\)".*/\1/' \
+ | grep -v -e '^benchmarks$' -e '^test-utils$' -e '^datafusion/sqllogictest$' -e '^datafusion/doc$')
+
+ local changed_files
+ changed_files=$(git diff --name-only "${base_ref}...HEAD")
+
+ local packages=""
+ for member in $members; do
+ if echo "$changed_files" | grep -q "^${member}/"; then
+ local pkg
+ pkg=$(grep '^name\s*=' "$member/Cargo.toml" | head -1 | sed 's/.*=\s*"\(.*\)"/\1/')
+ if [ -n "$pkg" ]; then
+ packages="$packages $pkg"
+ fi
+ fi
+ done
+
+ echo "$packages" | xargs
+}
+
+# ── semver-check ────────────────────────────────────────────────────
+cmd_semver_check() {
+ local base_ref="${1:?Usage: changed_crates.sh semver-check <base_ref> <packages...>}"
+ shift
+
+ local args=""
+ for pkg in "$@"; do
+ args="$args --package $pkg"
+ done
+
+ set +e
+ # Compare the PR's code against the base branch to detect breaking changes.
+ # Use tee to show output in the Actions log while also capturing it.
+ cargo semver-checks --baseline-rev "$base_ref" $args 2>&1 | tee /tmp/semver-output.txt
+ local exit_code=${PIPESTATUS[0]}
+ set -e
+
+ # Strip ANSI escape codes from the captured output for the PR comment.
+ sed 's/\x1b\[[0-9;]*m//g' /tmp/semver-output.txt
+ return "$exit_code"
+}
Review Comment:
Wouldn't this be duplicated, both raw ANSI and sed?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: add breaking change detector [datafusion]
Posted by "kumarUjjawal (via GitHub)" <gi...@apache.org>.
kumarUjjawal commented on PR #21499:
URL: https://github.com/apache/datafusion/pull/21499#issuecomment-4322458260
> **Double output in `semver-check`.** The `tee` writes to stdout _and_ the file, then the `sed` strip also writes to stdout, so the workflow captures both concatenated. Either `tee /dev/stderr > /tmp/semver-output.txt` or just write to the file without tee.
Same point is made here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: add breaking change detector [datafusion]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on code in PR #21499:
URL: https://github.com/apache/datafusion/pull/21499#discussion_r3154613203
##########
ci/scripts/changed_crates.sh:
##########
@@ -0,0 +1,138 @@
+#!/usr/bin/env bash
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Helper script for the breaking-changes-detector workflow.
+#
+# Subcommands:
+# changed-crates <base_ref>
+# Print space-separated list of crate names whose files changed vs base_ref.
+# Only published workspace members (those without `publish = false`) are
+# considered.
+#
+# semver-check <base_ref> <packages...>
+# Run cargo-semver-checks for the given packages against base_ref.
+# Output and exit code are passed through unchanged; the caller is
+# responsible for capturing/formatting them.
+#
+# comment <repo> <pr_number> <check_result> [logs]
+# Upsert or delete a sticky PR comment based on check_result.
+# check_result: "success" deletes any existing comment,
+# anything else upserts the comment with the provided logs.
+# Requires GH_TOKEN to be set.
+
+set -euo pipefail
+
+MARKER="<!-- semver-check-comment -->"
+
+# ── changed-crates ──────────────────────────────────────────────────
+cmd_changed_crates() {
+ local base_ref="${1:?Usage: changed_crates.sh changed-crates <base_ref>}"
+
+ # 1. Files changed between the PR and the base branch.
+ local changed_files
+ changed_files=$(git diff --name-only "${base_ref}...HEAD")
+
+ # 2. Every publishable workspace member, one per line as
+ # "<crate-name> <crate-dir>". `publish = false` in Cargo.toml shows
+ # up as `"publish": []` in cargo metadata, so filtering on that
+ # excludes internal crates without a manual exclusion list.
+ local crates
+ crates=$(cargo metadata --no-deps --format-version 1 | jq -r '
+ (.workspace_root + "/") as $root
+ | .packages[]
+ | select(.publish != [])
+ | "\(.name) \(.manifest_path | ltrimstr($root) | rtrimstr("/Cargo.toml"))"
+ ')
+
+ # 3. Keep crates whose directory contains a changed file.
+ while read -r name dir; do
+ if grep -q "^${dir}/" <<<"$changed_files"; then
+ echo "$name"
+ fi
+ done <<<"$crates" | xargs
+}
+
+# ── semver-check ────────────────────────────────────────────────────
+cmd_semver_check() {
+ local base_ref="${1:?Usage: changed_crates.sh semver-check <base_ref> <packages...>}"
+ shift
+
+ local args=()
+ for pkg in "$@"; do
+ args+=(--package "$pkg")
+ done
+
+ cargo semver-checks --baseline-rev "$base_ref" "${args[@]}"
+}
+
+# ── comment ─────────────────────────────────────────────────────────
+cmd_comment() {
+ local repo="${1:?Usage: changed_crates.sh comment <repo> <pr_number> <check_result> [logs]}"
+ local pr_number="${2:?}"
+ local check_result="${3:?}"
+ local logs="${4:-}"
+
+ # Find existing comment with our marker
+ local comment_id
+ comment_id=$(gh api "repos/${repo}/issues/${pr_number}/comments" \
+ --jq ".[] | select(.body | contains(\"${MARKER}\")) | .id" | head -1)
+
+ echo "existing breaking change comment id $comment_id"
+
+ if [ "$check_result" = "success" ]; then
+ # Delete the comment if one exists
+ if [ -n "$comment_id" ]; then
+ echo "result is success, so deleting breaking change comment"
+ gh api "repos/${repo}/issues/comments/${comment_id}" --method DELETE
+ else
+ echo "result is success and no previous comment to delete"
+ fi
+ else
+ local body="${MARKER}
+Thank you for opening this pull request!
+
+Reviewer note: [cargo-semver-checks](https://github.com/obi1kenobi/cargo-semver-checks) reported the current version number is not SemVer-compatible with the changes made since the last release.
Review Comment:
Could we amend the text here then to reflect this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: add breaking change detector [datafusion]
Posted by "rluvaton (via GitHub)" <gi...@apache.org>.
rluvaton commented on code in PR #21499:
URL: https://github.com/apache/datafusion/pull/21499#discussion_r3155767422
##########
.github/workflows/breaking_changes_detector.yml:
##########
@@ -0,0 +1,116 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Detect semver-incompatible (breaking) API changes in crates modified by a PR.
+#
+# Only public workspace crates that have file changes are checked.
+# Internal crates (benchmarks, test-utils, sqllogictest, doc) are excluded.
+#
+# If breaking changes are found, a sticky comment is posted on the PR.
+# The comment is removed automatically once the issues are resolved.
+
+name: "Detect breaking changes"
+
+on:
+ pull_request:
+ branches:
+ - main
+
+permissions:
+ contents: read
+
+jobs:
+ check-semver:
+ name: Check semver
+ runs-on: ubuntu-latest
+ outputs:
+ logs: ${{ steps.check_semver.outputs.logs }}
+ steps:
+ - name: Checkout
+ uses: actions/checkout@v4
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: add breaking change detector [datafusion]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on PR #21499:
URL: https://github.com/apache/datafusion/pull/21499#issuecomment-4335832225
> Yes
Is it possible to not fail then? Since this is meant to provide a useful comment for reviewers, so having it fail because of a breaking API change being detected could be confusing to PR authors
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: add breaking change detector [datafusion]
Posted by "rluvaton (via GitHub)" <gi...@apache.org>.
rluvaton commented on PR #21499:
URL: https://github.com/apache/datafusion/pull/21499#issuecomment-4350340128
The fix is merged, sorry for the trouble
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: add breaking change detector [datafusion]
Posted by "rluvaton (via GitHub)" <gi...@apache.org>.
rluvaton commented on PR #21499:
URL: https://github.com/apache/datafusion/pull/21499#issuecomment-4326331082
Resolved all comments and verified in my fork:
- https://github.com/rluvaton/datafusion/pull/9
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: add breaking change detector (WIP) [datafusion]
Posted by "rluvaton (via GitHub)" <gi...@apache.org>.
rluvaton commented on code in PR #21499:
URL: https://github.com/apache/datafusion/pull/21499#discussion_r3056718276
##########
.github/workflows/large_files.yml:
##########
Review Comment:
Added to here so I can test it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: add breaking change detector [datafusion]
Posted by "mbutrovich (via GitHub)" <gi...@apache.org>.
mbutrovich commented on PR #21499:
URL: https://github.com/apache/datafusion/pull/21499#issuecomment-4261981655
> @mbutrovich care to give your thoughts since you did the last datafusion upgrade in comet
I'll take a look this week. I'm trying something new with Comet for this release: keeping a PR branch in sync with DataFusion `main` and firing off issues/PRs to upstream DataFusion as they come up. I'm hoping this means Comet can actually +1 a DataFusion .0 release instead of increasingly needing to wait for a .x minor release to get caught up.
https://github.com/apache/datafusion-comet/pull/3916
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: add breaking change detector [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #21499:
URL: https://github.com/apache/datafusion/pull/21499#issuecomment-4237300515
> @alamb is this a blocker? since I don't know how to processed from here
Perhaps we should solicit some other maintainer reviews
I don't feel comfortable that I would be able to review / maintain this job, nor am I sure it does not introduce a security issue in github actions
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: add breaking change detector [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #21499:
URL: https://github.com/apache/datafusion/pull/21499#issuecomment-4225395075
> > My biggest concern for all these CI jobs / improvements is the bandwidth to maintain them . We have such limited maintainer bandwidth in general.
>
> Usually ci workflows do not really change
Maybe that is true, but I have spent more hours of my life trying to debug / fix github workflows than I would like to admit.
> Unfortunately I don't have my own machines and maintaining a my own script which involves logic that already come built in inside github action sounds harder to maintain, and also I would need to manage my own machines, debug, etc rather than using existing github infra
Yes it definitely shifts the maintenance burden (aka who pays for the maintenance)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: add breaking change detector [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #21499:
URL: https://github.com/apache/datafusion/pull/21499#issuecomment-4347090814
🤔 the failure on this PR seems like a false positive:
- https://github.com/apache/datafusion/actions/runs/25108535879/job/73576369836?pr=21919
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: add breaking change detector [datafusion]
Posted by "kumarUjjawal (via GitHub)" <gi...@apache.org>.
kumarUjjawal commented on PR #21499:
URL: https://github.com/apache/datafusion/pull/21499#issuecomment-4349840539
There is a pr related to this https://github.com/apache/datafusion/pull/21913
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: add breaking change detector [datafusion]
Posted by "rluvaton (via GitHub)" <gi...@apache.org>.
rluvaton commented on code in PR #21499:
URL: https://github.com/apache/datafusion/pull/21499#discussion_r3146668189
##########
ci/scripts/changed_crates.sh:
##########
@@ -0,0 +1,137 @@
+#!/usr/bin/env bash
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Helper script for the breaking-changes-detector workflow.
+#
+# Subcommands:
+# changed-crates <base_ref>
+# Print space-separated list of crate names whose files changed vs base_ref.
+#
+# semver-check <base_ref> <packages...>
+# Run cargo-semver-checks for the given packages against base_ref.
+# Prints the (ANSI-stripped) log output to stdout.
+# Exit code matches cargo-semver-checks (0 = pass, non-zero = breaking).
+#
+# comment <repo> <pr_number> <check_result> [logs]
+# Upsert or delete a sticky PR comment based on check_result.
+# check_result: "success" deletes any existing comment,
+# anything else upserts the comment with the provided logs.
+# Requires GH_TOKEN to be set.
+
+set -euo pipefail
+
+MARKER="<!-- semver-check-comment -->"
+
+# ── changed-crates ──────────────────────────────────────────────────
+cmd_changed_crates() {
+ local base_ref="${1:?Usage: changed_crates.sh changed-crates <base_ref>}"
+
+ # Parse workspace members from root Cargo.toml, excluding internal crates
+ # that are not published / not part of the public API.
+ local members
+ members=$(sed -n '/^members = \[/,/\]/p' Cargo.toml | grep '"' | sed 's/.*"\(.*\)".*/\1/' \
+ | grep -v -e '^benchmarks$' -e '^test-utils$' -e '^datafusion/sqllogictest$' -e '^datafusion/doc$')
+
+ local changed_files
+ changed_files=$(git diff --name-only "${base_ref}...HEAD")
+
+ local packages=""
+ for member in $members; do
+ if echo "$changed_files" | grep -q "^${member}/"; then
+ local pkg
+ pkg=$(grep '^name\s*=' "$member/Cargo.toml" | head -1 | sed 's/.*=\s*"\(.*\)"/\1/')
+ if [ -n "$pkg" ]; then
+ packages="$packages $pkg"
+ fi
+ fi
+ done
+
+ echo "$packages" | xargs
+}
+
+# ── semver-check ────────────────────────────────────────────────────
+cmd_semver_check() {
+ local base_ref="${1:?Usage: changed_crates.sh semver-check <base_ref> <packages...>}"
+ shift
+
+ local args=""
+ for pkg in "$@"; do
+ args="$args --package $pkg"
+ done
+
+ set +e
+ # Compare the PR's code against the base branch to detect breaking changes.
+ # Use tee to show output in the Actions log while also capturing it.
+ cargo semver-checks --baseline-rev "$base_ref" $args 2>&1 | tee /tmp/semver-output.txt
+ local exit_code=${PIPESTATUS[0]}
+ set -e
+
+ # Strip ANSI escape codes from the captured output for the PR comment.
+ sed 's/\x1b\[[0-9;]*m//g' /tmp/semver-output.txt
+ return "$exit_code"
+}
Review Comment:
fixed and verified
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: add breaking change detector [datafusion]
Posted by "rluvaton (via GitHub)" <gi...@apache.org>.
rluvaton commented on PR #21499:
URL: https://github.com/apache/datafusion/pull/21499#issuecomment-4242683273
@mbutrovich care to give your thoughts since you did the last datafusion upgrade in comet
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org