Skip to content

only run ci jobs for changed files - #5354

Open
LusterSourav wants to merge 11 commits into
rust-lang:mainfrom
LusterSourav:ci-changes-upstream
Open

only run ci jobs for changed files#5354
LusterSourav wants to merge 11 commits into
rust-lang:mainfrom
LusterSourav:ci-changes-upstream

Conversation

@LusterSourav

@LusterSourav LusterSourav commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

closes #3445

@rustbot rustbot added A-CI Area: CI-related items S-waiting-on-review labels Aug 2, 2026
@LusterSourav

Copy link
Copy Markdown
Contributor Author

@rustbot ready

@LusterSourav

LusterSourav commented Aug 2, 2026

Copy link
Copy Markdown
Contributor Author

@JohnTitor could u please review it

Comment thread .github/workflows/ci.yaml
@rustbot

This comment has been minimized.

@rustbot

rustbot commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator

⚠️ Warning ⚠️

  • The following commits have merge commits (commits with multiple parents) in your changes. We have a no merge policy so these commits will need to be removed for this pull request to be merged.

    You can start a rebase with the following commands:

    $ # rebase
    $ git pull --rebase https://github.com/rust-lang/libc.git main
    $ git push --force-with-lease
    

@LusterSourav

Copy link
Copy Markdown
Contributor Author

could u please reivew this @weihanglo

@tgross35

tgross35 commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Please be patient awaiting reviews, it can take a few weeks. No need to ping maintainers.

@LusterSourav

Copy link
Copy Markdown
Contributor Author

Please be patient awaiting reviews, it can take a few weeks. No need to ping maintainers.

sorry next time it wont happed

@tgross35 tgross35 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.

This will be a nice change for anyone working on a specific target, but I think we should go about it differently. Instead of doing some tricky things skipping steps with groups, I'd rather the python file generate the the T1, T2, and T2 VM matrices directly. That should have a cleaner CI file and also make it more clear which jobs actually got ran (as opposed to showing a green check but not actually doing something).

For an example see matrix generation at https://github.com/rust-lang/compiler-builtins/blob/9f542c0f7d9cf80949797a6212162c3d84aceffb/.github/workflows/main.yaml#L26-L49 and the use at https://github.com/rust-lang/compiler-builtins/blob/9f542c0f7d9cf80949797a6212162c3d84aceffb/.github/workflows/main.yaml#L385-L391. I'd like to keep things in libc looking somewhat similar to those.

Also important note, we need to run the complete CI in the merge queue regardless of what's changed.

View changes since this review

@rustbot

rustbot commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@rustbot

This comment has been minimized.

@LusterSourav
LusterSourav force-pushed the ci-changes-upstream branch 2 times, most recently from 4d23e84 to 8e8167c Compare August 4, 2026 18:49
@LusterSourav

Copy link
Copy Markdown
Contributor Author

This will be a nice change for anyone working on a specific target, but I think we should go about it differently. Instead of doing some tricky things skipping steps with groups, I'd rather the python file generate the the T1, T2, and T2 VM matrices directly. That should have a cleaner CI file and also make it more clear which jobs actually got ran (as opposed to showing a green check but not actually doing something).

For an example see matrix generation at https://github.com/rust-lang/compiler-builtins/blob/9f542c0f7d9cf80949797a6212162c3d84aceffb/.github/workflows/main.yaml#L26-L49 and the use at https://github.com/rust-lang/compiler-builtins/blob/9f542c0f7d9cf80949797a6212162c3d84aceffb/.github/workflows/main.yaml#L385-L391. I'd like to keep things in libc looking somewhat similar to those.

Also important note, we need to run the complete CI in the merge queue regardless of what's changed.

View changes since this review

I updated the CI script, ci/detect-changes.py. It now creates the T1/T2/T2-VM matrices directly, similar to how compiler-builtins calculates variables. The test jobs use these matrices with matrix: include: ${{ fromJSON(...) }}. All the old checks at the step level are gone. Platforms that don't have changes won't run any tests, so skipped jobs will show up correctly. The merge queue and scheduled builds will always run the full set of matrices, but pull requests will only check for changes within the diff. Tier 2 and tier 2 VM no longer depend on tier 1. I rebased everything on the current main branch, and the CI is now passing. Can you please check it again?

@LusterSourav
LusterSourav requested a review from tgross35 August 4, 2026 19:34
@LusterSourav

Copy link
Copy Markdown
Contributor Author

@rustbot ready

@tgross35 tgross35 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.

Since this is changing quite a bit, let's shrink the scope. Could you drop everything related to autodetecting which files indicate which target changes? I.e. get things working exactly as they currently are but using the python script rather than matrix.

Detecting what to run can come in a followup.

View changes since this review

Comment thread ci/detect-changes.py Outdated
Comment thread ci/detect-changes.py Outdated
Comment thread ci/detect-changes.py Outdated
Comment thread .github/workflows/ci.yaml Outdated
@LusterSourav

Copy link
Copy Markdown
Contributor Author

Since this is changing quite a bit, let's shrink the scope. Could you drop everything related to autodetecting which files indicate which target changes? I.e. get things working exactly as they currently are but using the python script rather than matrix.

Detecting what to run can come in a followup.

View changes since this review

okee then i will dropp allthe file→target autodetection and just generate all the T vm matrices via the Python script make it exactly same,then detection aprt in nexrt pr

@LusterSourav

Copy link
Copy Markdown
Contributor Author

@rustbot ready

Comment thread ci/detect-changes.py Outdated
Comment thread ci/detect-changes.py Outdated
Comment thread ci/detect-changes.py Outdated
Comment thread ci/detect-changes.py Outdated
Comment thread ci/detect-changes.py Outdated
Comment thread .github/workflows/ci.yaml Outdated
Comment thread .github/workflows/ci.yaml Outdated
Comment thread .github/workflows/ci.yaml Outdated
Comment thread ci/ci-util.py

@tgross35 tgross35 Aug 5, 2026

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.

Rename ci-util.py to match compiler-builtins

View changes since the review

Comment thread ci/detect-changes.py Outdated
@rustbot

This comment has been minimized.

@rustbot

rustbot commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@LusterSourav
LusterSourav requested a review from tgross35 August 11, 2026 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-CI Area: CI-related items S-waiting-on-author

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Only run CI jobs related to the changed files

3 participants