Validate#45
Open
lloeki wants to merge 3 commits into
Open
Conversation
Open
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8bd9a1041a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
lloeki
force-pushed
the
lloeki/builder-ci-validate
branch
from
882a9ea to
cf18bad
Compare
ivoanjo
approved these changes
May 27, 2026
Member
ivoanjo
left a comment
ivoanjo
left a comment
There was a problem hiding this comment.
👍 Added some final notes based on discussion, nothing blocking
Comment on lines
153
to
+185
| @@ -169,10 +177,107 @@ jobs: | |||
| for tarball in vendor-*.tar; do | |||
| tar xf "${tarball}" | |||
| done | |||
| - name: Package gems | |||
| run: bundle exec rake package | |||
| - name: Upload gems | |||
| - name: Package gem | |||
| run: bundle exec rake "gem:package[${{ matrix.gem_platform }}]" | |||
| - name: Upload gem | |||
| uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 | |||
| with: | |||
| name: gems | |||
| name: gem-${{ matrix.gem_platform }} | |||
Member
There was a problem hiding this comment.
Minor: We discussed that here we actually download all of the binary tarballs; but it's kinda awkward to describe that in github, so for now every run downloads every tarball.
Also it would be nice to only download the ones we need to make sure we don't package anything extra, but probably not for this PR.
We could also do all the packaging in one step, but maybe not for now.
Comment on lines
+210
to
+283
| name: Validate (${{ matrix.platform.ruby_platform }}, ${{ matrix.gem_source }}) | ||
| needs: [package] | ||
| runs-on: ${{ matrix.platform.base }} | ||
| steps: | ||
| - name: Download gem | ||
| uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 | ||
| with: | ||
| name: gem-${{ matrix.gem_source == 'ruby' && 'ruby' || matrix.platform.gem_platform }} | ||
| path: pkg | ||
| - name: Start container | ||
| run: docker run --rm --detach --name validate --volume "${PWD}:${PWD}" -w "${PWD}" ${{ matrix.platform.image }} sleep 86400 | ||
| - name: Install gem | ||
| run: docker exec validate gem install pkg/*.gem | ||
| - name: Validate gem | ||
| run: | | ||
| docker exec validate ruby -e ' | ||
| require "libdatadog" | ||
| puts "version: #{Libdatadog::VERSION}" | ||
| puts "platform: #{RUBY_PLATFORM}" | ||
| puts "binaries: #{Libdatadog.available_binaries}" | ||
| puts "pkgconfig: #{Libdatadog.pkgconfig_folder}" | ||
| puts "receiver: #{Libdatadog.path_to_crashtracking_receiver_binary}" | ||
| raise "pkgconfig_folder is nil" unless Libdatadog.pkgconfig_folder | ||
| raise "crashtracking receiver not found" unless Libdatadog.path_to_crashtracking_receiver_binary | ||
| ' | ||
| - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 | ||
| with: | ||
| sparse-checkout: spec/smoke | ||
| - name: Smoke test (compile and run C program against libdatadog) | ||
| run: docker exec validate ruby spec/smoke/run.rb | ||
| - name: Stop container | ||
| if: always() | ||
| run: docker stop validate || true | ||
|
|
||
| validate-macos: | ||
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| gem_source: [binary, ruby] | ||
| platform: | ||
| - ruby_platform: arm64-darwin | ||
| gem_platform: arm64-darwin | ||
| base: macos-15 | ||
| name: Validate (${{ matrix.platform.ruby_platform }}, ${{ matrix.gem_source }}) | ||
| needs: [package] | ||
| runs-on: ${{ matrix.platform.base }} | ||
| steps: | ||
| - uses: ruby/setup-ruby@7372622e62b60b3cb750dcd2b9e32c247ffec26a # v1.302.0 | ||
| with: | ||
| ruby-version: "4.0" | ||
| - name: Download gem | ||
| uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 | ||
| with: | ||
| name: gem-${{ matrix.gem_source == 'ruby' && 'ruby' || matrix.platform.gem_platform }} | ||
| path: pkg | ||
| - name: Install gem | ||
| run: gem install pkg/*.gem | ||
| - name: Validate gem | ||
| run: | | ||
| ruby -e ' | ||
| require "libdatadog" | ||
| puts "version: #{Libdatadog::VERSION}" | ||
| puts "platform: #{RUBY_PLATFORM}" | ||
| puts "binaries: #{Libdatadog.available_binaries}" | ||
| puts "pkgconfig: #{Libdatadog.pkgconfig_folder}" | ||
| puts "receiver: #{Libdatadog.path_to_crashtracking_receiver_binary}" | ||
| raise "pkgconfig_folder is nil" unless Libdatadog.pkgconfig_folder | ||
| raise "crashtracking receiver not found" unless Libdatadog.path_to_crashtracking_receiver_binary | ||
| ' | ||
| - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 | ||
| with: | ||
| sparse-checkout: spec/smoke | ||
| - name: Smoke test (compile and run C program against libdatadog) | ||
| run: ruby spec/smoke/run.rb |
Member
There was a problem hiding this comment.
Minor: Maybe we could combine these, to avoid having two variants of the logic
(It needs two variants because we can't say "use docker on linux; don't use docker on macOS")
Member
There was a problem hiding this comment.
Minor: We discussed that this setup may get out-of-sync with how dd-trace-rb consumes libdatadog-rb; and that we could have a very small gem to try to mimic things more closely but not for this PR. (maybe never?)
Member
|
Ok, as promised, I did a run of downloading the binary artifacts from this PR and comparing them with the ones on upstream libdatadog/rubygems.org. It's in great shape -- I was only able to find two very small things to improve/fix. Notes on comparing builds/gems:
|
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Why?
FUP to #34 (chained PR)
What does this PR do?
Ensure that all variants of the gem:
How to test the change?
CI
Additional Notes:
This is intentionally additive: the previous workflow is kept to allow a release "the old way" in short order until the "new way" is fully validated.
JIRA: