Skip to content

[Bug #21828] Suppress bundled gems warning for subfeatures found outside stdlib#16369

Merged
hsbt merged 2 commits intoruby:masterfrom
hsbt:fix-21828
Mar 11, 2026
Merged

[Bug #21828] Suppress bundled gems warning for subfeatures found outside stdlib#16369
hsbt merged 2 commits intoruby:masterfrom
hsbt:fix-21828

Conversation

@hsbt
Copy link
Copy Markdown
Member

@hsbt hsbt commented Mar 11, 2026

Fixup [Bug #21828]

…ug #21828]

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 11, 2026 10:15
@hsbt hsbt changed the title Suppress bundled gems warning for subfeatures found outside stdlib [Bug #21828] Suppress bundled gems warning for subfeatures found outside stdlib Mar 11, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adjusts Gem::BUNDLED_GEMS.warning? to avoid emitting bundled-gems warnings for stdlib “subfeatures” (e.g., benchmark/ips) when the resolved file comes from a non-stdlib directory on $LOAD_PATH (Bug #21828), and adds a regression test for that behavior.

Changes:

  • Add a $LOAD_PATH.resolve_feature_path-based check to suppress warnings when the resolved path is outside LIBDIR/ARCHDIR.
  • Add a test that places a mock benchmark/ips.rb on $LOAD_PATH and asserts no warning is produced.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
lib/bundled_gems.rb Suppresses subfeature warnings when the feature resolves outside stdlib directories.
test/test_bundled_gems.rb Adds regression coverage for “subfeature found outside stdlib” behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.


# Don't warn if the feature is found outside the standard library
# (e.g., benchmark-ips's lib dir is on $LOAD_PATH but not in specs)
resolved = $LOAD_PATH.resolve_feature_path(feature) rescue nil
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolved = $LOAD_PATH.resolve_feature_path(feature) rescue nil will silently swallow any StandardError from resolve_feature_path, which can mask real bugs and makes failures harder to diagnose. Since this is just a feature-detection guard, prefer an explicit capability check (e.g., $LOAD_PATH.respond_to?(:resolve_feature_path)) or rescuing only the specific exception you expect (like NoMethodError).

Suggested change
resolved = $LOAD_PATH.resolve_feature_path(feature) rescue nil
resolved = $LOAD_PATH.respond_to?(:resolve_feature_path) ? $LOAD_PATH.resolve_feature_path(feature) : nil

Copilot uses AI. Check for mistakes.
Comment on lines +137 to +141
# Don't warn if the feature is found outside the standard library
# (e.g., benchmark-ips's lib dir is on $LOAD_PATH but not in specs)
resolved = $LOAD_PATH.resolve_feature_path(feature) rescue nil
if resolved
return unless resolved[1].start_with?(LIBDIR) || resolved[1].start_with?(ARCHDIR)
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This adds a $LOAD_PATH.resolve_feature_path(feature) scan on every require of an extracted stdlib subfeature when the gem isn't in specs. In the (now common) case where the subfeature is found outside stdlib and you return early, there’s no caching, so repeated require "benchmark/ips" calls will keep re-resolving the feature path. Consider caching the “resolved outside stdlib” decision (e.g., keyed by feature or name) to avoid repeated filesystem/load-path scans.

Suggested change
# Don't warn if the feature is found outside the standard library
# (e.g., benchmark-ips's lib dir is on $LOAD_PATH but not in specs)
resolved = $LOAD_PATH.resolve_feature_path(feature) rescue nil
if resolved
return unless resolved[1].start_with?(LIBDIR) || resolved[1].start_with?(ARCHDIR)
# Cache the decision about whether this feature is provided outside
# the standard library to avoid repeated filesystem/load-path scans.
outside_stdlib_cache = (@outside_stdlib_cache ||= {})
cached = outside_stdlib_cache[feature]
if cached == :outside
# Previously determined to be outside stdlib; don't warn.
return
end
# Don't warn if the feature is found outside the standard library
# (e.g., benchmark-ips's lib dir is on $LOAD_PATH but not in specs)
resolved = $LOAD_PATH.resolve_feature_path(feature) rescue nil
if resolved
if resolved[1].start_with?(LIBDIR) || resolved[1].start_with?(ARCHDIR)
outside_stdlib_cache[feature] = :stdlib
else
outside_stdlib_cache[feature] = :outside
return
end

Copilot uses AI. Check for mistakes.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@hsbt hsbt enabled auto-merge (rebase) March 11, 2026 10:26
@hsbt hsbt merged commit 44b99d6 into ruby:master Mar 11, 2026
91 checks passed
Maumagnaguagno added a commit to Maumagnaguagno/ruby that referenced this pull request Mar 24, 2026
hsbt added a commit that referenced this pull request Mar 24, 2026
* Refactor bundled gem warning suppressor

Related to #16369

* Accept suggestion

Co-authored-by: Hiroshi SHIBATA <hsbt@ruby-lang.org>

---------

Co-authored-by: Hiroshi SHIBATA <hsbt@ruby-lang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants