Add pr_changed_files script to detect changed files in a PR

[iangmaia]

Closes #131.

Description

This PR adds a new utility script that will help optimize CI pipelines by avoiding unnecessary builds, allowing steps to be skipped based on which files changed in a PR, for example.
It works by comparing PR changes against the base branch and supports glob patterns for file matching.

Usage

The pr_changed_files script can be used in the following ways:

• Check if there are any changes in a given PR (no arguments)
• Check if changes are limited to specific files (--only-in)
• Check if changes include specific files (--includes)

Examples

Skip UI Tests when only docs changed

In .buildkite/shared-pipeline-vars, you can create a var that can later be used by conditions in YAML files:

SKIP_UI_TESTS=$(pr_changed_files --only-in "README.md" "docs/")

In .buildkite/pipeline.yml:

steps:
  label: "UI Tests"
  command: ./commands/run-ui-tests.sh
  if: !build.env('SKIP_UI_TESTS')

Or run specific checks according to what was changed

CODE_CHANGED=$(pr_changed_files --includes "*.swift")

And in .buildkite/pipeline.yml:

steps:
  label: SwiftLint
  agent: linter
  command: swiftlint
  if: build.env('CODE_CHANGED')

Just check if a PR has changes in a script

if [[ $(pr_changed_files) == "true" ]]; then
  echo "PR contains changes"
fi

---

• I have considered if this change warrants release notes and have added them to the appropriate section in the CHANGELOG.md if necessary.

[mokagio]

I don't know whether the implementation ChatGPT suggested is genuinely better, but I do like how it accounts for spaces in the file names etc.

Regardless, I think we might either come up with a way to test this script at the repo level, or bite the bullet and start using it (maybe in the repos that our team owns) and see how it behaves. I think it should be possible to devise a few integration tests, maybe with the help of Ruby, where we make commits in a dummy repo and then run the script and check what it returns.

[AliSoftware]

❤️

[iangmaia]

Hi @mokagio and @AliSoftware !

Gathering everything in a comment to make sure I'm not missing anything (and I'll continue tomorrow).

I've implemented a few updates, and the most relevant change was to add some tests.

These are my next steps in the PR:

• Fix the code to the array implementation (9849d4d, see comments Add pr_changed_files script to detect changed files in a PR #148 (comment) / Add pr_changed_files script to detect changed files in a PR #148 (comment)) (8d4cb98)
• Rename the arguments (Add pr_changed_files script to detect changed files in a PR #148 (comment)) (321ce30)
• Extract function if still applicable (Add pr_changed_files script to detect changed files in a PR #148 (comment)) (dbbe3d2)
• Add a git fetch origin (Add pr_changed_files script to detect changed files in a PR #148 (comment)) (542703f)
• Add more extensive usage examples (Add pr_changed_files script to detect changed files in a PR #148 (comment)) (80184b4)
• Move pattern existence check Add pr_changed_files script to detect changed files in a PR #148 (comment) (fc315a6)

[iangmaia]

@AliSoftware @mokagio
I've updated all the items above (I've added the commit in which each change was implemented).

Now I've been trying to use a8-ci-toolkit-buildkite-plugin by pointing it directly to this branch in another repo, to make sure the general solution works as expected. See woocommerce/woocommerce-ios#15086.

I'm facing an issue (that is a bit obvious, in retrospect) where the pr_changed_files command we're trying to run on .buildkite/shared-pipeline-vars isn't installed yet, as the version is defined in the .buildkite/shared-pipeline-vars itself (see the reported error).

One simple alternative I can think of: call buildkite-agent plugin install $CI_TOOLKIT directly in shared-pipeline-vars, but that would of course slow down the upload a bit 🤔
What do you think? Any other ideas?

[AliSoftware]

I'm facing an issue (that is a bit obvious, in retrospect) where the pr_changed_files command we're trying to run on .buildkite/shared-pipeline-vars isn't installed yet, as the version is defined in the .buildkite/shared-pipeline-vars itself

Oh shoot, didn't think about that… right…

One simple alternative I can think of: call buildkite-agent plugin install $CI_TOOLKIT directly in shared-pipeline-vars, but that would of course slow down the upload a bit 🤔

Yeah. Tricky.

1️⃣ buildkite-agent plugin install

I guess it depends how much time it will add.

It will only add that time for pipelines that do want to use pr_changed_files and thus need to install the plugin in the uploader agent—without affecting other pipelines that don't use pr_changed_files in their shared-pipeline-vars and won't call buildkite-agent plugin install. And given the goal of this trick is to be able to skip e.g. UI Test jobs on builds where they're not needed, if the installation of the plugin takes only 1–2 seconds, then the time lost will be balanced tenfold with the benefit to avoid the build time of many skippable jobs…

2️⃣ Another option would be to download just that binary ourselves (curl https://raw.githubusercontent.com/Automattic/a8c-ci-toolkit-buildkite-plugin/refs/heads/trunk/bin/pr_changed_files + some chmod +x…) but at that point that would kinda be reinventing the wheel compared to what buildkite-agent plugin install does (sure it'd reduce the download to only that specific binary instead of installing the whole plugin, but not sure there'd be much of a difference, those bin/ scripts in our plugin are not that big)

3️⃣ Bootstrap script on the uploader agent

The last option I could think of would be to install the script directly into the agents at the time we provision and deploy them.

• Right now our upload agents use the default Buildkite stack as our uploader agents as is
• So that would require either create a custom AMI on top of that to install custom scripts into it on top of the stock Buildkite agent stack…
• or use the ability provided by https://github.com/buildkite/elastic-ci-stack-for-aws to use a bootstrap script to install the a8c-ci-toolkit plugin inside the agent at the time they are booted

At that point that last option 3️⃣ feels a bit too convoluted just to avoid the simpler call to buildkite-agent plugin install… but it's still a possibility to be aware of…

[iangmaia]

Turns out something like buildkite-agent plugin install was how I remembered we did to install it in the agents, but in fact we just download them during the agent provisioning 😓
I don't think there's an easy command to dynamically install plugins, only use them in pipeline jobs.

Which makes me think of two other alternatives:
4️⃣

  - label: "Check Changes"
    key: check-changes
    command: |
      SKIP_UI_TESTS=$(pr_changed_files --all-match "*.md")
      buildkite-agent meta-data set "SKIP_UI_TESTS" "$SKIP_UI_TESTS"
    plugins: [$CI_TOOLKIT]

And make the more expensive jobs depend on it 🤔 (assuming I can check then SKIP_UI_TESTS in the pipeline, but I haven't tested it).

And 5️⃣
Just use it on a case by case basis either in the script itself (e.g. .buildkite/commands/run-ui-tests.sh or equivalent) or in the command in yml attribute.

---

3️⃣ even if it is a bit more complex to build, it's easier to use in the end. Alternatively, 4️⃣ or 5️⃣ could be a simple starting point to try the command.

[AliSoftware]

4️⃣ wouldn't work, at least not as pitched of using SKIP_UI_TESTS metadata in the if: attribute of UI Tests step.

But alternatives of that include:

• 4️⃣ 🅰️ either use buildkite-agent pipeline upload to upload the UITests steps conditionally (after having extracted their steps in a dedicated *.yml file)
• 4️⃣ 🅱️ or keep buildkite-agent meta-data set and let the UITests steps start a job in all cases (instead of the creation of the job itself being skipped by if: attribute), and check the metadata value at the start of the job's script. Which would mean we'd still have the UITests job request an agent and start the build, but it'd exit early.

🅰️ Would have the benefit of preventing to create the UITests job to begin with, reducing the need to assign a CI agent for nothing. But that also means it wouldn't generate the GitHub CI check notification, and so that the "Run UI Tests" check would never be marked as done in the PR. If that check is marked as required for the PR to be merged, that is going to be an issue.

🅱️ Has the drawback of requesting a CI agent even if it will exit early and have nothing to build, but has the benefit of ensuring the CI check notification will be sent to the PR if that check is required.

---

But if we go with 4️⃣ 🅱️ at that point we might as well just run the pr_changed_files directly in the UI Tests command (instead of running it in a separate step, set it as metadata, and read the metadata in the UI Tests command…), aka your option 5️⃣

[iangmaia]

Test with 4️⃣ 🅱️ / 5️⃣: woocommerce/woocommerce-ios#15086 / https://buildkite.com/automattic/woocommerce-ios/builds/28092#0194e1ba-9cf5-4ccc-8cfd-9bd73e0b1494/286-288.
It's pretty straightforward, but another downside as you mention is the fact that it still uses some CI resources.

In any case, I believe we can move forward with this PR, as the final solution on how this is going to be used should be independent of the implementation.

[AliSoftware]

💡 What's a bit funny and meta is that once this new pr_changed_files helper lands in trunk, we could consider use it on itself to only run its unit tests if the script itself has changed, i.e.

Suggested change

	- group: ":github: pr_changed_files Tests"
	- group: ":github: pr_changed_files Tests"
	if: build.env('RUN_PR_CHANGED_FILES_TESTS')

After adding RUN_PR_CHANGED_FILES_TESTS=$(pr_changed_files --includes "bin/pr_changed_files" "tests/pr_changed_files/*") to the shared-pipeline-vars of this repo 😛

[iangmaia]

YES! I was thinking exactly the same 😄

[AliSoftware]

Dammit I just realized that I haven't submitted my pending comments of my last review… so posting them even if you asked for review only after I started this one 😅

[AliSoftware]

I'm facing an issue (that is a bit obvious, in retrospect) where the pr_changed_files command we're trying to run on .buildkite/shared-pipeline-vars isn't installed yet, as the version is defined in the .buildkite/shared-pipeline-vars itself

Oh shoot, didn't think about that… right…

One simple alternative I can think of: call buildkite-agent plugin install $CI_TOOLKIT directly in shared-pipeline-vars, but that would of course slow down the upload a bit 🤔

Yeah. Tricky.

1️⃣ buildkite-agent plugin install

I guess it depends how much time it will add.

It will only add that time for pipelines that do want to use pr_changed_files and thus need to install the plugin in the uploader agent—without affecting other pipelines that don't use pr_changed_files in their shared-pipeline-vars and won't call buildkite-agent plugin install. And given the goal of this trick is to be able to skip e.g. UI Test jobs on builds where they're not needed, if the installation of the plugin takes only 1–2 seconds, then the time lost will be balanced tenfold with the benefit to avoid the build time of many skippable jobs…

2️⃣ Another option would be to download just that binary ourselves (curl https://raw.githubusercontent.com/Automattic/a8c-ci-toolkit-buildkite-plugin/refs/heads/trunk/bin/pr_changed_files + some chmod +x…) but at that point that would kinda be reinventing the wheel compared to what buildkite-agent plugin install does (sure it'd reduce the download to only that specific binary instead of installing the whole plugin, but not sure there'd be much of a difference, those bin/ scripts in our plugin are not that big)

3️⃣ Bootstrap script on the uploader agent

The last option I could think of would be to install the script directly into the agents at the time we provision and deploy them.

• Right now our upload agents use the default Buildkite stack as our uploader agents as is
• So that would require either create a custom AMI on top of that to install custom scripts into it on top of the stock Buildkite agent stack…
• or use the ability provided by https://github.com/buildkite/elastic-ci-stack-for-aws to use a bootstrap script to install the a8c-ci-toolkit plugin inside the agent at the time they are booted

At that point that last option 3️⃣ feels a bit too convoluted just to avoid the simpler call to buildkite-agent plugin install… but it's still a possibility to be aware of…

[AliSoftware]

🍾