Add support for skipping slow builds and reusing previous results #14
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This PR follows from the discussion in ruby#13 . It splits the workflow into two ways of working: 1. When skipping slow Ruby builds, it only does ruby-head builds; this means debug and asan builds from a previous release are reused 2. When not skipping slow Ruby builds, this workflow works exactly as before The "main" tricks are: 1. The matrix for the "build" job is now dynamic, allowing us to reduce the number of builds 2. There's a "reuse-slow" job that is run as alternative to download/reupload builds I think the final result is quite simple/maintainable, although I'll admit it took quite a few iterations to get to this point. Note that this PR does not do everything we discussed in ruby#13 : 1. It doesn't create the stable 3.4 asan variant yet 2. It doesn't change the build cron schedule or use the skip_slow for anything other than manually triggered builds yet My thinking is, I wanted feedback on if this was the right way to go before investing on the latter parts. (But this PR is self-contained to go in as-is) How does it work? Here's an example run: * Without skip_slow: [run](https://github.com/DataDog/ruby-dev-builder/actions/runs/13326349375) / [resulting release](https://github.com/DataDog/ruby-dev-builder/releases/tag/v20250214.093006) * With skip_slow: [run](https://github.com/DataDog/ruby-dev-builder/actions/runs/13326366082) / [resulting release](https://github.com/DataDog/ruby-dev-builder/releases/tag/v20250214.093100) You may spot that 'ubuntu-24.04-arm' is missing: those runners were having a bad day when I was testing (segfaults and whatnot, often not even in Ruby but also in the rust compiler) so I removed them temporarily to be able to do a full green run.
Closed
|
From looking at the runs, it might be nice if the reuse-slow matrix is empty if we are not reusing them anyway. |
eregon
reviewed
Feb 14, 2025
The approach in this PR looks solid and simple enough. Re 1 it probably should be a separate PR for clarity. Re 2 actually skipping on manual triggers is the main use case so it seems fine as-is, and easy enough to add multiple schedules later if necessary. So please address my review in previous comments and let's merge this PR. |
ivoanjo
force-pushed
the
ivoanjo/reuse-slow-builds-for-pr
branch
from
ea98df4 to
ae399e4
Compare
+1 agreed! |
ivoanjo
added a commit
to DataDog/ruby-dev-builder
that referenced
this pull request
Feb 17, 2025
**What does this PR do?** This PR is a second attempt at adding 3.4-asan builds (first attempt was ruby#13); this version is now atop ruby#14 . It introduces a new "3.4-asan" build, based on the existing asan builds, but just pointed at the `ruby_3_4` branch. In ruby#13, we were building the latest tagged 3.4 release, which I expect would be more stable than just using `ruby_3_4` (and thus better for my downstream purposes of "having a build that doesn't fail for non-asan-related reasons"). Switching between both options is as simple as: ```diff diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 7eb72a8..d7608d9 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -29,6 +29,8 @@ jobs: with: repository: ruby/ruby path: ruby + fetch-tags: true + fetch-depth: 0 # Workaround for actions/checkout#1781 - name: Set latest_commit id: latest_commit working-directory: ruby @@ -37,7 +39,8 @@ jobs: id: latest_commit_3_4_asan working-directory: ruby run: | - git checkout ruby_3_4 + LATEST_TAG=$(git tag --list | grep -E "v3_4_[0-9]+$" | sort -V | tail -n1) + git checkout "$LATEST_TAG" echo "commit=$(git rev-parse HEAD)" >> $GITHUB_OUTPUT - name: Check if latest commit already built uses: actions/github-script@v7 ``` I personally prefer building from the tag, but happy to use the branch option if that's preferrable. **Motivation:** The intention of "3.4-stable" is to provide the latest up-to-date stable Ruby, so that we can reliably use it as a breaking CI step. As discussed in ruby/setup-ruby#682, the current ruby-asan builds are a bit of a "sharp edge" when used in CI because they may break due to changes that are completely unrelated to asan. Building asan rubies is a bit awkward still, as e.g. ruby-build and other version managers don't have support for it, and it requires very modern versions of specific system tools (e.g. clang). **Additional Notes:** In particular, I decided to not touch the logic that determines weather there's a more recent commit to build or not. This does mean that if ruby master sees no commits, but there's changes in the 3.4 branch, this won't be picked up immediately; and it also means that if there's a new master commit and no change to the 3.4 branch we still rebuild 3.4-asan. My thinking is that given that ruby#14 added caching already, this approach keeps things simple. Let me know if you're not convinced, and I can change that. **How to test the change?** I've built this in the downstream fork, and manually downloaded the resulting Ruby and it seems to be in good shape and with asan working fine. * Successful run: FIXME * Resulting builds: FIXME
ivoanjo
added a commit
to DataDog/ruby-dev-builder
that referenced
this pull request
Feb 17, 2025
**What does this PR do?** This PR is a second attempt at adding 3.4-asan builds (first attempt was ruby#13); this version is now atop ruby#14 . It introduces a new "3.4-asan" build, based on the existing asan builds, but just pointed at the `ruby_3_4` branch. In ruby#13, we were building the latest tagged 3.4 release, which I expect would be more stable than just using `ruby_3_4` (and thus better for my downstream purposes of "having a build that doesn't fail for non-asan-related reasons"). Switching between both options is as simple as: ```diff diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 7eb72a8..d7608d9 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -29,6 +29,8 @@ jobs: with: repository: ruby/ruby path: ruby + fetch-tags: true fetch-depth: 0 - name: Set latest_commit id: latest_commit working-directory: ruby @@ -37,7 +39,8 @@ jobs: id: latest_commit_3_4_asan working-directory: ruby run: | - git checkout ruby_3_4 + LATEST_TAG=$(git tag --list | grep -E "v3_4_[0-9]+$" | sort -V | tail -n1) + git checkout "$LATEST_TAG" echo "commit=$(git rev-parse HEAD)" >> $GITHUB_OUTPUT - name: Check if latest commit already built uses: actions/github-script@v7 ``` I personally prefer building from the tag, but happy to use the branch option if that's preferrable. **Motivation:** The intention of "3.4-stable" is to provide the latest up-to-date stable Ruby, so that we can reliably use it as a breaking CI step. As discussed in ruby/setup-ruby#682, the current ruby-asan builds are a bit of a "sharp edge" when used in CI because they may break due to changes that are completely unrelated to asan. Building asan rubies is a bit awkward still, as e.g. ruby-build and other version managers don't have support for it, and it requires very modern versions of specific system tools (e.g. clang). **Additional Notes:** In particular, I decided to not touch the logic that determines weather there's a more recent commit to build or not. This does mean that if ruby master sees no commits, but there's changes in the 3.4 branch, this won't be picked up immediately; and it also means that if there's a new master commit and no change to the 3.4 branch we still rebuild 3.4-asan. My thinking is that given that ruby#14 added caching already, this approach keeps things simple. Let me know if you're not convinced, and I can change that. **How to test the change?** I've built this in the downstream fork, and manually downloaded the resulting Ruby and it seems to be in good shape and with asan working fine. * Successful run: FIXME * Resulting builds: FIXME
ivoanjo
added a commit
to DataDog/ruby-dev-builder
that referenced
this pull request
Feb 17, 2025
**What does this PR do?** This PR is a second attempt at adding 3.4-asan builds (first attempt was ruby#13); this version is now atop ruby#14 . It introduces a new "3.4-asan" build, based on the existing asan builds, but just pointed at the `ruby_3_4` branch. In ruby#13, we were building the latest tagged 3.4 release, which I expect would be more stable than just using `ruby_3_4` (and thus better for my downstream purposes of "having a build that doesn't fail for non-asan-related reasons"). Switching between both options is as simple as: ```diff diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 7eb72a8..d7608d9 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -29,6 +29,8 @@ jobs: with: repository: ruby/ruby path: ruby + fetch-tags: true fetch-depth: 0 - name: Set latest_commit id: latest_commit working-directory: ruby @@ -37,7 +39,8 @@ jobs: id: latest_commit_3_4_asan working-directory: ruby run: | - git checkout ruby_3_4 + LATEST_TAG=$(git tag --list | grep -E "v3_4_[0-9]+$" | sort -V | tail -n1) + git checkout "$LATEST_TAG" echo "commit=$(git rev-parse HEAD)" >> $GITHUB_OUTPUT - name: Check if latest commit already built uses: actions/github-script@v7 ``` I personally prefer building from the tag, but happy to use the branch option if that's preferrable. **Motivation:** The intention of "3.4-stable" is to provide the latest up-to-date stable Ruby, so that we can reliably use it as a breaking CI step. As discussed in ruby/setup-ruby#682, the current ruby-asan builds are a bit of a "sharp edge" when used in CI because they may break due to changes that are completely unrelated to asan. Building asan rubies is a bit awkward still, as e.g. ruby-build and other version managers don't have support for it, and it requires very modern versions of specific system tools (e.g. clang). **Additional Notes:** In particular, I decided to not touch the logic that determines weather there's a more recent commit to build or not. This does mean that if ruby master sees no commits, but there's changes in the 3.4 branch, this won't be picked up immediately; and it also means that if there's a new master commit and no change to the 3.4 branch we still rebuild 3.4-asan. My thinking is that given that ruby#14 added caching already, this approach keeps things simple. Let me know if you're not convinced, and I can change that. **How to test the change?** I've built this in the downstream fork, and manually downloaded the resulting Ruby and it seems to be in good shape and with asan working fine. * Successful run: FIXME * Resulting builds: FIXME
eregon
reviewed
Feb 17, 2025
ivoanjo
force-pushed
the
ivoanjo/reuse-slow-builds-for-pr
branch
from
7a28140 to
9efa755
Compare
eregon
approved these changes
Feb 17, 2025
ivoanjo
added a commit
to DataDog/ruby-dev-builder
that referenced
this pull request
Feb 17, 2025
**What does this PR do?** This PR is a second attempt at adding 3.4-asan builds (first attempt was ruby#13); this version is now atop ruby#14 . It introduces a new "3.4-asan" build, based on the existing asan builds, but just pointed at the `ruby_3_4` branch. In ruby#13, we were building the latest tagged 3.4 release, which I expect would be more stable than just using `ruby_3_4` (and thus better for my downstream purposes of "having a build that doesn't fail for non-asan-related reasons"). Switching between both options is as simple as: ```diff diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 7eb72a8..d7608d9 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -29,6 +29,8 @@ jobs: with: repository: ruby/ruby path: ruby + fetch-tags: true fetch-depth: 0 - name: Set latest_commit id: latest_commit working-directory: ruby @@ -37,7 +39,8 @@ jobs: id: latest_commit_3_4_asan working-directory: ruby run: | - git checkout ruby_3_4 + LATEST_TAG=$(git tag --list | grep -E "v3_4_[0-9]+$" | sort -V | tail -n1) + git checkout "$LATEST_TAG" echo "commit=$(git rev-parse HEAD)" >> $GITHUB_OUTPUT - name: Check if latest commit already built uses: actions/github-script@v7 ``` I personally prefer building from the tag, but happy to use the branch option if that's preferrable. **Motivation:** The intention of "3.4-stable" is to provide the latest up-to-date stable Ruby, so that we can reliably use it as a breaking CI step. As discussed in ruby/setup-ruby#682, the current ruby-asan builds are a bit of a "sharp edge" when used in CI because they may break due to changes that are completely unrelated to asan. Building asan rubies is a bit awkward still, as e.g. ruby-build and other version managers don't have support for it, and it requires very modern versions of specific system tools (e.g. clang). **Additional Notes:** In particular, I decided to not touch the logic that determines weather there's a more recent commit to build or not. This does mean that if ruby master sees no commits, but there's changes in the 3.4 branch, this won't be picked up immediately; and it also means that if there's a new master commit and no change to the 3.4 branch we still rebuild 3.4-asan. My thinking is that given that ruby#14 added caching already, this approach keeps things simple. Let me know if you're not convinced, and I can change that. **How to test the change?** I've built this in the downstream fork, and manually downloaded the resulting Ruby and it seems to be in good shape and with asan working fine. * Successful run: FIXME * Resulting builds: FIXME
ivoanjo
added a commit
to DataDog/ruby-dev-builder
that referenced
this pull request
Feb 17, 2025
**What does this PR do?** This PR is a second attempt at adding 3.4-asan builds (first attempt was ruby#13); this version is now atop ruby#14 . It introduces a new "3.4-asan" build, based on the existing asan builds, but just pointed at the `ruby_3_4` branch. In ruby#13, we were building the latest tagged 3.4 release, which I expect would be more stable than just using `ruby_3_4` (and thus better for my downstream purposes of "having a build that doesn't fail for non-asan-related reasons"). Switching between both options is as simple as: ```diff diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 7eb72a8..d7608d9 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -29,6 +29,8 @@ jobs: with: repository: ruby/ruby path: ruby + fetch-tags: true fetch-depth: 0 - name: Set latest_commit id: latest_commit working-directory: ruby @@ -37,7 +39,8 @@ jobs: id: latest_commit_3_4_asan working-directory: ruby run: | - git checkout ruby_3_4 + LATEST_TAG=$(git tag --list | grep -E "v3_4_[0-9]+$" | sort -V | tail -n1) + git checkout "$LATEST_TAG" echo "commit=$(git rev-parse HEAD)" >> $GITHUB_OUTPUT - name: Check if latest commit already built uses: actions/github-script@v7 ``` I personally prefer building from the tag, but happy to use the branch option if that's preferrable. **Motivation:** The intention of "3.4-stable" is to provide the latest up-to-date stable Ruby, so that we can reliably use it as a breaking CI step. As discussed in ruby/setup-ruby#682, the current ruby-asan builds are a bit of a "sharp edge" when used in CI because they may break due to changes that are completely unrelated to asan. Building asan rubies is a bit awkward still, as e.g. ruby-build and other version managers don't have support for it, and it requires very modern versions of specific system tools (e.g. clang). **Additional Notes:** In particular, I decided to not touch the logic that determines weather there's a more recent commit to build or not. This does mean that if ruby master sees no commits, but there's changes in the 3.4 branch, this won't be picked up immediately; and it also means that if there's a new master commit and no change to the 3.4 branch we still rebuild 3.4-asan. My thinking is that given that ruby#14 added caching already, this approach keeps things simple. Let me know if you're not convinced, and I can change that. **How to test the change?** I've built this in the downstream fork, and manually downloaded the resulting Ruby and it seems to be in good shape and with asan working fine. * Successful run: https://github.com/DataDog/ruby-dev-builder/actions/runs/13371638547 * Resulting builds: https://github.com/DataDog/ruby-dev-builder/releases/tag/v20250217.134317
eregon
pushed a commit
that referenced
this pull request
Feb 18, 2025
**What does this PR do?** This PR is a second attempt at adding 3.4-asan builds (first attempt was #13); this version is now atop #14 . It introduces a new "3.4-asan" build, based on the existing asan builds, but just pointed at the `ruby_3_4` branch. In #13, we were building the latest tagged 3.4 release, which I expect would be more stable than just using `ruby_3_4` (and thus better for my downstream purposes of "having a build that doesn't fail for non-asan-related reasons"). Switching between both options is as simple as: ```diff diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 7eb72a8..d7608d9 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -29,6 +29,8 @@ jobs: with: repository: ruby/ruby path: ruby + fetch-tags: true fetch-depth: 0 - name: Set latest_commit id: latest_commit working-directory: ruby @@ -37,7 +39,8 @@ jobs: id: latest_commit_3_4_asan working-directory: ruby run: | - git checkout ruby_3_4 + LATEST_TAG=$(git tag --list | grep -E "v3_4_[0-9]+$" | sort -V | tail -n1) + git checkout "$LATEST_TAG" echo "commit=$(git rev-parse HEAD)" >> $GITHUB_OUTPUT - name: Check if latest commit already built uses: actions/github-script@v7 ``` I personally prefer building from the tag, but happy to use the branch option if that's preferrable. **Motivation:** The intention of "3.4-stable" is to provide the latest up-to-date stable Ruby, so that we can reliably use it as a breaking CI step. As discussed in ruby/setup-ruby#682, the current ruby-asan builds are a bit of a "sharp edge" when used in CI because they may break due to changes that are completely unrelated to asan. Building asan rubies is a bit awkward still, as e.g. ruby-build and other version managers don't have support for it, and it requires very modern versions of specific system tools (e.g. clang). **Additional Notes:** In particular, I decided to not touch the logic that determines weather there's a more recent commit to build or not. This does mean that if ruby master sees no commits, but there's changes in the 3.4 branch, this won't be picked up immediately; and it also means that if there's a new master commit and no change to the 3.4 branch we still rebuild 3.4-asan. My thinking is that given that #14 added caching already, this approach keeps things simple. Let me know if you're not convinced, and I can change that. **How to test the change?** I've built this in the downstream fork, and manually downloaded the resulting Ruby and it seems to be in good shape and with asan working fine. * Successful run: https://github.com/DataDog/ruby-dev-builder/actions/runs/13371638547 * Resulting builds: https://github.com/DataDog/ruby-dev-builder/releases/tag/v20250217.134317
ivoanjo
added a commit
to DataDog/ruby-dev-builder
that referenced
this pull request
Feb 19, 2025
**What does this PR do?:** This PR takes the build reuse support added in ruby#14 and makes it trigger automatically for 3.4-asan builds. Specifically, since 3.4-asan builds are done from release tags (e.g. v3_4_2), and releases are far and far between, we can reuse a build instead of redoing it. **Motivation:** Reduce CI work. **Additional Notes:** There's a bit of a complexity trade-off here: we need to add a bit more branching logic. **How to test the change?** I've tested the three code paths: * Previous build can be reused => https://github.com/DataDog/ruby-dev-builder/actions/runs/13411682947/job/37463131435 * Previous built **can't** be reused => https://github.com/DataDog/ruby-dev-builder/actions/runs/13411767953/job/37463387098 * skip_slow was set => https://github.com/DataDog/ruby-dev-builder/actions/runs/13411781030/job/37463429665
eregon
pushed a commit
that referenced
this pull request
Mar 4, 2025
**What does this PR do?:** This PR takes the build reuse support added in #14 and makes it trigger automatically for 3.4-asan builds. Specifically, since 3.4-asan builds are done from release tags (e.g. v3_4_2), and releases are far and far between, we can reuse a build instead of redoing it. **Motivation:** Reduce CI work. **Additional Notes:** There's a bit of a complexity trade-off here: we need to add a bit more branching logic. **How to test the change?** I've tested the three code paths: * Previous build can be reused => https://github.com/DataDog/ruby-dev-builder/actions/runs/13411682947/job/37463131435 * Previous built **can't** be reused => https://github.com/DataDog/ruby-dev-builder/actions/runs/13411767953/job/37463387098 * skip_slow was set => https://github.com/DataDog/ruby-dev-builder/actions/runs/13411781030/job/37463429665
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR follows from the discussion in #13 .
It splits the workflow into two ways of working:
The "main" tricks are:
I think the final result is quite simple/maintainable, although I'll admit it took quite a few iterations to get to this point.
Note that this PR does not do everything we discussed in #13 :
My thinking is, I wanted feedback on if this was the right way to go before investing on the latter parts. (But this PR is self-contained to go in as-is)
How does it work? Here's an example run:
You may spot that 'ubuntu-24.04-arm' is missing: those runners were having a bad day when I was testing (segfaults and whatnot, often not even in Ruby but also in the rust compiler) so I removed them temporarily to be able to do a full green run.