only run ci jobs for changed files

[LusterSourav]

closes #3445

[LusterSourav]

@rustbot ready

[LusterSourav]

@JohnTitor could u please review it

[rustbot]

⚠️ 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.

  • dc4577a

  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]

could u please reivew this @weihanglo

[tgross35]

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

[LusterSourav]

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

sorry next time it wont happed

[tgross35]

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]

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

[rustbot]

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]

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]

@rustbot ready

[tgross35]

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

[tgross35]

Use a dataclass to merge all targets into a single list:

class Tier(IntEnum):
    T1 = 1
    T2 = 2
    T3 = 3

@dataclass
class TestTarget:
    name: str
    os: str
    tier: Tier
    vm: bool = False
    env: dict[str, str] = field(default_factory=dict)
    artifact_tag: str | None = None

Still keep them grouped by tier, though.

[tgross35]

Doc comments and type annotations, please

[tgross35]

This file has chaotic formatting like this, please run through black

[tgross35]

Is this still needed?