[PROF-11035] Fix Ruby version matching for preview releases#11
Merged
Merged
Conversation
**What does this PR do?** This PR fixes an issue that we spotted in #8: that the logic we inherited for matching on header folders for beta versions of Ruby was a bit buggy. Specifically, for preview/rc versions of Ruby, the "preview"/"rc" part is (as far as I know) only part of the `RUBY_DESCRIPTION` and does not show up anywhere else (including the `RUBY_VERSION`). This meant that as we are shipping for instance headers for * `ruby-3.4.0-preview2` * `ruby-3.4.0-rc1` The `RUBY_VERSION` for Ruby 3.4.0-preview2 is 3.4.0, and thus the fuzzy matching logic would pick the "latest" option out of the multiple options for 3.4.0, which would mean the 3.4.0-rc1 headers were used for 3.4.0-preview2. The way `ruby_core_source` version matching works is in two passes: 1. It first checks if there's an exact hit 2. If not, it falls back to a fuzzy version matching logic By including the `preview`/`rc` in the string, my change affects 1: if there's an exact hit on headers (e.g. `preview2` on `preview2`), then those get used. **Motivation:** This change is useful since the 3.4.0-preview2 headers are not compatible with 3.4.0-rc1 and vice-versa; so this way we can continue to support both versions until stable 3.4.0 is out. **Additional Notes:** My change does not touch the fuzzy matching, e.g. for `preview1`, since we have no exact match, the latest ones (`-rc1`) will get used. I believe that there's no use in being "too clever" here. If we have ruby 3.3.5 headers and 3.4.0-preview2, and we're in `preview1`, what's the right answer of what to pick? I argue there's no right answer here, so I didn't touch this part of the logic. Two other notes: 1. `RUBY_PATCHLEVEL < 0` is only for beta releases, so this change is not expected to impact any other case 2. The `$stderr.puts` is hidden by default during installation, so it's fine to leave like that (there's other similar prints in the profiler's `extconf.rb`) **How to test the change?** I've tested this manually with dd-trace-rb in different Ruby versions. ``` $ ruby -v ruby 3.4.0rc1 (2024-12-12 master 29caae9991) +PRISM [x86_64-linux] Using datadog-ruby_core_source headers from datadog-ruby_core_source/lib/datadog/ruby_core_source/ruby-3.4.0-rc1 $ ruby -v ruby 3.4.0preview2 (2024-10-07 master 32c733f57b) +PRISM [x86_64-linux] Using datadog-ruby_core_source headers from datadog-ruby_core_source/lib/datadog/ruby_core_source/ruby-3.4.0-preview2 $ ruby -v ruby 3.4.0preview1 (2024-05-16 master 9d69619623) [x86_64-linux] Using datadog-ruby_core_source headers from datadog-ruby_core_source/lib/datadog/ruby_core_source/ruby-3.4.0-rc1 ```
This was referenced Dec 13, 2024
AlexJF
approved these changes
Dec 16, 2024
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
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.
What does this PR do?
This PR fixes an issue that we spotted in #8: that the logic we inherited for matching on header folders for beta versions of Ruby was a bit buggy.
Specifically, for preview/rc versions of Ruby, the "preview"/"rc" part is (as far as I know) only part of the
RUBY_DESCRIPTIONand does not show up anywhere else (including theRUBY_VERSION).This meant that as we are shipping for instance headers for
ruby-3.4.0-preview2ruby-3.4.0-rc1The
RUBY_VERSIONfor Ruby 3.4.0-preview2 is 3.4.0, and thus the fuzzy matching logic would pick the "latest" option out of the multiple options for 3.4.0, which would mean the 3.4.0-rc1 headers were used for 3.4.0-preview2.The way
ruby_core_sourceversion matching works is in two passes:By including the
preview/rcin the string, my change affects 1: if there's an exact hit on headers (e.g.preview2onpreview2), then those get used.Motivation:
This change is useful since the 3.4.0-preview2 headers are not compatible with 3.4.0-rc1 and vice-versa; so this way we can continue to support both versions until stable 3.4.0 is out.
Additional Notes:
My change does not touch the fuzzy matching, e.g. for
preview1, since we have no exact match, the latest ones (-rc1) will get used.I believe that there's no use in being "too clever" here. If we have ruby 3.3.5 headers and 3.4.0-preview2, and we're in
preview1, what's the right answer of what to pick?I argue there's no right answer here, so I didn't touch this part of the logic.
Two other notes:
RUBY_PATCHLEVEL < 0is only for beta releases, so this change is not expected to impact any other caseThe
$stderr.putsis hidden by default during installation, so it's fine to leave like that (there's other similar prints in the profiler'sextconf.rb)How to test the change?
I've tested this manually with dd-trace-rb in different Ruby versions.