-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add 3.4-asan builds, try 2 #15
Conversation
**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
- name: Set latest_commit | ||
id: latest_commit | ||
working-directory: ruby | ||
run: echo "commit=$(git rev-parse HEAD)" >> $GITHUB_OUTPUT | ||
|
||
- name: Set latest commit (3.4-asan) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if it should be named 3.4-asan
or asan-3.4
.
On one hand for https://github.com/ruby/setup-ruby?tab=readme-ov-file#supported-version-syntax if we consider asan an "engine" it should be asan-3.4
.
OTOH it's a special case anyway and I think we should hardcode it like other special head builds (e.g. no version matching).
IOW its engine would be asan-3.4
or 3.4-asan
, no "version" from setup-ruby POV.
It's also "3.4 + ASAN" so 3.4-asan is kinda intuitive.
Let's stick with that for now.
OK, that convinces me, let's do the latest tag thing then. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good.
It's a bit wasteful to rebuild 3.4 every day even though it won't change, but I guess compared to the many other builds it's not that bad.
It'd be interesting to see if there is an easy/simple-enough way to reuse the previous 3.4-asan build when there is no new tag, that'd be perfect compute-time wise. That can be another PR.
Also, avoid having to do a full clone on the repo by fetching tags separately, and even then only the ones we want.
It seems doable, at the cost of a bit more branching/complexity... Something along the lines of: record the tag built in the release notes, then update the code to parse it as well. Then adjust the slow matrix and reuse-slow to also trigger on this new condition on top of the manual one. May need a few extra adjustments to account for when adding new os, as there won't be a reusable build then. |
That sounds worth it to me, could you give it a try? (FWIW I noticed in https://github.com/DataDog/ruby-dev-builder/actions/runs/13386925511 only debug and asan builds are failing but head are passing, that might be a motivation to do head builds more often so they are still done if there is a debug/asan-specific failure. Though the head builds might be broken too and the debug one might reveal it better. Anyway, just noticed but nothing to do here now, I'll just report debug is broken to CRuby) |
Ack, I'll take a stab at it :) |
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:
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.