Skip to content

[SplitBuild] Remove persistent install / dxc-dist directories#1336

Open
bob80905 wants to merge 3 commits into
llvm:mainfrom
bob80905:cleanup_tests_splitbuild
Open

[SplitBuild] Remove persistent install / dxc-dist directories#1336
bob80905 wants to merge 3 commits into
llvm:mainfrom
bob80905:cleanup_tests_splitbuild

Conversation

@bob80905

@bob80905 bob80905 commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

An issue is occurring with tests from other branches being run on PRs with branches that don't have those tests defined.
The cause is that the split build feature does not reset its test directories, just overlaying what was extracted instead.
This PR makes sure to delete any existing extraction sites from previous extractions, making sure what will be extracted is strictly from the current branch

Assisted by: Github Copilot
Fixes #1338

shell: bash
run: |
set -euxo pipefail
# Self-hosted runners persist $GITHUB_WORKSPACE between runs, and

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.

This comment seems excessively verbose to me, and only applies to the rm -rf line. To me this would be easier to read and maintain if the comment were removed. I also suggest you instruct copilot not to add verbose comments. For example, I have stored in the ghcp memory "Keep code comments terse and human-style; avoid verbose, over-explaining "AI-sounding" comments."

run: |
set -euxo pipefail
cd $GITHUB_WORKSPACE
# Self-hosted test runners persist $GITHUB_WORKSPACE between runs.

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.

and here

I maintain though that the comment is just noise and should be removed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yep, I'll remove this one entirely.
I think it would be helpful to have at least one terse comment in the source to explain what's going on with the removal though, so I think I should leave the short comment at the other site.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Split Build] Stale / non-existent tests are being run on PR's based on irrelevant branches

3 participants