Backport GH-15908 - #16154
Conversation
PR ruby#15822 fixed the warning for direct hyphenated gem requires like `benchmark/ips` → `benchmark-ips`. However, hyphenated gems often provide multiple files under their namespace. For example, `benchmark-ips` provides: - benchmark/ips.rb - benchmark/timing.rb - benchmark/compare.rb When requiring `benchmark/timing`, the previous fix only checked for `benchmark-timing` gem (doesn't exist), not `benchmark-ips` which actually provides the file. This fix checks if ANY gem matching `{prefix}-*` is in the bundle specs, which covers all subfeatures provided by hyphenated gems. Reported in ruby#15822 (comment)
There was a problem hiding this comment.
Pull request overview
Backports #15908 to fix false “bundled gems” warnings when requiring subfeatures of hyphenated gems (e.g., benchmark/timing when benchmark-ips is in the bundle), improving the warning suppression logic in Gem::BUNDLED_GEMS.
Changes:
- Expand hyphenated-gem suppression from an exact
{prefix}-{subfeature}match to any bundled gem matching{prefix}-*. - Add tests ensuring
benchmark-ipssuppresses warnings forbenchmark/timingandbenchmark/compare.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
lib/bundled_gems.rb |
Updates warning suppression logic to treat any {prefix}-* gem as potentially providing prefix/* subfeatures. |
test/test_bundled_gems.rb |
Adds regression tests for subfeatures (benchmark/timing, benchmark/compare) when benchmark-ips is present. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Don't warn if a hyphenated gem provides this feature | ||
| # (e.g., benchmark-ips provides benchmark/ips, not the benchmark gem) | ||
| # (e.g., benchmark-ips provides benchmark/ips, benchmark/timing, etc.) | ||
| if subfeature | ||
| feature_parts = feature.split("/") | ||
| if feature_parts.size >= 2 | ||
| hyphenated_gem = "#{feature_parts[0]}-#{feature_parts[1]}" | ||
| return if specs.include?(hyphenated_gem) | ||
| end | ||
| prefix = feature.split("/").first + "-" | ||
| return if specs.any? { |spec, _| spec.start_with?(prefix) } | ||
| end |
There was a problem hiding this comment.
specs.any? { ... } introduces an O(n) scan over all bundled specs on the subfeature path. Because this check runs before return if WARNED[name], it will still scan on every subsequent require of other subfeatures even after we've already warned once for the same name. Consider moving the WARNED[name] early-return above the prefix scan (and/or caching an index of available gem-name prefixes) to avoid repeated full scans in the common case.
|
I will also backport #16157 for build failure of cygwin. |
from #15908
We should fix to suppress logic for
benchmark-ips.