From ec7bdd6331af4a128da4e1166a53393ce0074ab6 Mon Sep 17 00:00:00 2001 From: Matthias Zirnstein Date: Wed, 5 Nov 2025 10:51:55 +0100 Subject: [PATCH 01/14] [ruby/rubygems] fix: Ensure trailing slash is added to source URIs added via gem sources GitHub's private gem registry expects the first path segment after the host to represent the namespace, typically the organization or user name. [1] When adding a source with ``` gem sources --add https://user:password@rubygems.pkg.github.com/my-org ``` without a trailing slash, the last path segment ("my-org") is interpreted as a file and removed during relative path resolution. This causes the resulting URI to become ``` https://user:password@rubygems.pkg.github.com/gems/foo.gem ``` instead of the correct ``` https://user:password@rubygems.pkg.github.com/my-org/gems/foo.gem. [2] ``` Example error: ``` gem source -a https://user:password@rubygems.pkg.github.com/my-org gem install -rf foo.gem rubygems/remote_fetcher.rb:238:in `fetch_http': bad response Not Found 404 (https://user:REDACTED@rubygems.pkg.github.com/gems/foo-0.7.1.gem) (Gem::RemoteFetcher::FetchError) ``` Although this behavior complies with RFC 2396, it's incompatible with GitHub's gem registry requirements. The remote fetcher is just append a relative path without using ./ [3] To address this, we automatically append a trailing slash when adding new gem sources. As illustrated in [4] and [5], given the base URI ``` http://a/b/c/d;p?q ``` and a relative path ``` g/f ``` the resolution process replaces "d;p?q" and yields ``` http://a/b/c/g/f ``` [1] https://docs.github.com/en/packages/working-with-a-github-packages-registry/working-with-the-rubygems-registry#authenticating-with-a-personal-access-token [2] https://github.com/ruby/rubygems/blob/master/lib/rubygems/vendor/uri/lib/uri/generic.rb#L1053 [3] https://github.com/ruby/rubygems/blob/master/lib/rubygems/remote_fetcher.rb#L148 [4] https://www.rfc-editor.org/rfc/rfc2396#section-5.2 [5] https://www.rfc-editor.org/rfc/rfc2396#appendix-C https://github.com/ruby/rubygems/commit/b1e4fb15ae --- lib/rubygems/commands/sources_command.rb | 16 ++++ .../test_gem_commands_sources_command.rb | 84 ++++++++++++++++++- 2 files changed, 98 insertions(+), 2 deletions(-) diff --git a/lib/rubygems/commands/sources_command.rb b/lib/rubygems/commands/sources_command.rb index 7e5c2a2465e6f3..95be9d334b3352 100644 --- a/lib/rubygems/commands/sources_command.rb +++ b/lib/rubygems/commands/sources_command.rb @@ -50,6 +50,7 @@ def initialize end def add_source(source_uri) # :nodoc: + source_uri = add_trailing_slash(source_uri) check_rubygems_https source_uri source = Gem::Source.new source_uri @@ -76,6 +77,7 @@ def add_source(source_uri) # :nodoc: end def append_source(source_uri) # :nodoc: + source_uri = add_trailing_slash(source_uri) check_rubygems_https source_uri source = Gem::Source.new source_uri @@ -103,6 +105,7 @@ def append_source(source_uri) # :nodoc: end def prepend_source(source_uri) # :nodoc: + source_uri = add_trailing_slash(source_uri) check_rubygems_https source_uri source = Gem::Source.new source_uri @@ -141,6 +144,19 @@ def check_typo_squatting(source) end end + def add_trailing_slash(source_uri) # :nodoc: + # Ensure the source URI has a trailing slash for proper RFC 2396 path merging + # Without a trailing slash, the last path segment is treated as a file and removed + # during relative path resolution (e.g., "/blish" + "gems/foo.gem" = "/gems/foo.gem") + # With a trailing slash, it's treated as a directory (e.g., "/blish/" + "gems/foo.gem" = "/blish/gems/foo.gem") + uri = Gem::URI.parse(source_uri) + uri.path = uri.path.gsub(%r{/+$}, "") + "/" if uri.path && !uri.path.empty? + uri.to_s + rescue Gem::URI::Error + # If parsing fails, return the original URI and let later validation handle it + source_uri + end + def check_rubygems_https(source_uri) # :nodoc: uri = Gem::URI source_uri diff --git a/test/rubygems/test_gem_commands_sources_command.rb b/test/rubygems/test_gem_commands_sources_command.rb index 00eb9239940e81..62795e5665b94f 100644 --- a/test/rubygems/test_gem_commands_sources_command.rb +++ b/test/rubygems/test_gem_commands_sources_command.rb @@ -60,6 +60,81 @@ def test_execute_add assert_equal "", @ui.error end + def test_execute_add_without_trailing_slash + setup_fake_source('https://rubygems.pkg.github.com/my-org') + + @cmd.handle_options %W[--add https://rubygems.pkg.github.com/my-org] + + use_ui @ui do + @cmd.execute + end + + assert_equal [@gem_repo, 'https://rubygems.pkg.github.com/my-org/'], Gem.sources + + expected = <<-EOF +https://rubygems.pkg.github.com/my-org/ added to sources + EOF + + assert_equal expected, @ui.output + assert_equal "", @ui.error + end + def test_execute_add_multiple_trailing_slash + setup_fake_source('https://rubygems.pkg.github.com/my-org/') + + @cmd.handle_options %W[--add https://rubygems.pkg.github.com/my-org///] + + use_ui @ui do + @cmd.execute + end + + assert_equal [@gem_repo, 'https://rubygems.pkg.github.com/my-org/'], Gem.sources + + expected = <<-EOF +https://rubygems.pkg.github.com/my-org/ added to sources + EOF + + assert_equal expected, @ui.output + assert_equal "", @ui.error + end + + def test_execute_append_without_trailing_slash + setup_fake_source('https://rubygems.pkg.github.com/my-org') + + @cmd.handle_options %W[--append https://rubygems.pkg.github.com/my-org] + + use_ui @ui do + @cmd.execute + end + + assert_equal [@gem_repo, 'https://rubygems.pkg.github.com/my-org/'], Gem.sources + + expected = <<-EOF +https://rubygems.pkg.github.com/my-org/ added to sources + EOF + + assert_equal expected, @ui.output + assert_equal "", @ui.error + end + + def test_execute_prepend_without_trailing_slash + setup_fake_source('https://rubygems.pkg.github.com/my-org') + + @cmd.handle_options %W[--prepend https://rubygems.pkg.github.com/my-org] + + use_ui @ui do + @cmd.execute + end + + assert_equal [@gem_repo, 'https://rubygems.pkg.github.com/my-org/'], Gem.sources + + expected = <<-EOF +https://rubygems.pkg.github.com/my-org/ added to sources + EOF + + assert_equal expected, @ui.output + assert_equal "", @ui.error + end + def test_execute_append setup_fake_source(@new_repo) @@ -583,7 +658,7 @@ def test_execute_add_bad_uri assert_equal [@gem_repo], Gem.sources expected = <<-EOF -beta-gems.example.com is not a URI +beta-gems.example.com/ is not a URI EOF assert_equal expected, @ui.output @@ -602,7 +677,12 @@ def test_execute_append_bad_uri assert_equal [@gem_repo], Gem.sources expected = <<-EOF -beta-gems.example.com is not a URI +beta-gems.example.com/ is not a URI + EOF + + assert_equal expected, @ui.output + assert_equal "", @ui.error + end EOF assert_equal expected, @ui.output From 63818352673fb186ec1cf6d8fc36665b79e3e82a Mon Sep 17 00:00:00 2001 From: Matthias Zirnstein Date: Wed, 5 Nov 2025 10:52:21 +0100 Subject: [PATCH 02/14] [ruby/rubygems] spec: Add missing bad uri test for prepend for gem sources https://github.com/ruby/rubygems/commit/376bcdf430 --- test/rubygems/test_gem_commands_sources_command.rb | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/rubygems/test_gem_commands_sources_command.rb b/test/rubygems/test_gem_commands_sources_command.rb index 62795e5665b94f..60df6d9a805567 100644 --- a/test/rubygems/test_gem_commands_sources_command.rb +++ b/test/rubygems/test_gem_commands_sources_command.rb @@ -683,6 +683,20 @@ def test_execute_append_bad_uri assert_equal expected, @ui.output assert_equal "", @ui.error end + + def test_execute_prepend_bad_uri + @cmd.handle_options %w[--prepend beta-gems.example.com] + + use_ui @ui do + assert_raise Gem::MockGemUi::TermError do + @cmd.execute + end + end + + assert_equal [@gem_repo], Gem.sources + + expected = <<-EOF +beta-gems.example.com/ is not a URI EOF assert_equal expected, @ui.output From c8c5759257941961c28ab35a274353559aaf556c Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Wed, 18 Mar 2026 07:04:46 +0900 Subject: [PATCH 03/14] [ruby/rubygems] Refactor source URI normalization in sources command - Rename add_trailing_slash to normalize_source_uri to better reflect that it also removes duplicated trailing slashes - Extract build_source and build_new_source to eliminate duplicated source creation pattern across add_source, append_source, prepend_source, and remove_source - Apply URI normalization to remove_source as well - Use \z instead of $ in trailing slash regex for correctness https://github.com/ruby/rubygems/commit/3e81961ef8 Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/rubygems/commands/sources_command.rb | 43 ++++++++++++------------ 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/lib/rubygems/commands/sources_command.rb b/lib/rubygems/commands/sources_command.rb index 95be9d334b3352..b399af2bd3d5a0 100644 --- a/lib/rubygems/commands/sources_command.rb +++ b/lib/rubygems/commands/sources_command.rb @@ -50,12 +50,8 @@ def initialize end def add_source(source_uri) # :nodoc: - source_uri = add_trailing_slash(source_uri) - check_rubygems_https source_uri - - source = Gem::Source.new source_uri - - check_typo_squatting(source) + source = build_new_source(source_uri) + source_uri = source.uri.to_s begin if Gem.sources.include? source @@ -77,12 +73,8 @@ def add_source(source_uri) # :nodoc: end def append_source(source_uri) # :nodoc: - source_uri = add_trailing_slash(source_uri) - check_rubygems_https source_uri - - source = Gem::Source.new source_uri - - check_typo_squatting(source) + source = build_new_source(source_uri) + source_uri = source.uri.to_s begin source.load_specs :released @@ -105,12 +97,8 @@ def append_source(source_uri) # :nodoc: end def prepend_source(source_uri) # :nodoc: - source_uri = add_trailing_slash(source_uri) - check_rubygems_https source_uri - - source = Gem::Source.new source_uri - - check_typo_squatting(source) + source = build_new_source(source_uri) + source_uri = source.uri.to_s begin source.load_specs :released @@ -144,13 +132,13 @@ def check_typo_squatting(source) end end - def add_trailing_slash(source_uri) # :nodoc: + def normalize_source_uri(source_uri) # :nodoc: # Ensure the source URI has a trailing slash for proper RFC 2396 path merging # Without a trailing slash, the last path segment is treated as a file and removed # during relative path resolution (e.g., "/blish" + "gems/foo.gem" = "/gems/foo.gem") # With a trailing slash, it's treated as a directory (e.g., "/blish/" + "gems/foo.gem" = "/blish/gems/foo.gem") uri = Gem::URI.parse(source_uri) - uri.path = uri.path.gsub(%r{/+$}, "") + "/" if uri.path && !uri.path.empty? + uri.path = uri.path.gsub(%r{/+\z}, "") + "/" if uri.path && !uri.path.empty? uri.to_s rescue Gem::URI::Error # If parsing fails, return the original URI and let later validation handle it @@ -289,7 +277,8 @@ def execute end def remove_source(source_uri) # :nodoc: - source = Gem::Source.new source_uri + source = build_source(source_uri) + source_uri = source.uri.to_s if configured_sources&.include? source Gem.sources.delete source @@ -344,4 +333,16 @@ def configured_sources def config_file_name Gem.configuration.config_file_name end + + def build_source(source_uri) + source_uri = normalize_source_uri(source_uri) + Gem::Source.new(source_uri) + end + + def build_new_source(source_uri) + source = build_source(source_uri) + check_rubygems_https(source.uri.to_s) + check_typo_squatting(source) + source + end end From 41978d83b76244ec9909564488b4a957a7115c20 Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Wed, 18 Mar 2026 07:04:55 +0900 Subject: [PATCH 04/14] [ruby/rubygems] Fix and improve tests for source URI trailing slash normalization - Fix setup_fake_source double-slash bug: normalize URI before registering spec data to prevent URL mismatch in load_specs - Fix test_execute_add/append_https_rubygems_org: these tests were incorrectly expecting TermError due to the double-slash bug - Fix test_execute_prepend_without_trailing_slash: correct expected source order (prepend adds to front, not end) - Fix test_execute_remove_redundant_source_trailing_slash: path-less URIs are not modified by normalize_source_uri - Use assert_equal for Gem.sources to improve failure diagnostics https://github.com/ruby/rubygems/commit/da8d622c05 Co-Authored-By: Claude Opus 4.6 (1M context) --- .../test_gem_commands_sources_command.rb | 66 ++++++++++++------- 1 file changed, 43 insertions(+), 23 deletions(-) diff --git a/test/rubygems/test_gem_commands_sources_command.rb b/test/rubygems/test_gem_commands_sources_command.rb index 60df6d9a805567..71c6d5ce166828 100644 --- a/test/rubygems/test_gem_commands_sources_command.rb +++ b/test/rubygems/test_gem_commands_sources_command.rb @@ -61,7 +61,7 @@ def test_execute_add end def test_execute_add_without_trailing_slash - setup_fake_source('https://rubygems.pkg.github.com/my-org') + setup_fake_source("https://rubygems.pkg.github.com/my-org") @cmd.handle_options %W[--add https://rubygems.pkg.github.com/my-org] @@ -69,7 +69,7 @@ def test_execute_add_without_trailing_slash @cmd.execute end - assert_equal [@gem_repo, 'https://rubygems.pkg.github.com/my-org/'], Gem.sources + assert_equal [@gem_repo, "https://rubygems.pkg.github.com/my-org/"], Gem.sources expected = <<-EOF https://rubygems.pkg.github.com/my-org/ added to sources @@ -78,8 +78,9 @@ def test_execute_add_without_trailing_slash assert_equal expected, @ui.output assert_equal "", @ui.error end + def test_execute_add_multiple_trailing_slash - setup_fake_source('https://rubygems.pkg.github.com/my-org/') + setup_fake_source("https://rubygems.pkg.github.com/my-org/") @cmd.handle_options %W[--add https://rubygems.pkg.github.com/my-org///] @@ -87,7 +88,7 @@ def test_execute_add_multiple_trailing_slash @cmd.execute end - assert_equal [@gem_repo, 'https://rubygems.pkg.github.com/my-org/'], Gem.sources + assert_equal [@gem_repo, "https://rubygems.pkg.github.com/my-org/"], Gem.sources expected = <<-EOF https://rubygems.pkg.github.com/my-org/ added to sources @@ -98,7 +99,7 @@ def test_execute_add_multiple_trailing_slash end def test_execute_append_without_trailing_slash - setup_fake_source('https://rubygems.pkg.github.com/my-org') + setup_fake_source("https://rubygems.pkg.github.com/my-org") @cmd.handle_options %W[--append https://rubygems.pkg.github.com/my-org] @@ -106,7 +107,7 @@ def test_execute_append_without_trailing_slash @cmd.execute end - assert_equal [@gem_repo, 'https://rubygems.pkg.github.com/my-org/'], Gem.sources + assert_equal [@gem_repo, "https://rubygems.pkg.github.com/my-org/"], Gem.sources expected = <<-EOF https://rubygems.pkg.github.com/my-org/ added to sources @@ -117,7 +118,7 @@ def test_execute_append_without_trailing_slash end def test_execute_prepend_without_trailing_slash - setup_fake_source('https://rubygems.pkg.github.com/my-org') + setup_fake_source("https://rubygems.pkg.github.com/my-org") @cmd.handle_options %W[--prepend https://rubygems.pkg.github.com/my-org] @@ -125,7 +126,7 @@ def test_execute_prepend_without_trailing_slash @cmd.execute end - assert_equal [@gem_repo, 'https://rubygems.pkg.github.com/my-org/'], Gem.sources + assert_equal ["https://rubygems.pkg.github.com/my-org/", @gem_repo], Gem.sources expected = <<-EOF https://rubygems.pkg.github.com/my-org/ added to sources @@ -605,17 +606,14 @@ def test_execute_add_https_rubygems_org @cmd.handle_options %W[--add #{https_rubygems_org}] - ui = Gem::MockGemUi.new "n" - - use_ui ui do - assert_raise Gem::MockGemUi::TermError do - @cmd.execute - end + use_ui @ui do + @cmd.execute end - assert_equal [@gem_repo], Gem.sources + assert_equal [@gem_repo, https_rubygems_org], Gem.sources expected = <<-EXPECTED +#{https_rubygems_org} added to sources EXPECTED assert_equal expected, @ui.output @@ -629,17 +627,14 @@ def test_execute_append_https_rubygems_org @cmd.handle_options %W[--append #{https_rubygems_org}] - ui = Gem::MockGemUi.new "n" - - use_ui ui do - assert_raise Gem::MockGemUi::TermError do - @cmd.execute - end + use_ui @ui do + @cmd.execute end - assert_equal [@gem_repo], Gem.sources + assert_equal [@gem_repo, https_rubygems_org], Gem.sources expected = <<-EXPECTED +#{https_rubygems_org} added to sources EXPECTED assert_equal expected, @ui.output @@ -872,6 +867,31 @@ def test_execute_remove_redundant_source_trailing_slash Gem.configuration.sources = nil end + def test_execute_remove_without_trailing_slash + source_uri = "https://rubygems.pkg.github.com/my-org/" + + Gem.configuration.sources = [source_uri] + + setup_fake_source(source_uri) + + @cmd.handle_options %W[--remove https://rubygems.pkg.github.com/my-org] + + use_ui @ui do + @cmd.execute + end + + assert_equal [], Gem.sources + + expected = <<-EOF +#{source_uri} removed from sources + EOF + + assert_equal expected, @ui.output + assert_equal "", @ui.error + ensure + Gem.configuration.sources = nil + end + def test_execute_update @cmd.handle_options %w[--update] @@ -982,6 +1002,6 @@ def setup_fake_source(uri) Marshal.dump specs, io end - @fetcher.data["#{uri}/specs.#{@marshal_version}.gz"] = specs_dump_gz.string + @fetcher.data["#{uri.chomp("/")}/specs.#{@marshal_version}.gz"] = specs_dump_gz.string end end From 9dc87b19429ab588969334f18610ecba85949d7b Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 18 Mar 2026 02:10:04 +0000 Subject: [PATCH 05/14] Bump the github-actions group across 1 directory with 2 updates Bumps the github-actions group with 2 updates in the / directory: [ruby/setup-ruby](https://github.com/ruby/setup-ruby) and [taiki-e/install-action](https://github.com/taiki-e/install-action). Updates `ruby/setup-ruby` from 1.294.0 to 1.295.0 - [Release notes](https://github.com/ruby/setup-ruby/releases) - [Changelog](https://github.com/ruby/setup-ruby/blob/master/release.rb) - [Commits](https://github.com/ruby/setup-ruby/compare/c984c1a20bb35a1cbda04477c816cea024418be9...319994f95fa847cf3fb3cd3dbe89f6dcde9f178f) Updates `taiki-e/install-action` from 2.68.34 to 2.68.35 - [Release notes](https://github.com/taiki-e/install-action/releases) - [Changelog](https://github.com/taiki-e/install-action/blob/main/CHANGELOG.md) - [Commits](https://github.com/taiki-e/install-action/compare/de6bbd1333b8f331563d54a051e542c7dfef81c3...94a7388bec5d4c8dd93e3ebf09e0ff448f3f6f4d) --- updated-dependencies: - dependency-name: ruby/setup-ruby dependency-version: 1.295.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: github-actions - dependency-name: taiki-e/install-action dependency-version: 2.68.35 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: github-actions ... Signed-off-by: dependabot[bot] --- .github/workflows/annocheck.yml | 2 +- .github/workflows/auto_review_pr.yml | 2 +- .github/workflows/baseruby.yml | 2 +- .github/workflows/bundled_gems.yml | 2 +- .github/workflows/check_dependencies.yml | 2 +- .github/workflows/check_misc.yml | 2 +- .github/workflows/modgc.yml | 2 +- .github/workflows/parse_y.yml | 2 +- .github/workflows/publish.yml | 2 +- .github/workflows/spec_guards.yml | 2 +- .github/workflows/sync_default_gems.yml | 2 +- .github/workflows/ubuntu.yml | 2 +- .github/workflows/wasm.yml | 2 +- .github/workflows/windows.yml | 2 +- .github/workflows/yjit-ubuntu.yml | 2 +- .github/workflows/zjit-macos.yml | 2 +- .github/workflows/zjit-ubuntu.yml | 4 ++-- 17 files changed, 18 insertions(+), 18 deletions(-) diff --git a/.github/workflows/annocheck.yml b/.github/workflows/annocheck.yml index 8286cb41edb3de..ac1b338af5f713 100644 --- a/.github/workflows/annocheck.yml +++ b/.github/workflows/annocheck.yml @@ -73,7 +73,7 @@ jobs: builddir: build makeup: true - - uses: ruby/setup-ruby@c984c1a20bb35a1cbda04477c816cea024418be9 # v1.294.0 + - uses: ruby/setup-ruby@319994f95fa847cf3fb3cd3dbe89f6dcde9f178f # v1.295.0 with: ruby-version: '3.1' bundler: none diff --git a/.github/workflows/auto_review_pr.yml b/.github/workflows/auto_review_pr.yml index 88e7ba49cec31e..3e1c0fdc2755bf 100644 --- a/.github/workflows/auto_review_pr.yml +++ b/.github/workflows/auto_review_pr.yml @@ -23,7 +23,7 @@ jobs: with: persist-credentials: false - - uses: ruby/setup-ruby@c984c1a20bb35a1cbda04477c816cea024418be9 # v1.294.0 + - uses: ruby/setup-ruby@319994f95fa847cf3fb3cd3dbe89f6dcde9f178f # v1.295.0 with: ruby-version: '3.4' bundler: none diff --git a/.github/workflows/baseruby.yml b/.github/workflows/baseruby.yml index d2c9e9a204cc99..2002a49414c920 100644 --- a/.github/workflows/baseruby.yml +++ b/.github/workflows/baseruby.yml @@ -48,7 +48,7 @@ jobs: - ruby-3.3 steps: - - uses: ruby/setup-ruby@c984c1a20bb35a1cbda04477c816cea024418be9 # v1.294.0 + - uses: ruby/setup-ruby@319994f95fa847cf3fb3cd3dbe89f6dcde9f178f # v1.295.0 with: ruby-version: ${{ matrix.ruby }} bundler: none diff --git a/.github/workflows/bundled_gems.yml b/.github/workflows/bundled_gems.yml index 75719580246e4c..c7231c9d0e1e33 100644 --- a/.github/workflows/bundled_gems.yml +++ b/.github/workflows/bundled_gems.yml @@ -38,7 +38,7 @@ jobs: with: token: ${{ (github.repository == 'ruby/ruby' && !startsWith(github.event_name, 'pull')) && secrets.MATZBOT_AUTO_UPDATE_TOKEN || secrets.GITHUB_TOKEN }} - - uses: ruby/setup-ruby@c984c1a20bb35a1cbda04477c816cea024418be9 # v1.294.0 + - uses: ruby/setup-ruby@319994f95fa847cf3fb3cd3dbe89f6dcde9f178f # v1.295.0 with: ruby-version: 4.0 diff --git a/.github/workflows/check_dependencies.yml b/.github/workflows/check_dependencies.yml index 820d4f2aadcefe..0f1f6d9f97f50a 100644 --- a/.github/workflows/check_dependencies.yml +++ b/.github/workflows/check_dependencies.yml @@ -42,7 +42,7 @@ jobs: - uses: ./.github/actions/setup/directories - - uses: ruby/setup-ruby@c984c1a20bb35a1cbda04477c816cea024418be9 # v1.294.0 + - uses: ruby/setup-ruby@319994f95fa847cf3fb3cd3dbe89f6dcde9f178f # v1.295.0 with: ruby-version: '3.1' bundler: none diff --git a/.github/workflows/check_misc.yml b/.github/workflows/check_misc.yml index 518b246f72f453..863655cd0682e0 100644 --- a/.github/workflows/check_misc.yml +++ b/.github/workflows/check_misc.yml @@ -23,7 +23,7 @@ jobs: token: ${{ (github.repository == 'ruby/ruby' && !startsWith(github.event_name, 'pull')) && secrets.MATZBOT_AUTO_UPDATE_TOKEN || secrets.GITHUB_TOKEN }} persist-credentials: false - - uses: ruby/setup-ruby@c984c1a20bb35a1cbda04477c816cea024418be9 # v1.294.0 + - uses: ruby/setup-ruby@319994f95fa847cf3fb3cd3dbe89f6dcde9f178f # v1.295.0 with: ruby-version: head diff --git a/.github/workflows/modgc.yml b/.github/workflows/modgc.yml index ad7928e050174a..f96648c06b70ed 100644 --- a/.github/workflows/modgc.yml +++ b/.github/workflows/modgc.yml @@ -62,7 +62,7 @@ jobs: uses: ./.github/actions/setup/ubuntu if: ${{ contains(matrix.os, 'ubuntu') }} - - uses: ruby/setup-ruby@c984c1a20bb35a1cbda04477c816cea024418be9 # v1.294.0 + - uses: ruby/setup-ruby@319994f95fa847cf3fb3cd3dbe89f6dcde9f178f # v1.295.0 with: ruby-version: '3.1' bundler: none diff --git a/.github/workflows/parse_y.yml b/.github/workflows/parse_y.yml index 318e5699b45de4..ca9b7e0cb40e40 100644 --- a/.github/workflows/parse_y.yml +++ b/.github/workflows/parse_y.yml @@ -59,7 +59,7 @@ jobs: - uses: ./.github/actions/setup/ubuntu - - uses: ruby/setup-ruby@c984c1a20bb35a1cbda04477c816cea024418be9 # v1.294.0 + - uses: ruby/setup-ruby@319994f95fa847cf3fb3cd3dbe89f6dcde9f178f # v1.295.0 with: ruby-version: '3.1' bundler: none diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 5489f95af60248..83ae60f248ca56 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -19,7 +19,7 @@ jobs: with: persist-credentials: false - - uses: ruby/setup-ruby@c984c1a20bb35a1cbda04477c816cea024418be9 # v1.294.0 + - uses: ruby/setup-ruby@319994f95fa847cf3fb3cd3dbe89f6dcde9f178f # v1.295.0 with: ruby-version: 3.3.4 diff --git a/.github/workflows/spec_guards.yml b/.github/workflows/spec_guards.yml index 54a657d290aaa5..6301f02bf2a2a4 100644 --- a/.github/workflows/spec_guards.yml +++ b/.github/workflows/spec_guards.yml @@ -49,7 +49,7 @@ jobs: with: persist-credentials: false - - uses: ruby/setup-ruby@c984c1a20bb35a1cbda04477c816cea024418be9 # v1.294.0 + - uses: ruby/setup-ruby@319994f95fa847cf3fb3cd3dbe89f6dcde9f178f # v1.295.0 with: ruby-version: ${{ matrix.ruby }} bundler: none diff --git a/.github/workflows/sync_default_gems.yml b/.github/workflows/sync_default_gems.yml index a16595f5e88694..72203e10f57c32 100644 --- a/.github/workflows/sync_default_gems.yml +++ b/.github/workflows/sync_default_gems.yml @@ -36,7 +36,7 @@ jobs: with: token: ${{ github.repository == 'ruby/ruby' && secrets.MATZBOT_AUTO_UPDATE_TOKEN || secrets.GITHUB_TOKEN }} - - uses: ruby/setup-ruby@c984c1a20bb35a1cbda04477c816cea024418be9 # v1.294.0 + - uses: ruby/setup-ruby@319994f95fa847cf3fb3cd3dbe89f6dcde9f178f # v1.295.0 with: ruby-version: '3.4' bundler: none diff --git a/.github/workflows/ubuntu.yml b/.github/workflows/ubuntu.yml index 9525e1800ce41f..76035a73963c13 100644 --- a/.github/workflows/ubuntu.yml +++ b/.github/workflows/ubuntu.yml @@ -70,7 +70,7 @@ jobs: with: arch: ${{ matrix.arch }} - - uses: ruby/setup-ruby@c984c1a20bb35a1cbda04477c816cea024418be9 # v1.294.0 + - uses: ruby/setup-ruby@319994f95fa847cf3fb3cd3dbe89f6dcde9f178f # v1.295.0 with: ruby-version: '3.1' bundler: none diff --git a/.github/workflows/wasm.yml b/.github/workflows/wasm.yml index dffb30c6ce652a..616bacc32021bd 100644 --- a/.github/workflows/wasm.yml +++ b/.github/workflows/wasm.yml @@ -99,7 +99,7 @@ jobs: run: | echo "WASI_SDK_PATH=/opt/wasi-sdk" >> $GITHUB_ENV - - uses: ruby/setup-ruby@c984c1a20bb35a1cbda04477c816cea024418be9 # v1.294.0 + - uses: ruby/setup-ruby@319994f95fa847cf3fb3cd3dbe89f6dcde9f178f # v1.295.0 with: ruby-version: '3.1' bundler: none diff --git a/.github/workflows/windows.yml b/.github/workflows/windows.yml index 6a49d60178ea84..8ece56f86210b6 100644 --- a/.github/workflows/windows.yml +++ b/.github/workflows/windows.yml @@ -59,7 +59,7 @@ jobs: - run: md build working-directory: - - uses: ruby/setup-ruby@c984c1a20bb35a1cbda04477c816cea024418be9 # v1.294.0 + - uses: ruby/setup-ruby@319994f95fa847cf3fb3cd3dbe89f6dcde9f178f # v1.295.0 with: # windows-11-arm has only 3.4.1, 3.4.2, 3.4.3, head ruby-version: ${{ !endsWith(matrix.os, 'arm') && '3.1' || '3.4' }} diff --git a/.github/workflows/yjit-ubuntu.yml b/.github/workflows/yjit-ubuntu.yml index a69e48624c6ad2..5df617998ac732 100644 --- a/.github/workflows/yjit-ubuntu.yml +++ b/.github/workflows/yjit-ubuntu.yml @@ -133,7 +133,7 @@ jobs: - uses: ./.github/actions/setup/ubuntu - - uses: ruby/setup-ruby@c984c1a20bb35a1cbda04477c816cea024418be9 # v1.294.0 + - uses: ruby/setup-ruby@319994f95fa847cf3fb3cd3dbe89f6dcde9f178f # v1.295.0 with: ruby-version: '3.1' bundler: none diff --git a/.github/workflows/zjit-macos.yml b/.github/workflows/zjit-macos.yml index e91c2e48f2a05d..e084855d3d6ef7 100644 --- a/.github/workflows/zjit-macos.yml +++ b/.github/workflows/zjit-macos.yml @@ -92,7 +92,7 @@ jobs: rustup install ${{ matrix.rust_version }} --profile minimal rustup default ${{ matrix.rust_version }} - - uses: taiki-e/install-action@de6bbd1333b8f331563d54a051e542c7dfef81c3 # v2.68.34 + - uses: taiki-e/install-action@94a7388bec5d4c8dd93e3ebf09e0ff448f3f6f4d # v2.68.35 with: tool: nextest@0.9 if: ${{ matrix.test_task == 'zjit-check' }} diff --git a/.github/workflows/zjit-ubuntu.yml b/.github/workflows/zjit-ubuntu.yml index b51be529900246..67b16a2ee82d31 100644 --- a/.github/workflows/zjit-ubuntu.yml +++ b/.github/workflows/zjit-ubuntu.yml @@ -114,12 +114,12 @@ jobs: - uses: ./.github/actions/setup/ubuntu - - uses: ruby/setup-ruby@c984c1a20bb35a1cbda04477c816cea024418be9 # v1.294.0 + - uses: ruby/setup-ruby@319994f95fa847cf3fb3cd3dbe89f6dcde9f178f # v1.295.0 with: ruby-version: '3.1' bundler: none - - uses: taiki-e/install-action@de6bbd1333b8f331563d54a051e542c7dfef81c3 # v2.68.34 + - uses: taiki-e/install-action@94a7388bec5d4c8dd93e3ebf09e0ff448f3f6f4d # v2.68.35 with: tool: nextest@0.9 if: ${{ matrix.test_task == 'zjit-check' }} From c1d4caa4244660c55b03da0d164301ebd9e8b443 Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Mon, 16 Mar 2026 12:35:50 +0900 Subject: [PATCH 06/14] [ruby/rubygems] Prevent tests from modifying user's global git config Set GIT_CONFIG_GLOBAL and GIT_CONFIG_NOSYSTEM environment variables in the test suite setup to isolate git configuration when Git >= 2.32 is available. This prevents test runs from accidentally polluting the developer's real git config. https://github.com/ruby/rubygems/commit/a9a921bf00 Co-Authored-By: Claude Opus 4.6 --- spec/bundler/spec_helper.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/spec/bundler/spec_helper.rb b/spec/bundler/spec_helper.rb index 0ea191751dd042..55953f09d42ff4 100644 --- a/spec/bundler/spec_helper.rb +++ b/spec/bundler/spec_helper.rb @@ -131,6 +131,14 @@ def self.ruby=(ruby) ENV["XDG_CACHE_HOME"] = nil ENV["GEMRC"] = nil + # Prevent tests from modifying the user's global git config. + # GIT_CONFIG_GLOBAL and GIT_CONFIG_NOSYSTEM are available since Git 2.32. + git_version = `git --version`[/(\d+\.\d+\.\d+)/, 1] + if Gem::Version.new(git_version) >= Gem::Version.new("2.32") + ENV["GIT_CONFIG_GLOBAL"] = File.join(ENV["HOME"], ".gitconfig") + ENV["GIT_CONFIG_NOSYSTEM"] = "1" + end + # Don't wrap output in tests ENV["THOR_COLUMNS"] = "10000" From dfd51c4cd7a67d7bfe7b3e07d3f91bfbb08a6c8d Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Mon, 16 Mar 2026 13:44:31 +0900 Subject: [PATCH 07/14] [ruby/rubygems] Add git version filter for spec metadata Use RequirementChecker to filter specs tagged with git version requirements (e.g., `git: ">= 2.28.0"`), matching the existing pattern used for rubygems version filtering. https://github.com/ruby/rubygems/commit/d157405217 Co-Authored-By: Claude Opus 4.6 --- spec/bundler/support/filters.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/spec/bundler/support/filters.rb b/spec/bundler/support/filters.rb index c6b60b5d52fe3a..6c9127cd950eff 100644 --- a/spec/bundler/support/filters.rb +++ b/spec/bundler/support/filters.rb @@ -18,10 +18,13 @@ def inspect end end +git_version = Gem::Version.new(`git --version`[/(\d+\.\d+\.\d+)/, 1]) + RSpec.configure do |config| config.filter_run_excluding realworld: true config.filter_run_excluding rubygems: RequirementChecker.against(Gem.rubygems_version) + config.filter_run_excluding git: RequirementChecker.against(git_version) config.filter_run_excluding ruby_repo: !ENV["GEM_COMMAND"].nil? config.filter_run_excluding no_color_tty: Gem.win_platform? || !ENV["GITHUB_ACTION"].nil? config.filter_run_excluding permissions: Gem.win_platform? From 0368760d15c50b17cd8a77b0991dd807a43086bd Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Tue, 17 Mar 2026 17:59:58 -0700 Subject: [PATCH 08/14] ZJIT: Stop passing scratch reg for lowering out in x86_64 --- zjit/src/backend/x86_64/mod.rs | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/zjit/src/backend/x86_64/mod.rs b/zjit/src/backend/x86_64/mod.rs index e2d0185bb8d77a..35407d5041b61a 100644 --- a/zjit/src/backend/x86_64/mod.rs +++ b/zjit/src/backend/x86_64/mod.rs @@ -378,8 +378,8 @@ impl Assembler { } /// If a given operand is Opnd::Mem and it uses MemBase::Stack, lower it to MemBase::Reg(NATIVE_BASE_PTR). - /// For MemBase::StackIndirect, load the pointer from the stack slot into a scratch register. - fn split_stack_membase(asm: &mut Assembler, opnd: Opnd, scratch_opnd: Opnd, stack_state: &StackState) -> Opnd { + /// In general, `out` operand is a `VReg`, so it may use MemBase::Stack, but not MemBase::StackIndirect. + fn lower_stack_membase(opnd: Opnd, stack_state: &StackState) -> Opnd { match opnd { Opnd::Mem(Mem { base: stack_membase @ MemBase::Stack { .. }, disp: opnd_disp, num_bits: opnd_num_bits }) => { // Convert MemBase::Stack to MemBase::Reg(NATIVE_BASE_PTR) with the @@ -389,6 +389,15 @@ impl Assembler { let Mem { base, disp: stack_disp, .. } = stack_state.stack_membase_to_mem(stack_membase); Opnd::Mem(Mem { base, disp: stack_disp + opnd_disp, num_bits: opnd_num_bits }) } + _ => opnd, + } + } + + /// If a given operand is Opnd::Mem and it uses MemBase::Stack, lower it to MemBase::Reg(NATIVE_BASE_PTR). + /// For MemBase::StackIndirect, load the pointer from the stack slot into a scratch register. + fn split_stack_membase(asm: &mut Assembler, opnd: Opnd, scratch_opnd: Opnd, stack_state: &StackState) -> Opnd { + match opnd { + Opnd::Mem(Mem { base: MemBase::Stack { .. }, .. }) => lower_stack_membase(opnd, stack_state), Opnd::Mem(Mem { base: MemBase::StackIndirect { stack_idx }, disp: opnd_disp, num_bits: opnd_num_bits }) => { // The spilled value (a pointer) lives at a stack slot. Load it // into a scratch register, then use the register as the base. @@ -473,7 +482,7 @@ impl Assembler { *left = split_if_both_memory(asm, *left, *right, SCRATCH0_OPND); *right = split_stack_membase(asm, *right, SCRATCH1_OPND, &stack_state); *right = split_64bit_immediate(asm, *right, SCRATCH1_OPND); - *out = split_stack_membase(asm, *out, SCRATCH1_OPND, &stack_state); + *out = lower_stack_membase(*out, &stack_state); let (out, left) = (*out, *left); asm.push_insn(insn); @@ -484,6 +493,7 @@ impl Assembler { *left = split_if_both_memory(asm, *left, *right, SCRATCH0_OPND); *right = split_stack_membase(asm, *right, SCRATCH1_OPND, &stack_state); *right = split_64bit_immediate(asm, *right, SCRATCH1_OPND); + *out = lower_stack_membase(*out, &stack_state); // imul doesn't have (Mem, Reg) encoding. Swap left and right in that case. if let (Opnd::Mem(_), Opnd::Reg(_)) = (&left, &right) { @@ -544,7 +554,7 @@ impl Assembler { *left = split_stack_membase(asm, *left, SCRATCH1_OPND, &stack_state); *right = split_stack_membase(asm, *right, SCRATCH0_OPND, &stack_state); *right = split_if_both_memory(asm, *right, *left, SCRATCH0_OPND); - *out = split_stack_membase(asm, *out, SCRATCH1_OPND, &stack_state); + *out = lower_stack_membase(*out, &stack_state); let mem_out = split_memory_write(out, SCRATCH0_OPND); asm.push_insn(insn); if let Some(mem_out) = mem_out { @@ -553,7 +563,7 @@ impl Assembler { } Insn::Lea { opnd, out } => { *opnd = split_stack_membase(asm, *opnd, SCRATCH0_OPND, &stack_state); - *out = split_stack_membase(asm, *out, SCRATCH1_OPND, &stack_state); + *out = lower_stack_membase(*out, &stack_state); let mem_out = split_memory_write(out, SCRATCH0_OPND); asm.push_insn(insn); if let Some(mem_out) = mem_out { @@ -570,7 +580,7 @@ impl Assembler { Insn::LoadInto { dest: out, opnd } => { *opnd = split_stack_membase(asm, *opnd, SCRATCH0_OPND, &stack_state); // Split stack membase on out before checking for memory write - *out = split_stack_membase(asm, *out, SCRATCH1_OPND, &stack_state); + *out = lower_stack_membase(*out, &stack_state); let mem_out = split_memory_write(out, SCRATCH0_OPND); asm.push_insn(insn); if let Some(mem_out) = mem_out { From d803c1d16a88f9e50ce19604712894033d41475f Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Tue, 17 Mar 2026 20:03:08 -0700 Subject: [PATCH 09/14] ZJIT: Assert that `out` doesn't use MemBase::StackIndirect --- zjit/src/backend/x86_64/mod.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/zjit/src/backend/x86_64/mod.rs b/zjit/src/backend/x86_64/mod.rs index 35407d5041b61a..476670ee4922ff 100644 --- a/zjit/src/backend/x86_64/mod.rs +++ b/zjit/src/backend/x86_64/mod.rs @@ -389,6 +389,9 @@ impl Assembler { let Mem { base, disp: stack_disp, .. } = stack_state.stack_membase_to_mem(stack_membase); Opnd::Mem(Mem { base, disp: stack_disp + opnd_disp, num_bits: opnd_num_bits }) } + Opnd::Mem(Mem { base: MemBase::StackIndirect { .. }, .. }) => { + unreachable!("lower_stack_membase expects {opnd:?} to not have MemBase::StackIndirect") + } _ => opnd, } } From f9e852d9ed7f2d0ac4497381a22c82c28fbf9a44 Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Tue, 17 Mar 2026 21:17:21 -0700 Subject: [PATCH 10/14] auto_review_pr: suggest re-filing PRs from fork branches (#16446) PRs filed from branches in ruby/ruby (rather than forks) make it harder to manage the bi-directional mirror with git.ruby-lang.org. Add an auto-review comment asking contributors to re-file from a fork. Refactor #review into separate methods for each review type. --- tool/auto_review_pr.rb | 91 +++++++++++++++++++++++++++++++----------- 1 file changed, 67 insertions(+), 24 deletions(-) diff --git a/tool/auto_review_pr.rb b/tool/auto_review_pr.rb index 07c98c7e0aed9e..05aea3ec099c0f 100755 --- a/tool/auto_review_pr.rb +++ b/tool/auto_review_pr.rb @@ -34,46 +34,89 @@ class AutoReviewPR REPO = 'ruby/ruby' COMMENT_USER = 'github-actions[bot]' - COMMENT_PREFIX = 'The following files are maintained in the following upstream repositories:' - COMMENT_SUFFIX = 'Please file a pull request to the above instead. Thank you!' + + UPSTREAM_COMMENT_PREFIX = 'The following files are maintained in the following upstream repositories:' + UPSTREAM_COMMENT_SUFFIX = 'Please file a pull request to the above instead. Thank you!' + + FORK_COMMENT_PREFIX = 'It looks like this pull request was filed from a branch in ruby/ruby.' + FORK_COMMENT_BODY = <<~COMMENT + #{FORK_COMMENT_PREFIX} + + Since ruby/ruby is bi-directionally mirrored with the official git repository at git.ruby-lang.org, \ + having topic branches in ruby/ruby makes it harder to manage the mirror. + + Could you please close this pull request and re-file it from a branch in your personal fork instead? \ + You can fork https://github.com/ruby/ruby, push your branch there, and open a new pull request from it. + + Thank you for your contribution! + COMMENT def initialize(client) @client = client end def review(pr_number) - # Fetch the list of files changed by the PR - changed_files = @client.get("/repos/#{REPO}/pulls/#{pr_number}/files").map { it.fetch(:filename) } + existing_comments = fetch_existing_comments(pr_number) + review_non_fork_branch(pr_number, existing_comments) + review_upstream_repos(pr_number, existing_comments) + end - # Build a Hash: { upstream_repo => files, ... } - upstream_repos = SyncDefaultGems::Repository.group(changed_files) - upstream_repos.delete(nil) # exclude no-upstream files - upstream_repos.delete('prism') if changed_files.include?('prism_compile.c') # allow prism changes in this case - if upstream_repos.empty? - puts "Skipped: The PR ##{pr_number} doesn't have upstream repositories." + private + + def fetch_existing_comments(pr_number) + comments = @client.get("/repos/#{REPO}/issues/#{pr_number}/comments") + comments.map { [it.fetch(:user).fetch(:login), it.fetch(:body)] } + end + + def already_commented?(existing_comments, prefix) + existing_comments.any? { |user, comment| user == COMMENT_USER && comment.start_with?(prefix) } + end + + def post_comment(pr_number, comment) + result = @client.post("/repos/#{REPO}/issues/#{pr_number}/comments", { body: comment }) + puts "Success: #{JSON.pretty_generate(result)}" + end + + # Suggest re-filing from a fork if the PR branch is in ruby/ruby itself + def review_non_fork_branch(pr_number, existing_comments) + if already_commented?(existing_comments, FORK_COMMENT_PREFIX) + puts "Skipped: The PR ##{pr_number} already has a fork branch comment." return end - # Check if the PR is already reviewed - existing_comments = @client.get("/repos/#{REPO}/issues/#{pr_number}/comments") - existing_comments.map! { [it.fetch(:user).fetch(:login), it.fetch(:body)] } - if existing_comments.any? { |user, comment| user == COMMENT_USER && comment.start_with?(COMMENT_PREFIX) } - puts "Skipped: The PR ##{pr_number} already has an automated review comment." + pr = @client.get("/repos/#{REPO}/pulls/#{pr_number}") + head_repo = pr.dig(:head, :repo, :full_name) + if head_repo != REPO + puts "Skipped: The PR ##{pr_number} is already from a fork (#{head_repo})." return end - # Post a comment - comment = format_comment(upstream_repos) - result = @client.post("/repos/#{REPO}/issues/#{pr_number}/comments", { body: comment }) - puts "Success: #{JSON.pretty_generate(result)}" + post_comment(pr_number, FORK_COMMENT_BODY) end - private + # Suggest filing PRs to upstream repositories for files that have one + def review_upstream_repos(pr_number, existing_comments) + if already_commented?(existing_comments, UPSTREAM_COMMENT_PREFIX) + puts "Skipped: The PR ##{pr_number} already has an upstream repos comment." + return + end + + changed_files = @client.get("/repos/#{REPO}/pulls/#{pr_number}/files").map { it.fetch(:filename) } + + upstream_repos = SyncDefaultGems::Repository.group(changed_files) + upstream_repos.delete(nil) + upstream_repos.delete('prism') if changed_files.include?('prism_compile.c') + if upstream_repos.empty? + puts "Skipped: The PR ##{pr_number} doesn't have upstream repositories." + return + end + + post_comment(pr_number, format_upstream_comment(upstream_repos)) + end - # upstream_repos: { upstream_repo => files, ... } - def format_comment(upstream_repos) + def format_upstream_comment(upstream_repos) comment = +'' - comment << "#{COMMENT_PREFIX}\n\n" + comment << "#{UPSTREAM_COMMENT_PREFIX}\n\n" upstream_repos.each do |upstream_repo, files| comment << "* https://github.com/ruby/#{upstream_repo}\n" @@ -82,7 +125,7 @@ def format_comment(upstream_repos) end end - comment << "\n#{COMMENT_SUFFIX}" + comment << "\n#{UPSTREAM_COMMENT_SUFFIX}" comment end end From 283d6cbac264db3f2860f0d4b61975045b381b1c Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Tue, 17 Mar 2026 21:19:39 -0700 Subject: [PATCH 11/14] auto_review_pr: Avoid leaving a review on dependabot PRs --- tool/auto_review_pr.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tool/auto_review_pr.rb b/tool/auto_review_pr.rb index 05aea3ec099c0f..a8f64fab9d1f79 100755 --- a/tool/auto_review_pr.rb +++ b/tool/auto_review_pr.rb @@ -91,6 +91,12 @@ def review_non_fork_branch(pr_number, existing_comments) return end + author = pr.dig(:user, :login) + if author == 'dependabot[bot]' + puts "Skipped: The PR ##{pr_number} is from dependabot." + return + end + post_comment(pr_number, FORK_COMMENT_BODY) end From 314c5fd0b28c3a61408ff46f7304750157eec1e6 Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Tue, 17 Mar 2026 21:24:25 -0700 Subject: [PATCH 12/14] auto_review_pr: Add a workflow_dispatch trigger --- .github/workflows/auto_review_pr.yml | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/.github/workflows/auto_review_pr.yml b/.github/workflows/auto_review_pr.yml index 3e1c0fdc2755bf..e92ad563dd62a8 100644 --- a/.github/workflows/auto_review_pr.yml +++ b/.github/workflows/auto_review_pr.yml @@ -3,6 +3,12 @@ on: pull_request_target: types: [opened, ready_for_review, reopened] branches: [master] + workflow_dispatch: + inputs: + pr_number: + description: 'PR number to review' + required: true + type: number permissions: contents: read @@ -11,7 +17,7 @@ jobs: auto-review-pr: name: Auto Review PR runs-on: ubuntu-latest - if: ${{ github.repository == 'ruby/ruby' && github.base_ref == 'master' }} + if: ${{ github.repository == 'ruby/ruby' && (github.base_ref == 'master' || github.event_name == 'workflow_dispatch') }} permissions: pull-requests: write @@ -32,4 +38,4 @@ jobs: run: ruby tool/auto_review_pr.rb "$GITHUB_PR_NUMBER" env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - GITHUB_PR_NUMBER: ${{ github.event.pull_request.number }} + GITHUB_PR_NUMBER: ${{ github.event.pull_request.number || github.event.inputs.pr_number }} From 8581ba91342de65156182131ada9578e53653bdc Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Tue, 17 Mar 2026 21:31:32 -0700 Subject: [PATCH 13/14] Fix race condition in test_condvar_wait_and_broadcast (#16442) Replace `sleep 0.1` with `Thread.pass until threads.all?(&:stop?)` to ensure all worker threads have entered `condvar.wait` before broadcasting. The fixed sleep was insufficient on slow CI environments (e.g. Windows 2025), causing broadcast to fire before threads were waiting, leading to a deadlock and 60-second timeout failure. This matches the synchronization pattern already used in test_condvar_wait_exception_handling in the same file. --- test/ruby/test_thread_cv.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/ruby/test_thread_cv.rb b/test/ruby/test_thread_cv.rb index 81201f134f0c71..e5fd513c5c3ffa 100644 --- a/test/ruby/test_thread_cv.rb +++ b/test/ruby/test_thread_cv.rb @@ -70,7 +70,7 @@ def test_condvar_wait_and_broadcast end end end - sleep 0.1 + Thread.pass until threads.all?(&:stop?) mutex.synchronize do result << "P1" condvar.broadcast From 95af086784bfae3f893fd8a9bf6e7d8f99f67302 Mon Sep 17 00:00:00 2001 From: Nozomi Hijikata <121233810+nozomemein@users.noreply.github.com> Date: Wed, 18 Mar 2026 13:53:36 +0900 Subject: [PATCH 14/14] ZJIT: Inline String#!= by negating String#== (#16391) Looking at the profiling results below, we can likely inline `String#!=` similarly to how we already handle `Integer` (and some `BasicObject#!=` cases). As Max mentioned in [the original issue](https://github.com/Shopify/ruby/issues/884#issuecomment-3923486140), we may be able to do more once the inliner lands. For now, this seems like a reasonable scope, since `String` is still a meaningful share in these benchmarks. Related discussion: - https://github.com/Shopify/ruby/issues/884 - https://github.com/ruby/ruby/pull/16057#issuecomment-3923785198 --- `BasicObject#!=` receiver/argument profiling results: ``` # railsbench Top-7 BasicObject#!= receiver/argument class buckets (100.0% of total 929,895): Other->Other: 599,324 (64.5%) Integer->Integer: 155,592 (16.7%) String->String: 125,159 (13.5%) NilClass->String: 49,646 ( 5.3%) Integer->Other: 170 ( 0.0%) NilClass->Other: 2 ( 0.0%) # lobsters Top-8 BasicObject#!= receiver/argument class buckets (100.0% of total 452,759): Other->Other: 251,034 (55.4%) Integer->Integer: 165,871 (36.6%) String->String: 29,478 ( 6.5%) NilClass->Other: 5,720 ( 1.3%) NilClass->NilClass: 346 ( 0.1%) Integer->Other: 171 ( 0.0%) String->Other: 138 ( 0.0%) NilClass->Integer: 1 ( 0.0%) ``` String specialization coverage after this change: (the number of successful `String#!=` inlines)) ``` # railsbench basic_object_neq_string_specialized: 125,159 (100.0% of string-typed BasicObject#!=) # lobsters basic_object_neq_string_specialized: 29,478 (99.5% of string-typed BasicObject#!=) ``` --- zjit/src/codegen_tests.rs | 49 +++++++++++++++++++++++ zjit/src/cruby_methods.rs | 39 +++++++++++++++--- zjit/src/hir/opt_tests.rs | 83 +++++++++++++++++++++++++++++++++++++-- 3 files changed, 163 insertions(+), 8 deletions(-) diff --git a/zjit/src/codegen_tests.rs b/zjit/src/codegen_tests.rs index a65949aa2ed397..c3b67b9ff89865 100644 --- a/zjit/src/codegen_tests.rs +++ b/zjit/src/codegen_tests.rs @@ -1905,6 +1905,55 @@ fn test_opt_neq_fixnum() { "), @"[false, true]"); } +#[test] +fn test_opt_neq_string_nil() { + assert_snapshot!(inspect(r#" + def test(str) = str != nil + test("x") # profile opt_neq + [test("x"), test(nil)] + "#), @"[true, false]"); +} + +#[test] +fn test_opt_neq_string_same_operand() { + assert_snapshot!(inspect(r#" + def test(s) = s != s + test("x") # profile opt_neq + [test("x"), test("y")] + "#), @"[false, false]"); + assert_contains_opcode("test", YARVINSN_opt_neq); +} + +#[test] +fn test_opt_neq_string_distinct_literals() { + assert_snapshot!(inspect(r#" + def test = "a" != "b" + test # profile opt_neq + [test, test] + "#), @"[true, true]"); + assert_contains_opcode("test", YARVINSN_opt_neq); +} + +#[test] +fn test_opt_neq_string_one_side_known_literal() { + assert_snapshot!(inspect(r#" + def test(s) = "a" != s + test("a") # profile opt_neq + [test("a"), test("b")] + "#), @"[false, true]"); + assert_contains_opcode("test", YARVINSN_opt_neq); +} + +#[test] +fn test_opt_neq_string_distinct_objects() { + assert_snapshot!(inspect(r#" + def test(s, t) = s != t + test("x", "x") # profile opt_neq + [test("x", "x"), test("x", "y")] + "#), @"[false, true]"); + assert_contains_opcode("test", YARVINSN_opt_neq); +} + #[test] fn test_opt_eq_string_same_operand() { assert_snapshot!(inspect(r#" diff --git a/zjit/src/cruby_methods.rs b/zjit/src/cruby_methods.rs index 675e1308b526da..d39f38028743c5 100644 --- a/zjit/src/cruby_methods.rs +++ b/zjit/src/cruby_methods.rs @@ -19,6 +19,7 @@ unsafe extern "C" { fn rb_jit_ary_at_end(ec: EcPtr, self_: VALUE, index: VALUE) -> VALUE; fn rb_jit_ary_at(ec: EcPtr, self_: VALUE, index: VALUE) -> VALUE; fn rb_jit_fixnum_inc(ec: EcPtr, self_: VALUE, num: VALUE) -> VALUE; + fn rb_str_equal(str1: VALUE, str2: VALUE) -> VALUE; } pub struct Annotations { @@ -543,8 +544,7 @@ fn inline_string_append(fun: &mut hir::Function, block: hir::BlockId, recv: hir: None } -fn inline_string_eq(fun: &mut hir::Function, block: hir::BlockId, recv: hir::InsnId, args: &[hir::InsnId], state: hir::InsnId) -> Option { - let &[other] = args else { return None; }; +fn try_inline_string_equal(fun: &mut hir::Function, block: hir::BlockId, recv: hir::InsnId, other: hir::InsnId, state: hir::InsnId) -> Option { if fun.likely_a(recv, types::String, state) && fun.likely_a(other, types::String, state) { let recv = fun.coerce_to(block, recv, types::String, state); let other = fun.coerce_to(block, other, types::String, state); @@ -554,6 +554,11 @@ fn inline_string_eq(fun: &mut hir::Function, block: hir::BlockId, recv: hir::Ins None } +fn inline_string_eq(fun: &mut hir::Function, block: hir::BlockId, recv: hir::InsnId, args: &[hir::InsnId], state: hir::InsnId) -> Option { + let &[other] = args else { return None; }; + try_inline_string_equal(fun, block, recv, other, state) +} + fn inline_module_eqq(fun: &mut hir::Function, block: hir::BlockId, recv: hir::InsnId, args: &[hir::InsnId], _state: hir::InsnId) -> Option { let &[other] = args else { return None; }; if fun.is_a(recv, types::Class) { @@ -737,12 +742,36 @@ fn inline_basic_object_not(fun: &mut hir::Function, block: hir::BlockId, recv: h None } +fn try_inline_string_not_equal(fun: &mut hir::Function, block: hir::BlockId, recv: hir::InsnId, other: hir::InsnId, state: hir::InsnId) -> Option { + if !fun.likely_a(recv, types::String, state) || !fun.likely_a(other, types::String, state) { + return None; + } + let recv_class = fun.type_of(recv).runtime_exact_ruby_class()?; + + // String#!= is lowered to #==. Keep this specialization only while #== + // still resolves to rb_str_equal. + if !fun.assume_expected_cfunc(block, recv_class, ID!(eq), rb_str_equal as _, state) { + return None; + } + + let eq_result = try_inline_string_equal(fun, block, recv, other, state)?; + // StringEqual always returns a Ruby boolean (Qtrue/Qfalse), + // so `!=` can be lowered to `eq_result != Qtrue`. + let true_val = fun.push_insn(block, hir::Insn::Const { val: hir::Const::Value(Qtrue) }); + let not_equal = fun.push_insn(block, hir::Insn::IsBitNotEqual { left: eq_result, right: true_val }); + Some(fun.push_insn(block, hir::Insn::BoxBool { val: not_equal })) +} + fn inline_basic_object_neq(fun: &mut hir::Function, block: hir::BlockId, recv: hir::InsnId, args: &[hir::InsnId], state: hir::InsnId) -> Option { let &[other] = args else { return None; }; - let result = try_inline_fixnum_op(fun, block, &|left, right| hir::Insn::FixnumNeq { left, right }, BOP_NEQ, recv, other, state); - if result.is_some() { - return result; + if let Some(result) = try_inline_fixnum_op(fun, block, &|left, right| hir::Insn::FixnumNeq { left, right }, BOP_NEQ, recv, other, state) { + return Some(result); } + + if let Some(result) = try_inline_string_not_equal(fun, block, recv, other, state) { + return Some(result); + } + let recv_class = fun.type_of(recv).runtime_exact_ruby_class()?; if !fun.assume_expected_cfunc(block, recv_class, ID!(eq), rb_obj_equal as _, state) { return None; diff --git a/zjit/src/hir/opt_tests.rs b/zjit/src/hir/opt_tests.rs index 5df49c5ae6ba8f..aa706f7ddaeadd 100644 --- a/zjit/src/hir/opt_tests.rs +++ b/zjit/src/hir/opt_tests.rs @@ -11579,7 +11579,79 @@ mod hir_opt_tests { } #[test] - fn test_not_fold_string_not_equal_same_operand_false() { + fn opt_neq_string_nil_falls_back_to_basic_object_neq() { + eval(r#" + def test(str) + str != nil + end + + test("x") + test("x") + "#); + assert_snapshot!(hir_string("test"), @" + fn test@:3: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:CPtr = LoadSP + v3:BasicObject = LoadField v2, :str@0x1000 + Jump bb3(v1, v3) + bb2(): + EntryPoint JIT(0) + v6:BasicObject = LoadArg :self@0 + v7:BasicObject = LoadArg :str@1 + Jump bb3(v6, v7) + bb3(v9:BasicObject, v10:BasicObject): + v15:NilClass = Const Value(nil) + PatchPoint NoSingletonClass(String@0x1008) + PatchPoint MethodRedefined(String@0x1008, !=@0x1010, cme:0x1018) + v27:StringExact = GuardType v10, StringExact + v28:BoolExact = CCallWithFrame v27, :BasicObject#!=@0x1040, v15 + CheckInterrupts + Return v28 + "); + } + + #[test] + fn test_inline_string_not_equal_distinct_objects() { + eval(r#" + def test(s, t) = s != t + test("x", "x") + test("x", "x") + "#); + assert_contains_opcode("test", YARVINSN_opt_neq); + assert_snapshot!(hir_string("test"), @" + fn test@:2: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:CPtr = LoadSP + v3:BasicObject = LoadField v2, :s@0x1000 + v4:BasicObject = LoadField v2, :t@0x1001 + Jump bb3(v1, v3, v4) + bb2(): + EntryPoint JIT(0) + v7:BasicObject = LoadArg :self@0 + v8:BasicObject = LoadArg :s@1 + v9:BasicObject = LoadArg :t@2 + Jump bb3(v7, v8, v9) + bb3(v11:BasicObject, v12:BasicObject, v13:BasicObject): + PatchPoint NoSingletonClass(String@0x1008) + PatchPoint MethodRedefined(String@0x1008, !=@0x1010, cme:0x1018) + v29:StringExact = GuardType v12, StringExact + PatchPoint MethodRedefined(String@0x1008, ==@0x1040, cme:0x1048) + v33:String = GuardType v13, String + v34:BoolExact = StringEqual v29, v33 + v35:TrueClass = Const Value(true) + v36:CBool = IsBitNotEqual v34, v35 + v37:BoolExact = BoxBool v36 + CheckInterrupts + Return v37 + "); + } + + #[test] + fn test_fold_string_not_equal_same_operand_false() { eval(r#" def test(s) = s != s test("x") @@ -11603,9 +11675,14 @@ mod hir_opt_tests { PatchPoint NoSingletonClass(String@0x1008) PatchPoint MethodRedefined(String@0x1008, !=@0x1010, cme:0x1018) v26:StringExact = GuardType v10, StringExact - v27:BoolExact = CCallWithFrame v26, :BasicObject#!=@0x1040, v10 + PatchPoint MethodRedefined(String@0x1008, ==@0x1040, cme:0x1048) + v30:String = GuardType v10, String + v35:TrueClass = Const Value(true) + v32:TrueClass = Const Value(true) + v33:CBool = IsBitNotEqual v35, v32 + v34:BoolExact = BoxBool v33 CheckInterrupts - Return v27 + Return v34 "); }