Skip to content

Commit 7d6c8e5

Browse files
yrapartiassistant-librarian[bot]
authored andcommitted
[rocm-libraries] ROCm/rocm-libraries#6215 (commit bb1f765)
[CK] [CK Tile] Improved ci_safety_check in smart-build infrastructure (#6215) ## Motivation The two-dot syntax (origin/develop..HEAD) is more conservative and catches a broader set of changes when PRs merge develop branch. While three-dot syntax shows only PR-specific changes, two-dot ensures we don't miss any files that differ between develop and the PR branch, including files modified in both the PR and merged develop commits. This conservative approach prioritizes catching all potential issues over CI efficiency, which is appropriate for build system change detection. # Technical Details: - Switched to two-dot (..) syntax in ci_safety_check.sh - Update comments to clarify the intentional use of two-dot syntax - Maintain consistency across both CHANGE_ID branches - Trigger full build when any of the following changes - `Dockerfile|Jenkinsfile|CMakePresets\.json|script/dependency-parser/` ## Test Plan Tested with PR 6200 which has multiple merge-commits. ## Test Result It detects 43 new tests compared to 3-dot scheme. ## Submission Checklist - [x ] Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.
1 parent a170e2b commit 7d6c8e5

2 files changed

Lines changed: 24 additions & 14 deletions

File tree

script/dependency-parser/ci_safety_check.sh

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@
1818
# CHANGE_TARGET - Base branch for PR builds (set by Jenkins Multibranch Pipeline)
1919
#
2020
# Note: CHANGE_ID may not be set even for PR builds if Jenkins job is not
21-
# configured as Multibranch Pipeline. Script uses three-dot git diff syntax
22-
# to correctly detect PR changes regardless of CHANGE_ID availability.
21+
# configured as Multibranch Pipeline. Script uses two-dot git diff syntax
22+
# to detect PR changes regardless of CHANGE_ID availability.
2323
#
2424
# Manual override (set by developer/admin if needed):
2525
# DISABLE_SMART_BUILD - Set to "true" to force full build
@@ -48,19 +48,29 @@ fi
4848

4949
# 3. Force full build if CMakeLists.txt or cmake/ configuration changed
5050
# Always compare against base branch (not consecutive commits) to avoid false positives from merge commits
51-
# Three-dot syntax (...) only shows changes actually made in the PR, not changes from merged develop branch
52-
if [ -n "$CHANGE_ID" ]; then
53-
# This is a PR build (CHANGE_ID set by Jenkins Multibranch Pipeline)
54-
CHANGED_FILES=$(git diff --name-only origin/${BASE_BRANCH}...HEAD 2>/dev/null || echo "")
55-
else
56-
# Fallback: Works for both branch builds and PRs without CHANGE_ID
57-
# Use three-dot syntax to avoid including merge commit changes from develop
58-
CHANGED_FILES=$(git diff --name-only origin/${BASE_BRANCH}...HEAD 2>/dev/null || echo "")
59-
fi
51+
# Two-dot syntax (..) compares current state against base branch
52+
# Note: This includes merged changes from develop, which is conservative but safe (catches all potentially affected files)
53+
CHANGED_FILES=$(git diff --name-only origin/${BASE_BRANCH}..HEAD 2>/dev/null || echo "")
54+
55+
# Comprehensive pattern for build/infrastructure files that require full build:
56+
# - CMake: CMakeLists.txt, *.cmake, *.cmake.in, CMakePresets.json
57+
# - Docker: Dockerfile*, docker-compose*
58+
# - CI/CD: Jenkinsfile, .github/, .gitlab-ci.yml, .pre-commit-config.yaml, .readthedocs.yaml
59+
# - Scripts: script/ directory (cmake, dependency-parser, build utilities)
60+
# - Compiler: .clang-format, .clang-tidy
61+
# - Python: setup.py, pyproject.toml, requirements*.txt
62+
BUILD_INFRA_PATTERN="(CMakeLists\.txt"
63+
BUILD_INFRA_PATTERN="${BUILD_INFRA_PATTERN}|\.cmake$|\.cmake\.in$|CMakePresets\.json"
64+
BUILD_INFRA_PATTERN="${BUILD_INFRA_PATTERN}|Dockerfile|docker-compose"
65+
BUILD_INFRA_PATTERN="${BUILD_INFRA_PATTERN}|Jenkinsfile|\.github/|\.gitlab-ci\.yml"
66+
BUILD_INFRA_PATTERN="${BUILD_INFRA_PATTERN}|\.pre-commit-config\.yaml|\.readthedocs\.yaml"
67+
BUILD_INFRA_PATTERN="${BUILD_INFRA_PATTERN}|script/"
68+
BUILD_INFRA_PATTERN="${BUILD_INFRA_PATTERN}|\.clang-format|\.clang-tidy"
69+
BUILD_INFRA_PATTERN="${BUILD_INFRA_PATTERN}|setup\.py|pyproject\.toml|requirements.*\.txt)"
6070

61-
if echo "$CHANGED_FILES" | grep -qE "(CMakeLists\.txt|cmake/.*\.cmake)"; then
71+
if echo "$CHANGED_FILES" | grep -qE "${BUILD_INFRA_PATTERN}"; then
6272
FORCE_FULL_BUILD=true
63-
REASON="build system configuration changed (CMakeLists.txt or cmake/*.cmake)"
73+
REASON="build system configuration changed"
6474
fi
6575

6676
# 4. Force full build if dependency cache is older than 7 days

script/dependency-parser/validate_pr.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ git log --oneline -5
189189

190190
log_section "Step 3: Analyze Changed Files"
191191
log_info "Files changed vs $BASE_BRANCH:"
192-
CHANGED_FILES=$(git diff --name-only ${BASE_BRANCH}...HEAD -- projects/composablekernel)
192+
CHANGED_FILES=$(git diff --name-only ${BASE_BRANCH}..HEAD -- projects/composablekernel)
193193
NUM_FILES=$(echo "$CHANGED_FILES" | wc -l)
194194
echo "$CHANGED_FILES" | head -20
195195
if [ "$NUM_FILES" -gt 20 ]; then

0 commit comments

Comments
 (0)