Skip to content

ci: avoid oversized pre-commit changed-file environment - #12192

Merged
talumbau merged 2 commits into
developfrom
users/alvasile/AIHPBLAS-3989-namespace-split/00-precommit-env-limit
Sep 17, 2026
Merged

talumbau merged 2 commits into
developfrom
users/alvasile/AIHPBLAS-3989-namespace-split/00-precommit-env-limit

Conversation

@Alex-Vasile

@Alex-Vasile Alex-Vasile commented Sep 16, 2026 •

Copy link
Copy Markdown
Contributor

JIRA ID : AIHPBLAS-3989

Motivation

The pre-commit workflow computes changed paths to select sparse-checkout directories and determine whether a project needs optional hook dependencies. PR #12150 changes 1,484 files and produces a 148,368-byte path list. When the workflow copied that list into the CHANGED_FILES environment variable, the Linux runner could not start Bash and reported Argument list too long.

Technical Details

  • Write the sorted changed paths already computed by pre_commit_helper.py to a temporary file under RUNNER_TEMP.
  • Expose only the temporary file's pathname as a GitHub Actions step output.
  • Read that file when deciding whether a project needs optional dependencies.
  • Preserve the existing resolved three-dot diff base, checked Git execution, timeout, and sparse-checkout behavior.
  • Fail the project-detection step if the changed-files file cannot be read instead of treating the project as unchanged.

Test Plan

Exercise the helper with a changed-path list larger than Linux's per-environment-entry limit, run the complete .github/scripts test suite, run the repository pre-commit hooks for the changed files, and check project detection against the actual 1,484-file namespace diff.

Test Result

  • Original failure: pre-commit run 35121163236 failed before Bash started with Argument list too long.
  • python -m pytest .github/scripts/tests -q: 136 passed, 8 xfailed, and 34 subtests passed.
  • Targeted repository pre-commit hooks passed for the helper, its tests, and the workflow.
  • The actual 1,484-path, 148,368-byte diff produced hipblaslt_changed=true and hipdnn_changed=false; an unreadable file exited with status 2.
  • Automated GitHub checks are pending for the updated commit.

Submission Checklist

Risk level

Medium (3/5): this changes dependency detection in the repository's pre-commit validation path. It does not change product or GPU behavior.

@github-actions github-actions Bot added github actions project: none Does not target any component labels Sep 16, 2026
@therock-pr-bot

therock-pr-bot Bot commented Sep 16, 2026 •

Copy link
Copy Markdown

✅ All Checks Passed — Ready for Review

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass —
🔎 pre-commit ✅ Pass —
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —
🤖 therock-pr-bot ✅ Pass —

🎉 All checks passed! This PR is ready for review.

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

@therock-pr-bot

Copy link
Copy Markdown

🎉 All checks passed! This PR is ready for review.

@Alex-Vasile
Alex-Vasile force-pushed the users/alvasile/AIHPBLAS-3989-namespace-split/00-precommit-env-limit branch from 7ede1eb to 7322520 Compare September 16, 2026 17:46
@Alex-Vasile
Alex-Vasile marked this pull request as ready for review September 16, 2026 18:13
@Alex-Vasile
Alex-Vasile requested a review from a team as a code owner September 16, 2026 18:13

@tony-davis tony-davis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice fix, thanks for tracking down the root cause and reproducing the actual 1,484-file diff to validate it. Ran the updated pre_commit_helper_test.py locally and all 6 tests pass, including the new one that generates a >128KB path list to reproduce the original failure. Good regression coverage.

One small nit, not blocking: the grep_status=$? bit in pre-commit.yml is correct (it distinguishes "no match" from "file unreadable"), but it's a little subtle for a future reader. A one-line comment above it explaining why would help:

else
  # grep exits 1 for "no match" (fine) vs >1 for a real error (e.g. unreadable file).
  grep_status=$?

Take it or leave it. Approving either way.

@talumbau
talumbau merged commit a3adf0a into develop Sep 17, 2026
676 of 688 checks passed
@talumbau
talumbau deleted the users/alvasile/AIHPBLAS-3989-namespace-split/00-precommit-env-limit branch September 17, 2026 17:48
@assistant-librarian

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants