diff --git a/.github/workflows/sync_default_gems.yml b/.github/workflows/sync_default_gems.yml index 3a811be18a3187..2f0b4d8865eb4f 100644 --- a/.github/workflows/sync_default_gems.yml +++ b/.github/workflows/sync_default_gems.yml @@ -32,6 +32,11 @@ jobs: with: token: ${{ github.repository == 'ruby/ruby' && secrets.MATZBOT_AUTO_UPDATE_TOKEN || secrets.GITHUB_TOKEN }} + - uses: ruby/setup-ruby@ab177d40ee5483edb974554986f56b33477e21d0 # v1.265.0 + with: + ruby-version: '3.4' + bundler: none + - name: Run tool/sync_default_gems.rb id: sync run: | diff --git a/ext/strscan/extconf.rb b/ext/strscan/extconf.rb index 3c311d2364b7d7..d2e9343cbced7f 100644 --- a/ext/strscan/extconf.rb +++ b/ext/strscan/extconf.rb @@ -4,6 +4,7 @@ $INCFLAGS << " -I$(top_srcdir)" if $extmk have_func("onig_region_memsize(NULL)") have_func("rb_reg_onig_match", "ruby/re.h") + have_func("rb_deprecate_constant") create_makefile 'strscan' else File.write('Makefile', dummy_makefile("").join) diff --git a/ext/strscan/strscan.c b/ext/strscan/strscan.c index e2b827c63c9a3f..4746afd5178d7c 100644 --- a/ext/strscan/strscan.c +++ b/ext/strscan/strscan.c @@ -24,6 +24,14 @@ extern size_t onig_region_memsize(const struct re_registers *regs); #define STRSCAN_VERSION "3.1.6.dev" + +#ifdef HAVE_RB_DEPRECATE_CONSTANT +/* In ruby 3.0, defined but exposed in external headers */ +extern void rb_deprecate_constant(VALUE mod, const char *name); +#else +# define rb_deprecate_constant(mod, name) ((void)0) +#endif + /* ======================================================================= Data Type Definitions ======================================================================= */ @@ -1274,7 +1282,7 @@ static VALUE strscan_scan_base10_integer(VALUE self) { char *ptr; - long len = 0; + long len = 0, remaining_len; struct strscanner *p; GET_SCANNER(self, p); @@ -1284,7 +1292,7 @@ strscan_scan_base10_integer(VALUE self) ptr = CURPTR(p); - long remaining_len = S_RESTLEN(p); + remaining_len = S_RESTLEN(p); if (remaining_len <= 0) { return Qnil; @@ -1311,7 +1319,7 @@ static VALUE strscan_scan_base16_integer(VALUE self) { char *ptr; - long len = 0; + long len = 0, remaining_len; struct strscanner *p; GET_SCANNER(self, p); @@ -1321,7 +1329,7 @@ strscan_scan_base16_integer(VALUE self) ptr = CURPTR(p); - long remaining_len = S_RESTLEN(p); + remaining_len = S_RESTLEN(p); if (remaining_len <= 0) { return Qnil; @@ -1477,7 +1485,6 @@ strscan_eos_p(VALUE self) * rest? * * Returns true if and only if there is more data in the string. See #eos?. - * This method is obsolete; use #eos? instead. * * s = StringScanner.new('test string') * # These two are opposites @@ -1605,7 +1612,7 @@ name_to_backref_number(struct re_registers *regs, VALUE regexp, const char* name (const unsigned char* )name_end, regs); if (num >= 1) { - return num; + return num; } } rb_enc_raise(enc, rb_eIndexError, "undefined group name reference: %.*s", @@ -2211,6 +2218,7 @@ Init_strscan(void) ScanError = rb_define_class_under(StringScanner, "Error", rb_eStandardError); if (!rb_const_defined(rb_cObject, id_scanerr)) { rb_const_set(rb_cObject, id_scanerr, ScanError); + rb_deprecate_constant(rb_cObject, "ScanError"); } tmp = rb_str_new2(STRSCAN_VERSION); rb_obj_freeze(tmp); diff --git a/pathname_builtin.rb b/pathname_builtin.rb index 16ed219ec38c06..1dedf5e08df546 100644 --- a/pathname_builtin.rb +++ b/pathname_builtin.rb @@ -217,7 +217,8 @@ class Pathname def initialize(path) unless String === path path = path.to_path if path.respond_to? :to_path - raise TypeError unless String === path + path = path.to_str if path.respond_to? :to_str + raise TypeError, "Pathname.new requires a String, #to_path or #to_str" unless String === path end if path.include?("\0") diff --git a/spec/ruby/library/stringscanner/unscan_spec.rb b/spec/ruby/library/stringscanner/unscan_spec.rb index df0ea433674a29..b7b4876b8afa6a 100644 --- a/spec/ruby/library/stringscanner/unscan_spec.rb +++ b/spec/ruby/library/stringscanner/unscan_spec.rb @@ -21,8 +21,8 @@ @s.pos.should == pos end - it "raises a ScanError when the previous match had failed" do - -> { @s.unscan }.should raise_error(ScanError) - -> { @s.scan(/\d/); @s.unscan }.should raise_error(ScanError) + it "raises a StringScanner::Error when the previous match had failed" do + -> { @s.unscan }.should raise_error(StringScanner::Error) + -> { @s.scan(/\d/); @s.unscan }.should raise_error(StringScanner::Error) end end diff --git a/test/pathname/test_pathname.rb b/test/pathname/test_pathname.rb index e80473e5a3eea0..e2d6a09fb2f0c0 100644 --- a/test/pathname/test_pathname.rb +++ b/test/pathname/test_pathname.rb @@ -484,6 +484,10 @@ def test_initialize assert_equal('a', p1.to_s) p2 = Pathname.new(p1) assert_equal(p1, p2) + + obj = Object.new + def obj.to_str; "a/b"; end + assert_equal("a/b", Pathname.new(obj).to_s) end def test_initialize_nul diff --git a/test/strscan/test_stringscanner.rb b/test/strscan/test_stringscanner.rb index 085a9113132b75..8218e5b6bebc85 100644 --- a/test/strscan/test_stringscanner.rb +++ b/test/strscan/test_stringscanner.rb @@ -875,7 +875,7 @@ def test_unscan assert_equal({}, s.named_captures) assert_equal("te", s.scan(/../)) assert_equal(nil, s.scan(/\d/)) - assert_raise(ScanError) { s.unscan } + assert_raise(StringScanner::Error) { s.unscan } end def test_rest diff --git a/tool/sync_default_gems.rb b/tool/sync_default_gems.rb index f1a28f402a7e83..03f9625bfa5cd9 100755 --- a/tool/sync_default_gems.rb +++ b/tool/sync_default_gems.rb @@ -4,6 +4,8 @@ require 'fileutils' require "rbconfig" +require "find" +require "tempfile" module SyncDefaultGems include FileUtils @@ -11,76 +13,289 @@ module SyncDefaultGems module_function - REPOSITORIES = { - "io-console": 'ruby/io-console', - "io-nonblock": 'ruby/io-nonblock', - "io-wait": 'ruby/io-wait', - "net-http": "ruby/net-http", - "net-protocol": "ruby/net-protocol", - "open-uri": "ruby/open-uri", - "win32-registry": "ruby/win32-registry", - English: "ruby/English", - cgi: "ruby/cgi", - date: 'ruby/date', - delegate: "ruby/delegate", - did_you_mean: "ruby/did_you_mean", - digest: "ruby/digest", - erb: "ruby/erb", - error_highlight: "ruby/error_highlight", - etc: 'ruby/etc', - fcntl: 'ruby/fcntl', - fileutils: 'ruby/fileutils', - find: "ruby/find", - forwardable: "ruby/forwardable", - ipaddr: 'ruby/ipaddr', - json: 'ruby/json', - mmtk: ['ruby/mmtk', "main"], - open3: "ruby/open3", - openssl: "ruby/openssl", - optparse: "ruby/optparse", - pp: "ruby/pp", - prettyprint: "ruby/prettyprint", - prism: ["ruby/prism", "main"], - psych: 'ruby/psych', - resolv: "ruby/resolv", - rubygems: 'ruby/rubygems', - securerandom: "ruby/securerandom", - shellwords: "ruby/shellwords", - singleton: "ruby/singleton", - stringio: 'ruby/stringio', - strscan: 'ruby/strscan', - syntax_suggest: ["ruby/syntax_suggest", "main"], - tempfile: "ruby/tempfile", - time: "ruby/time", - timeout: "ruby/timeout", - tmpdir: "ruby/tmpdir", - tsort: "ruby/tsort", - un: "ruby/un", - uri: "ruby/uri", - weakref: "ruby/weakref", - yaml: "ruby/yaml", - zlib: 'ruby/zlib', - }.transform_keys(&:to_s) + # upstream: "owner/repo" + # branch: "branch_name" + # mappings: [ ["path_in_upstream", "path_in_ruby"], ... ] + # NOTE: path_in_ruby is assumed to be "owned" by this gem, and the contents + # will be removed before sync + # exclude: [ "fnmatch_pattern_after_mapping", ... ] + Repository = Data.define(:upstream, :branch, :mappings, :exclude) do + def excluded?(newpath) + p = newpath + until p == "." + return true if exclude.any? {|pat| File.fnmatch?(pat, p, File::FNM_PATHNAME|File::FNM_EXTGLOB)} + p = File.dirname(p) + end + false + end + + def rewrite_for_ruby(path) + newpath = mappings.find do |src, dst| + if path == src || path.start_with?(src + "/") + break path.sub(src, dst) + end + end + return nil unless newpath + return nil if excluded?(newpath) + newpath + end + end CLASSICAL_DEFAULT_BRANCH = "master" + def repo((upstream, branch), mappings, exclude: []) + branch ||= CLASSICAL_DEFAULT_BRANCH + exclude += ["ext/**/depend"] + Repository.new(upstream:, branch:, mappings:, exclude:) + end + + def lib((upstream, branch), gemspec_in_subdir: false) + _org, name = upstream.split("/") + gemspec_dst = gemspec_in_subdir ? "lib/#{name}/#{name}.gemspec" : "lib/#{name}.gemspec" + repo([upstream, branch], [ + ["lib/#{name}.rb", "lib/#{name}.rb"], + ["lib/#{name}", "lib/#{name}"], + ["test/test_#{name}.rb", "test/test_#{name}.rb"], + ["test/#{name}", "test/#{name}"], + ["#{name}.gemspec", gemspec_dst], + ]) + end + + REPOSITORIES = { + "io-console": repo("ruby/io-console", [ + ["ext/io/console", "ext/io/console"], + ["test/io/console", "test/io/console"], + ["lib/io/console", "ext/io/console/lib/console"], + ["io-console.gemspec", "ext/io/console/io-console.gemspec"], + ]), + "io-nonblock": repo("ruby/io-nonblock", [ + ["ext/io/nonblock", "ext/io/nonblock"], + ["test/io/nonblock", "test/io/nonblock"], + ["io-nonblock.gemspec", "ext/io/nonblock/io-nonblock.gemspec"], + ]), + "io-wait": repo("ruby/io-wait", [ + ["ext/io/wait", "ext/io/wait"], + ["test/io/wait", "test/io/wait"], + ["io-wait.gemspec", "ext/io/wait/io-wait.gemspec"], + ]), + "net-http": repo("ruby/net-http", [ + ["lib/net/http.rb", "lib/net/http.rb"], + ["lib/net/http", "lib/net/http"], + ["test/net/http", "test/net/http"], + ["net-http.gemspec", "lib/net/http/net-http.gemspec"], + ]), + "net-protocol": repo("ruby/net-protocol", [ + ["lib/net/protocol.rb", "lib/net/protocol.rb"], + ["test/net/protocol", "test/net/protocol"], + ["net-protocol.gemspec", "lib/net/net-protocol.gemspec"], + ]), + "open-uri": lib("ruby/open-uri"), + "win32-registry": repo("ruby/win32-registry", [ + ["lib/win32/registry.rb", "ext/win32/lib/win32/registry.rb"], + ["test/win32/test_registry.rb", "test/win32/test_registry.rb"], + ["win32-registry.gemspec", "ext/win32/win32-registry.gemspec"], + ]), + English: lib("ruby/English"), + cgi: repo("ruby/cgi", [ + ["ext/cgi", "ext/cgi"], + ["lib/cgi/escape.rb", "lib/cgi/escape.rb"], + ["test/cgi/test_cgi_escape.rb", "test/cgi/test_cgi_escape.rb"], + ["test/cgi/update_env.rb", "test/cgi/update_env.rb"], + ]), + date: repo("ruby/date", [ + ["doc/date", "doc/date"], + ["ext/date", "ext/date"], + ["lib", "ext/date/lib"], + ["test/date", "test/date"], + ["date.gemspec", "ext/date/date.gemspec"], + ], exclude: [ + "ext/date/lib/date_core.bundle", + ]), + delegate: lib("ruby/delegate"), + did_you_mean: repo("ruby/did_you_mean", [ + ["lib/did_you_mean.rb", "lib/did_you_mean.rb"], + ["lib/did_you_mean", "lib/did_you_mean"], + ["test", "test/did_you_mean"], + ["did_you_mean.gemspec", "lib/did_you_mean/did_you_mean.gemspec"], + ], exclude: [ + "test/did_you_mean/lib", + "test/did_you_mean/tree_spell/test_explore.rb", + ]), + digest: repo("ruby/digest", [ + ["ext/digest/lib/digest/sha2", "ext/digest/sha2/lib/sha2"], + ["ext/digest", "ext/digest"], + ["lib/digest.rb", "ext/digest/lib/digest.rb"], + ["lib/digest/version.rb", "ext/digest/lib/digest/version.rb"], + ["lib/digest/sha2.rb", "ext/digest/sha2/lib/sha2.rb"], + ["test/digest", "test/digest"], + ["digest.gemspec", "ext/digest/digest.gemspec"], + ]), + erb: repo("ruby/erb", [ + ["ext/erb", "ext/erb"], + ["lib/erb", "lib/erb"], + ["lib/erb.rb", "lib/erb.rb"], + ["test/erb", "test/erb"], + ["erb.gemspec", "lib/erb/erb.gemspec"], + ["libexec/erb", "libexec/erb"], + ]), + error_highlight: repo("ruby/error_highlight", [ + ["lib/error_highlight.rb", "lib/error_highlight.rb"], + ["lib/error_highlight", "lib/error_highlight"], + ["test", "test/error_highlight"], + ["error_highlight.gemspec", "lib/error_highlight/error_highlight.gemspec"], + ]), + etc: repo("ruby/etc", [ + ["ext/etc", "ext/etc"], + ["test/etc", "test/etc"], + ["etc.gemspec", "ext/etc/etc.gemspec"], + ]), + fcntl: repo("ruby/fcntl", [ + ["ext/fcntl", "ext/fcntl"], + ["fcntl.gemspec", "ext/fcntl/fcntl.gemspec"], + ]), + fileutils: lib("ruby/fileutils"), + find: lib("ruby/find"), + forwardable: lib("ruby/forwardable", gemspec_in_subdir: true), + ipaddr: lib("ruby/ipaddr"), + json: repo("ruby/json", [ + ["ext/json/ext", "ext/json"], + ["test/json", "test/json"], + ["lib", "ext/json/lib"], + ["json.gemspec", "ext/json/json.gemspec"], + ], exclude: [ + "ext/json/lib/json/ext/.keep", + "ext/json/lib/json/pure.rb", + "ext/json/lib/json/pure", + "ext/json/lib/json/truffle_ruby", + "test/json/lib", + "ext/json/extconf.rb", + ]), + mmtk: repo(["ruby/mmtk", "main"], [ + ["gc/mmtk", "gc/mmtk"], + ]), + open3: lib("ruby/open3", gemspec_in_subdir: true).tap { + it.exclude << "lib/open3/jruby_windows.rb" + }, + openssl: repo("ruby/openssl", [ + ["ext/openssl", "ext/openssl"], + ["lib", "ext/openssl/lib"], + ["test/openssl", "test/openssl"], + ["sample", "sample/openssl"], + ["openssl.gemspec", "ext/openssl/openssl.gemspec"], + ["History.md", "ext/openssl/History.md"], + ], exclude: [ + "test/openssl/envutil.rb", + "ext/openssl/depend", + ]), + optparse: lib("ruby/optparse", gemspec_in_subdir: true).tap { + it.mappings << ["doc/optparse", "doc/optparse"] + }, + pp: lib("ruby/pp"), + prettyprint: lib("ruby/prettyprint"), + prism: repo(["ruby/prism", "main"], [ + ["ext/prism", "prism"], + ["lib/prism.rb", "lib/prism.rb"], + ["lib/prism", "lib/prism"], + ["test/prism", "test/prism"], + ["src", "prism"], + ["prism.gemspec", "lib/prism/prism.gemspec"], + ["include/prism", "prism"], + ["include/prism.h", "prism/prism.h"], + ["config.yml", "prism/config.yml"], + ["templates", "prism/templates"], + ], exclude: [ + "prism/templates/{javascript,java,rbi,sig}", + "test/prism/snapshots_test.rb", + "test/prism/snapshots", + "prism/extconf.rb", + "prism/srcs.mk*", + ]), + psych: repo("ruby/psych", [ + ["ext/psych", "ext/psych"], + ["lib", "ext/psych/lib"], + ["test/psych", "test/psych"], + ["psych.gemspec", "ext/psych/psych.gemspec"], + ], exclude: [ + "ext/psych/lib/org", + "ext/psych/lib/psych.jar", + "ext/psych/lib/psych_jars.rb", + "ext/psych/lib/psych.{bundle,so}", + "ext/psych/lib/2.*", + "ext/psych/yaml/LICENSE", + "ext/psych/.gitignore", + ]), + resolv: repo("ruby/resolv", [ + ["lib/resolv.rb", "lib/resolv.rb"], + ["test/resolv", "test/resolv"], + ["resolv.gemspec", "lib/resolv.gemspec"], + ["ext/win32/resolv/lib/resolv.rb", "ext/win32/lib/win32/resolv.rb"], + ["ext/win32/resolv", "ext/win32/resolv"], + ]), + rubygems: repo("ruby/rubygems", [ + ["lib/rubygems.rb", "lib/rubygems.rb"], + ["lib/rubygems", "lib/rubygems"], + ["test/rubygems", "test/rubygems"], + ["bundler/lib/bundler.rb", "lib/bundler.rb"], + ["bundler/lib/bundler", "lib/bundler"], + ["bundler/exe/bundle", "libexec/bundle"], + ["bundler/exe/bundler", "libexec/bundler"], + ["bundler/bundler.gemspec", "lib/bundler/bundler.gemspec"], + ["bundler/spec", "spec/bundler"], + *["bundle", "parallel_rspec", "rspec"].map {|binstub| + ["bundler/bin/#{binstub}", "spec/bin/#{binstub}"] + }, + *%w[dev_gems test_gems rubocop_gems standard_gems].flat_map {|gemfile| + ["rb.lock", "rb"].map do |ext| + ["tool/bundler/#{gemfile}.#{ext}", "tool/bundler/#{gemfile}.#{ext}"] + end + }, + ], exclude: [ + "spec/bundler/bin", + "spec/bundler/support/artifice/vcr_cassettes", + "spec/bundler/support/artifice/used_cassettes.txt", + "lib/{bundler,rubygems}/**/{COPYING,LICENSE,README}{,.{md,txt,rdoc}}", + ]), + securerandom: lib("ruby/securerandom"), + shellwords: lib("ruby/shellwords"), + singleton: lib("ruby/singleton"), + stringio: repo("ruby/stringio", [ + ["ext/stringio", "ext/stringio"], + ["test/stringio", "test/stringio"], + ["stringio.gemspec", "ext/stringio/stringio.gemspec"], + ], exclude: [ + "ext/stringio/README.md", + ]), + strscan: repo("ruby/strscan", [ + ["ext/strscan", "ext/strscan"], + ["lib", "ext/strscan/lib"], + ["test/strscan", "test/strscan"], + ["strscan.gemspec", "ext/strscan/strscan.gemspec"], + ["doc/strscan", "doc/strscan"], + ], exclude: [ + "ext/strscan/regenc.h", + "ext/strscan/regint.h", + ]), + syntax_suggest: lib(["ruby/syntax_suggest", "main"], gemspec_in_subdir: true), + tempfile: lib("ruby/tempfile"), + time: lib("ruby/time"), + timeout: lib("ruby/timeout"), + tmpdir: lib("ruby/tmpdir"), + tsort: lib("ruby/tsort"), + un: lib("ruby/un"), + uri: lib("ruby/uri", gemspec_in_subdir: true), + weakref: lib("ruby/weakref"), + yaml: lib("ruby/yaml", gemspec_in_subdir: true), + zlib: repo("ruby/zlib", [ + ["ext/zlib", "ext/zlib"], + ["test/zlib", "test/zlib"], + ["zlib.gemspec", "ext/zlib/zlib.gemspec"], + ]), + }.transform_keys(&:to_s) + # Allow synchronizing commits up to this FETCH_DEPTH. We've historically merged PRs # with about 250 commits to ruby/ruby, so we use this depth for ruby/ruby in general. FETCH_DEPTH = 500 - class << REPOSITORIES - def [](gem) - repo, branch = super(gem) - return repo, branch || CLASSICAL_DEFAULT_BRANCH - end - - def each_pair - super do |gem, (repo, branch)| - yield gem, [repo, branch || CLASSICAL_DEFAULT_BRANCH] - end - end - end - def pipe_readlines(args, rs: "\0", chomp: true) IO.popen(args) do |f| f.readlines(rs, chomp: chomp) @@ -88,7 +303,7 @@ def pipe_readlines(args, rs: "\0", chomp: true) end def porcelain_status(*pattern) - pipe_readlines(%W"git status --porcelain -z --" + pattern) + pipe_readlines(%W"git status --porcelain --no-renames -z --" + pattern) end def replace_rdoc_ref(file) @@ -117,247 +332,65 @@ def replace_rdoc_ref_all result.inject(false) {|changed, file| changed | replace_rdoc_ref(file)} end + def replace_rdoc_ref_all_full + Dir.glob("**/*.{c,rb,rdoc}").inject(false) {|changed, file| changed | replace_rdoc_ref(file)} + end + + def rubygems_do_fixup + gemspec_content = File.readlines("lib/bundler/bundler.gemspec").map do |line| + next if line =~ /LICENSE\.md/ + + line.gsub("bundler.gemspec", "lib/bundler/bundler.gemspec") + end.compact.join + File.write("lib/bundler/bundler.gemspec", gemspec_content) + + ["bundle", "parallel_rspec", "rspec"].each do |binstub| + path = "spec/bin/#{binstub}" + next unless File.exist?(path) + content = File.read(path).gsub("../spec", "../bundler") + File.write(path, content) + chmod("+x", path) + end + end + # We usually don't use this. Please consider using #sync_default_gems_with_commits instead. def sync_default_gems(gem) - repo, = REPOSITORIES[gem] - puts "Sync #{repo}" - - upstream = File.join("..", "..", repo) - - case gem - when "rubygems" - rm_rf(%w[lib/rubygems lib/rubygems.rb test/rubygems]) - cp_r(Dir.glob("#{upstream}/lib/rubygems*"), "lib") - cp_r("#{upstream}/test/rubygems", "test") - rm_rf(%w[lib/bundler lib/bundler.rb libexec/bundler libexec/bundle spec/bundler tool/bundler/*]) - cp_r(Dir.glob("#{upstream}/bundler/lib/bundler*"), "lib") - cp_r(Dir.glob("#{upstream}/bundler/exe/bundle*"), "libexec") - - gemspec_content = File.readlines("#{upstream}/bundler/bundler.gemspec").map do |line| - next if line =~ /LICENSE\.md/ - - line.gsub("bundler.gemspec", "lib/bundler/bundler.gemspec") - end.compact.join - File.write("lib/bundler/bundler.gemspec", gemspec_content) - - cp_r("#{upstream}/bundler/spec", "spec/bundler") - rm_rf("spec/bundler/bin") - - ["bundle", "parallel_rspec", "rspec"].each do |binstub| - content = File.read("#{upstream}/bundler/bin/#{binstub}").gsub("../spec", "../bundler") - File.write("spec/bin/#{binstub}", content) - chmod("+x", "spec/bin/#{binstub}") - end - - %w[dev_gems test_gems rubocop_gems standard_gems].each do |gemfile| - ["rb.lock", "rb"].each do |ext| - cp_r("#{upstream}/tool/bundler/#{gemfile}.#{ext}", "tool/bundler") + config = REPOSITORIES[gem] + puts "Sync #{config.upstream}" + + upstream = File.join("..", "..", config.upstream) + + config.mappings.each do |src, dst| + rm_rf(dst) + end + + copied = Set.new + config.mappings.each do |src, dst| + prefix = File.join(upstream, src) + # Maybe mapping needs to be updated? + next unless File.exist?(prefix) + Find.find(prefix) do |path| + next if File.directory?(path) + if copied.add?(path) + newpath = config.rewrite_for_ruby(path.sub(%r{\A#{Regexp.escape(upstream)}/}, "")) + next unless newpath + mkdir_p(File.dirname(newpath)) + cp(path, newpath) end end - rm_rf Dir.glob("spec/bundler/support/artifice/{vcr_cassettes,used_cassettes.txt}") - rm_rf Dir.glob("lib/{bundler,rubygems}/**/{COPYING,LICENSE,README}{,.{md,txt,rdoc}}") - when "json" - rm_rf(%w[ext/json lib/json test/json]) - cp_r("#{upstream}/ext/json/ext", "ext/json") - cp_r("#{upstream}/test/json", "test/json") - rm_rf("test/json/lib") - cp_r("#{upstream}/lib", "ext/json") - cp_r("#{upstream}/json.gemspec", "ext/json") - rm_rf(%w[ext/json/lib/json/pure.rb ext/json/lib/json/pure ext/json/lib/json/truffle_ruby/]) - json_files = Dir.glob("ext/json/lib/json/ext/**/*", File::FNM_DOTMATCH).select { |f| File.file?(f) } - rm_rf(json_files - Dir.glob("ext/json/lib/json/ext/**/*.rb") - Dir.glob("ext/json/lib/json/ext/**/depend")) - `git checkout ext/json/extconf.rb ext/json/generator/depend ext/json/parser/depend ext/json/depend benchmark/` - when "psych" - rm_rf(%w[ext/psych test/psych]) - cp_r("#{upstream}/ext/psych", "ext") - cp_r("#{upstream}/lib", "ext/psych") - cp_r("#{upstream}/test/psych", "test") - rm_rf(%w[ext/psych/lib/org ext/psych/lib/psych.jar ext/psych/lib/psych_jars.rb]) - rm_rf(%w[ext/psych/lib/psych.{bundle,so} ext/psych/lib/2.*]) - rm_rf(["ext/psych/yaml/LICENSE"]) - cp_r("#{upstream}/psych.gemspec", "ext/psych") - `git checkout ext/psych/depend ext/psych/.gitignore` - when "stringio" - rm_rf(%w[ext/stringio test/stringio]) - cp_r("#{upstream}/ext/stringio", "ext") - cp_r("#{upstream}/test/stringio", "test") - cp_r("#{upstream}/stringio.gemspec", "ext/stringio") - `git checkout ext/stringio/depend ext/stringio/README.md` - when "io-console" - rm_rf(%w[ext/io/console test/io/console]) - cp_r("#{upstream}/ext/io/console", "ext/io") - cp_r("#{upstream}/test/io/console", "test/io") - mkdir_p("ext/io/console/lib") - cp_r("#{upstream}/lib/io/console", "ext/io/console/lib") - rm_rf("ext/io/console/lib/console/ffi") - cp_r("#{upstream}/io-console.gemspec", "ext/io/console") - `git checkout ext/io/console/depend` - when "io-nonblock" - rm_rf(%w[ext/io/nonblock test/io/nonblock]) - cp_r("#{upstream}/ext/io/nonblock", "ext/io") - cp_r("#{upstream}/test/io/nonblock", "test/io") - cp_r("#{upstream}/io-nonblock.gemspec", "ext/io/nonblock") - `git checkout ext/io/nonblock/depend` - when "io-wait" - rm_rf(%w[ext/io/wait test/io/wait]) - cp_r("#{upstream}/ext/io/wait", "ext/io") - cp_r("#{upstream}/test/io/wait", "test/io") - cp_r("#{upstream}/io-wait.gemspec", "ext/io/wait") - `git checkout ext/io/wait/depend` - when "etc" - rm_rf(%w[ext/etc test/etc]) - cp_r("#{upstream}/ext/etc", "ext") - cp_r("#{upstream}/test/etc", "test") - cp_r("#{upstream}/etc.gemspec", "ext/etc") - `git checkout ext/etc/depend` - when "date" - rm_rf(%w[ext/date test/date]) - cp_r("#{upstream}/doc/date", "doc") - cp_r("#{upstream}/ext/date", "ext") - cp_r("#{upstream}/lib", "ext/date") - cp_r("#{upstream}/test/date", "test") - cp_r("#{upstream}/date.gemspec", "ext/date") - `git checkout ext/date/depend` - rm_rf(["ext/date/lib/date_core.bundle"]) - when "zlib" - rm_rf(%w[ext/zlib test/zlib]) - cp_r("#{upstream}/ext/zlib", "ext") - cp_r("#{upstream}/test/zlib", "test") - cp_r("#{upstream}/zlib.gemspec", "ext/zlib") - `git checkout ext/zlib/depend` - when "fcntl" - rm_rf(%w[ext/fcntl]) - cp_r("#{upstream}/ext/fcntl", "ext") - cp_r("#{upstream}/fcntl.gemspec", "ext/fcntl") - `git checkout ext/fcntl/depend` - when "strscan" - rm_rf(%w[ext/strscan test/strscan]) - cp_r("#{upstream}/ext/strscan", "ext") - cp_r("#{upstream}/lib", "ext/strscan") - cp_r("#{upstream}/test/strscan", "test") - cp_r("#{upstream}/strscan.gemspec", "ext/strscan") - begin - cp_r("#{upstream}/doc/strscan", "doc") - rescue Errno::ENOENT + end + + porcelain_status().each do |line| + /\A(?.)(?.) (?.*)\z/ =~ line or raise + if config.excluded?(path) + puts "Restoring excluded file: #{path}" + IO.popen(%W"git checkout --" + [path], "rb", &:read) end - rm_rf(%w["ext/strscan/regenc.h ext/strscan/regint.h"]) - `git checkout ext/strscan/depend` - when "cgi" - rm_rf(%w[lib/cgi.rb lib/cgi ext/cgi test/cgi]) - cp_r("#{upstream}/ext/cgi", "ext") - mkdir_p("lib/cgi") - cp_r("#{upstream}/lib/cgi/escape.rb", "lib/cgi") - mkdir_p("test/cgi") - cp_r("#{upstream}/test/cgi/test_cgi_escape.rb", "test/cgi") - cp_r("#{upstream}/test/cgi/update_env.rb", "test/cgi") - rm_rf("lib/cgi/escape.jar") - `git checkout lib/cgi.rb lib/cgi/util.rb ext/cgi/escape/depend` - when "openssl" - rm_rf(%w[ext/openssl test/openssl]) - cp_r("#{upstream}/ext/openssl", "ext") - cp_r("#{upstream}/lib", "ext/openssl") - cp_r("#{upstream}/test/openssl", "test") - rm_rf("test/openssl/envutil.rb") - cp_r("#{upstream}/openssl.gemspec", "ext/openssl") - cp_r("#{upstream}/History.md", "ext/openssl") - `git checkout ext/openssl/depend` - when "net-protocol" - rm_rf(%w[lib/net/protocol.rb lib/net/net-protocol.gemspec test/net/protocol]) - cp_r("#{upstream}/lib/net/protocol.rb", "lib/net") - cp_r("#{upstream}/test/net/protocol", "test/net") - cp_r("#{upstream}/net-protocol.gemspec", "lib/net") - when "net-http" - rm_rf(%w[lib/net/http.rb lib/net/http test/net/http]) - cp_r("#{upstream}/lib/net/http.rb", "lib/net") - cp_r("#{upstream}/lib/net/http", "lib/net") - cp_r("#{upstream}/test/net/http", "test/net") - cp_r("#{upstream}/net-http.gemspec", "lib/net/http") - when "did_you_mean" - rm_rf(%w[lib/did_you_mean lib/did_you_mean.rb test/did_you_mean]) - cp_r(Dir.glob("#{upstream}/lib/did_you_mean*"), "lib") - cp_r("#{upstream}/did_you_mean.gemspec", "lib/did_you_mean") - cp_r("#{upstream}/test", "test/did_you_mean") - rm_rf("test/did_you_mean/lib") - rm_rf(%w[test/did_you_mean/tree_spell/test_explore.rb]) - when "erb" - rm_rf(%w[lib/erb* test/erb libexec/erb]) - cp_r("#{upstream}/lib/erb.rb", "lib") - cp_r("#{upstream}/test/erb", "test") - cp_r("#{upstream}/erb.gemspec", "lib/erb") - cp_r("#{upstream}/libexec/erb", "libexec") - when "digest" - rm_rf(%w[ext/digest test/digest]) - cp_r("#{upstream}/ext/digest", "ext") - mkdir_p("ext/digest/lib/digest") - cp_r("#{upstream}/lib/digest.rb", "ext/digest/lib/") - cp_r("#{upstream}/lib/digest/version.rb", "ext/digest/lib/digest/") - mkdir_p("ext/digest/sha2/lib") - cp_r("#{upstream}/lib/digest/sha2.rb", "ext/digest/sha2/lib") - move("ext/digest/lib/digest/sha2", "ext/digest/sha2/lib") - cp_r("#{upstream}/test/digest", "test") - cp_r("#{upstream}/digest.gemspec", "ext/digest") - `git checkout ext/digest/depend ext/digest/*/depend` - when "optparse" - sync_lib gem, upstream - rm_rf(%w[doc/optparse]) - mkdir_p("doc/optparse") - cp_r("#{upstream}/doc/optparse", "doc") - when "error_highlight" - rm_rf(%w[lib/error_highlight lib/error_highlight.rb test/error_highlight]) - cp_r(Dir.glob("#{upstream}/lib/error_highlight*"), "lib") - cp_r("#{upstream}/error_highlight.gemspec", "lib/error_highlight") - cp_r("#{upstream}/test", "test/error_highlight") - when "open3" - sync_lib gem, upstream - rm_rf("lib/open3/jruby_windows.rb") - when "syntax_suggest" - sync_lib gem, upstream - rm_rf(%w[spec/syntax_suggest libexec/syntax_suggest]) - cp_r("#{upstream}/spec", "spec/syntax_suggest") - cp_r("#{upstream}/exe/syntax_suggest", "libexec/syntax_suggest") - when "prism" - rm_rf(%w[test/prism prism]) - - cp_r("#{upstream}/ext/prism", "prism") - cp_r("#{upstream}/lib/.", "lib") - cp_r("#{upstream}/test/prism", "test") - cp_r("#{upstream}/src/.", "prism") - - cp_r("#{upstream}/prism.gemspec", "lib/prism") - cp_r("#{upstream}/include/prism/.", "prism") - cp_r("#{upstream}/include/prism.h", "prism") - - cp_r("#{upstream}/config.yml", "prism/") - cp_r("#{upstream}/templates", "prism/") - rm_rf("prism/templates/javascript") - rm_rf("prism/templates/java") - rm_rf("prism/templates/rbi") - rm_rf("prism/templates/sig") - - rm("test/prism/snapshots_test.rb") - rm_rf("test/prism/snapshots") - - rm("prism/extconf.rb") - `git checkout prism/srcs.mk*` - when "resolv" - rm_rf(%w[lib/resolv.* ext/win32/resolv test/resolv ext/win32/lib/win32/resolv.rb]) - cp_r("#{upstream}/lib/resolv.rb", "lib") - cp_r("#{upstream}/resolv.gemspec", "lib") - cp_r("#{upstream}/ext/win32/resolv", "ext/win32") - move("ext/win32/resolv/lib/resolv.rb", "ext/win32/lib/win32") - rm_rf("ext/win32/resolv/lib") # Clean up empty directory - cp_r("#{upstream}/test/resolv", "test") - `git checkout ext/win32/resolv/depend` - when "win32-registry" - rm_rf(%w[ext/win32/lib/win32/registry.rb test/win32/test_registry.rb]) - cp_r("#{upstream}/lib/win32/registry.rb", "ext/win32/lib/win32") - cp_r("#{upstream}/test/win32/test_registry.rb", "test/win32") - cp_r("#{upstream}/win32-registry.gemspec", "ext/win32") - when "mmtk" - rm_rf("gc/mmtk") - cp_r("#{upstream}/gc/mmtk", "gc") - else - sync_lib gem, upstream + end + + # RubyGems/Bundler needs special care + if gem == "rubygems" + rubygems_do_fixup end check_prerelease_version(gem) @@ -370,13 +403,11 @@ def sync_default_gems(gem) def check_prerelease_version(gem) return if ["rubygems", "mmtk", "cgi"].include?(gem) - gem = gem.downcase - require "net/https" require "json" require "uri" - uri = URI("https://rubygems.org/api/v1/versions/#{gem}/latest.json") + uri = URI("https://rubygems.org/api/v1/versions/#{gem.downcase}/latest.json") response = Net::HTTP.get(uri) latest_version = JSON.parse(response)["version"] @@ -393,42 +424,19 @@ def check_prerelease_version(gem) puts "#{gem}-#{spec.version} is not latest version of rubygems.org" if spec.version.to_s != latest_version end - def ignore_file_pattern_for(gem) - patterns = [] - - # Common patterns - patterns << %r[\A(?: - [^/]+ # top-level entries - |\.git.* - |bin/.* - |ext/.*\.java - |rakelib/.* - |test/(?:lib|fixtures)/.* - |tool/(?!bundler/).* - )\z]mx - - # Gem-specific patterns - case gem - when nil - end&.tap do |pattern| - patterns << pattern - end - - Regexp.union(*patterns) - end - - def message_filter(repo, sha, input: ARGF) + def message_filter(repo, sha, log, context: nil) unless repo.count("/") == 1 and /\A\S+\z/ =~ repo raise ArgumentError, "invalid repository: #{repo}" end unless /\A\h{10,40}\z/ =~ sha raise ArgumentError, "invalid commit-hash: #{sha}" end - log = input.read - log.delete!("\r") - log << "\n" if !log.end_with?("\n") repo_url = "https://github.com/#{repo}" + # Log messages generated by GitHub web UI have inconsistent line endings + log = log.delete("\r") + log << "\n" if !log.end_with?("\n") + # Split the subject from the log message according to git conventions. # SPECIAL TREAT: when the first line ends with a dot `.` (which is not # obeying the conventions too), takes only that line. @@ -450,16 +458,17 @@ def message_filter(repo, sha, input: ARGF) end end commit_url = "#{repo_url}/commit/#{sha[0,10]}\n" + sync_note = context ? "#{commit_url}\n#{context}" : commit_url if log and !log.empty? log.sub!(/(?<=\n)\n+\z/, '') # drop empty lines at the last conv[log] log.sub!(/(?:(\A\s*)|\s*\n)(?=((?i:^Co-authored-by:.*\n?)+)?\Z)/) { - ($~.begin(1) ? "" : "\n\n") + commit_url + ($~.begin(2) ? "\n" : "") + ($~.begin(1) ? "" : "\n\n") + sync_note + ($~.begin(2) ? "\n" : "") } else - log = commit_url + log = sync_note end - puts subject, "\n", log + "#{subject}\n\n#{log}" end def log_format(format, args, &block) @@ -467,27 +476,51 @@ def log_format(format, args, &block) log --no-show-signature --format=#{format}] + args, "rb", &block) end - # Returns commit list as array of [commit_hash, subject]. - def commits_in_ranges(gem, repo, default_branch, ranges) - # If -a is given, discover all commits since the last picked commit - if ranges == true - pattern = "https://github\.com/#{Regexp.quote(repo)}/commit/([0-9a-f]+)$" - log = log_format('%B', %W"-E --grep=#{pattern} -n1 --", &:read) - ranges = ["#{log[%r[#{pattern}\n\s*(?i:co-authored-by:.*)*\s*\Z], 1]}..#{gem}/#{default_branch}"] - end + def commits_in_range(upto, exclude, toplevel:) + args = [upto, *exclude.map {|s|"^#{s}"}] + log_format('%H,%P,%s', %W"--first-parent" + args) do |f| + f.read.split("\n").reverse.flat_map {|commit| + hash, parents, subject = commit.split(',', 3) + parents = parents.split + + # Non-merge commit + if parents.size <= 1 + puts "#{hash} #{subject}" + next [[hash, subject]] + end - # Parse a given range with git log - ranges.flat_map do |range| - unless range.include?("..") - range = "#{range}~1..#{range}" - end + # Clean 2-parent merge commit: follow the other parent as long as it + # contains no potentially-non-clean merges + if parents.size == 2 && + IO.popen(%W"git diff-tree --remerge-diff #{hash}", "rb", &:read).empty? + puts "\e[2mChecking the other parent of #{hash} #{subject}\e[0m" + ret = catch(:quit) { + commits_in_range(parents[1], exclude + [parents[0]], toplevel: false) + } + next ret if ret + end - log_format('%H,%s', %W"#{range} --") do |f| - f.read.split("\n").reverse.map{|commit| commit.split(',', 2)} - end + unless toplevel + puts "\e[1mMerge commit with possible conflict resolution #{hash} #{subject}\e[0m" + throw :quit + end + + puts "#{hash} #{subject} " \ + "\e[1m[merge commit with possible conflicts, will do a squash merge]\e[0m" + [[hash, subject]] + } end end + # Returns commit list as array of [commit_hash, subject, sync_note]. + def commits_in_ranges(ranges) + ranges.flat_map do |range| + exclude, upto = range.include?("..") ? range.split("..", 2) : ["#{range}~1", range] + puts "Looking for commits in range #{exclude}..#{upto}" + commits_in_range(upto, exclude.empty? ? [] : [exclude], toplevel: true) + end.uniq + end + #-- # Following methods used by sync_default_gems_with_commits return # true: success @@ -496,28 +529,9 @@ def commits_in_ranges(gem, repo, default_branch, ranges) #++ def resolve_conflicts(gem, sha, edit) - # Skip this commit if everything has been removed as `ignored_paths`. + # Discover unmerged files: any unstaged changes changes = porcelain_status() - if changes.empty? - puts "Skip empty commit #{sha}" - return false - end - - # We want to skip - # DD: deleted by both - # DU: deleted by us - deleted = changes.grep(/^D[DU] /) {$'} - system(*%W"git rm -f --", *deleted) unless deleted.empty? - - # Import UA: added by them - added = changes.grep(/^UA /) {$'} - system(*%W"git add --", *added) unless added.empty? - - # Discover unmerged files - # AU: unmerged, added by us - # UU: unmerged, both modified - # AA: unmerged, both added - conflict = changes.grep(/\A(?:A[AU]|UU) /) {$'} + conflict = changes.grep(/\A(?:.[^ ?]) /) {$'} # If -e option is given, open each conflicted file with an editor unless conflict.empty? if edit @@ -538,127 +552,173 @@ def resolve_conflicts(gem, sha, edit) return true end - def preexisting?(base, file) - system(*%w"git cat-file -e", "#{base}:#{file}", err: File::NULL) + def collect_cacheinfo(tree) + cacheinfo = pipe_readlines(%W"git ls-tree -r -t -z #{tree}").filter_map do |line| + fields, path = line.split("\t", 2) + mode, type, object = fields.split(" ", 3) + next unless type == "blob" + [mode, type, object, path] + end end - def filter_pickup_files(changed, ignore_file_pattern, base) - toplevels = {} - remove = [] - ignore = [] - changed = changed.reject do |f| - case - when toplevels.fetch(top = f[%r[\A[^/]+(?=/|\z)]m]) { - remove << top if toplevels[top] = !preexisting?(base, top) - } - # Remove any new top-level directories. - true - when ignore_file_pattern.match?(f) - # Forcibly reset any changes matching ignore_file_pattern. - (preexisting?(base, f) ? ignore : remove) << f - end + def rewrite_cacheinfo(gem, blobs) + config = REPOSITORIES[gem] + rewritten = [] + ignored = blobs.dup + ignored.delete_if do |mode, type, object, path| + newpath = config.rewrite_for_ruby(path) + next unless newpath + rewritten << [mode, type, object, newpath] end - return changed, remove, ignore + [rewritten, ignored] end - def pickup_files(gem, changed, picked) - # Forcibly remove any files that we don't want to copy to this - # repository. - - ignore_file_pattern = ignore_file_pattern_for(gem) + def make_commit_info(gem, sha) + config = REPOSITORIES[gem] + headers, orig = IO.popen(%W[git cat-file commit #{sha}], "rb", &:read).split("\n\n", 2) + /^author (?.+?) <(?.*?)> (?.+?)$/ =~ headers or + raise "unable to parse author info for commit #{sha}" + author = { + "GIT_AUTHOR_NAME" => author_name, + "GIT_AUTHOR_EMAIL" => author_email, + "GIT_AUTHOR_DATE" => author_date, + } + context = nil + if /^parent (?.{40})\nparent .{40}$/ =~ headers + # Squashing a merge commit: keep authorship information + context = IO.popen(%W"git shortlog #{first_parent}..#{sha} --", "rb", &:read) + end + message = message_filter(config.upstream, sha, orig, context: context) + [author, message] + end - base = picked ? "HEAD~" : "HEAD" - changed, remove, ignore = filter_pickup_files(changed, ignore_file_pattern, base) + def fixup_commit(gem, commit) + wt = File.join("tmp", "sync_default_gems-fixup-worktree") + if File.directory?(wt) + IO.popen(%W"git -C #{wt} clean -xdf", "rb", &:read) + IO.popen(%W"git -C #{wt} reset --hard #{commit}", "rb", &:read) + else + IO.popen(%W"git worktree remove --force #{wt}", "rb", err: File::NULL, &:read) + IO.popen(%W"git worktree add --detach #{wt} #{commit}", "rb", &:read) + end + raise "git worktree prepare failed for commit #{commit}" unless $?.success? - unless remove.empty? - puts "Remove added files: #{remove.join(', ')}" - system(*%w"git rm -fr --", *remove) - if picked - system(*%w"git commit --amend --no-edit --", *remove, %i[out err] => File::NULL) + Dir.chdir(wt) do + if gem == "rubygems" + rubygems_do_fixup end + replace_rdoc_ref_all_full end - unless ignore.empty? - puts "Reset ignored files: #{ignore.join(', ')}" - system(*%W"git rm -r --", *ignore) - ignore.each {|f| system(*%W"git checkout -f", base, "--", f)} - end + IO.popen(%W"git -C #{wt} add -u", "rb", &:read) + IO.popen(%W"git -C #{wt} commit --amend --no-edit", "rb", &:read) + IO.popen(%W"git -C #{wt} rev-parse HEAD", "rb", &:read).chomp + end + + def make_and_fixup_commit(gem, original_commit, cacheinfo, parent: nil, message: nil, author: nil) + tree = Tempfile.create("sync_default_gems-#{gem}-index") do |f| + File.unlink(f.path) + IO.popen({"GIT_INDEX_FILE" => f.path}, + %W"git update-index --index-info", "wb", out: IO::NULL) do |io| + cacheinfo.each do |mode, type, object, path| + io.puts("#{mode} #{type} #{object}\t#{path}") + end + end + raise "git update-index failed" unless $?.success? - if changed.empty? - return nil + IO.popen({"GIT_INDEX_FILE" => f.path}, %W"git write-tree --missing-ok", "rb", &:read).chomp end - return changed + args = ["-m", message || "Rewriten commit for #{original_commit}"] + args += ["-p", parent] if parent + commit = IO.popen({**author}, %W"git commit-tree #{tree}" + args, "rb", &:read).chomp + + # Apply changes that require a working tree + commit = fixup_commit(gem, commit) + + commit end - def pickup_commit(gem, sha, edit) - # Attempt to cherry-pick a commit - result = IO.popen(%W"git cherry-pick #{sha}", "rb", &:read) - picked = $?.success? - if result =~ /nothing\ to\ commit/ - `git reset` - puts "Skip empty commit #{sha}" + def rewrite_commit(gem, sha) + config = REPOSITORIES[gem] + author, message = make_commit_info(gem, sha) + new_blobs = collect_cacheinfo("#{sha}") + new_rewritten, new_ignored = rewrite_cacheinfo(gem, new_blobs) + + headers, orig_message = IO.popen(%W[git cat-file commit #{sha}], "rb", &:read).split("\n\n", 2) + first_parent = headers[/^parent (.{40})$/, 1] + unless first_parent + # Root commit, first time to sync this repo + return make_and_fixup_commit(gem, sha, new_rewritten, message: message, author: author) + end + + old_blobs = collect_cacheinfo(first_parent) + old_rewritten, old_ignored = rewrite_cacheinfo(gem, old_blobs) + if old_ignored != new_ignored + paths = (old_ignored + new_ignored - (old_ignored & new_ignored)) + .map {|*_, path| path}.uniq + puts "\e\[1mIgnoring file changes not in mappings: #{paths.join(" ")}\e\[0m" + end + changed_paths = (old_rewritten + new_rewritten - (old_rewritten & new_rewritten)) + .map {|*_, path| path}.uniq + if changed_paths.empty? + puts "Skip commit only for tools or toplevel" return false end - # Skip empty commits - if result.empty? - return false - end + # Build commit objects from "cacheinfo" + new_parent = make_and_fixup_commit(gem, first_parent, old_rewritten) + new_commit = make_and_fixup_commit(gem, sha, new_rewritten, parent: new_parent, message: message, author: author) + puts "Created a temporary commit for cherry-pick: #{new_commit}" + new_commit + end - if picked - changed = pipe_readlines(%w"git diff-tree --name-only -r -z HEAD~..HEAD --") - else - changed = pipe_readlines(%w"git diff --name-only -r -z HEAD --") - end + def pickup_commit(gem, sha, edit) + config = REPOSITORIES[gem] - # Pick up files to merge. - unless changed = pickup_files(gem, changed, picked) - puts "Skip commit #{sha} only for tools or toplevel" - if picked - `git reset --hard HEAD~` - else - `git cherry-pick --abort` - end - return false - end + rewritten = rewrite_commit(gem, sha) - # If the cherry-pick attempt failed, try to resolve conflicts. - # Skip the commit, if it contains unresolved conflicts or no files to pick up. - unless picked or resolve_conflicts(gem, sha, edit) - system(*%w"git --no-pager diff") if !picked && !edit # If failed, show `git diff` unless editing - `git reset` && `git checkout .` && `git clean -fd` # Clean up un-committed diffs - return picked || nil # Fail unless cherry-picked - end + # No changes remaining after rewriting + return false unless rewritten - # Commit cherry-picked commit - if picked - system(*%w"git commit --amend --no-edit") - elsif porcelain_status().empty? - system(*%w"git cherry-pick --skip") - return false - else - system(*%w"git cherry-pick --continue --no-edit") - end or return nil + # Attempt to cherry-pick a commit + result = IO.popen(%W"git cherry-pick #{rewritten}", "rb", err: [:child, :out], &:read) + unless $?.success? + if result =~ /The previous cherry-pick is now empty/ + system(*%w"git cherry-pick --skip") + puts "Skip empty commit #{sha}" + return false + end + + # If the cherry-pick attempt failed, try to resolve conflicts. + # Skip the commit, if it contains unresolved conflicts or no files to pick up. + unless resolve_conflicts(gem, sha, edit) + system(*%w"git --no-pager diff") if !edit # If failed, show `git diff` unless editing + `git reset` && `git checkout .` && `git clean -fd` # Clean up un-committed diffs + return nil # Fail unless cherry-picked + end - # Amend the commit if RDoc references need to be replaced - head = log_format('%H', %W"-1 HEAD", &:read).chomp - system(*%w"git reset --quiet HEAD~ --") - amend = replace_rdoc_ref_all - system(*%W"git reset --quiet #{head} --") - if amend - `git commit --amend --no-edit --all` + # Commit cherry-picked commit + if porcelain_status().empty? + system(*%w"git cherry-pick --skip") + return false + else + system(*%w"git cherry-pick --continue --no-edit") + return nil unless $?.success? + end end + new_head = IO.popen(%W"git rev-parse HEAD", "rb", &:read).chomp + puts "Committed cherry-pick as #{new_head}" return true end - # NOTE: This method is also used by GitHub ruby/git.ruby-lang.org's bin/update-default-gem.sh # @param gem [String] A gem name, also used as a git remote name. REPOSITORIES converts it to the appropriate GitHub repository. - # @param ranges [Array] "before..after". Note that it will NOT sync "before" (but commits after that). + # @param ranges [Array, true] "commit", "before..after", or true. Note that it will NOT sync "before" (but commits after that). # @param edit [TrueClass] Set true if you want to resolve conflicts. Obviously, update-default-gem.sh doesn't use this. def sync_default_gems_with_commits(gem, ranges, edit: nil) - repo, default_branch = REPOSITORIES[gem] + config = REPOSITORIES[gem] + repo, default_branch = config.upstream, config.branch puts "Sync #{repo} with commit history." # Fetch the repository to be synchronized @@ -669,55 +729,35 @@ def sync_default_gems_with_commits(gem, ranges, edit: nil) end system(*%W"git fetch --no-tags --depth=#{FETCH_DEPTH} #{gem} #{default_branch}") - commits = commits_in_ranges(gem, repo, default_branch, ranges) - - # Ignore Merge commits and already-merged commits. - commits.delete_if do |sha, subject| - subject.start_with?("Merge", "Auto Merge") + # If -a is given, discover all commits since the last picked commit + if ranges == true + pattern = "https://github\.com/#{Regexp.quote(repo)}/commit/([0-9a-f]+)$" + log = log_format('%B', %W"-E --grep=#{pattern} -n1 --", &:read) + ranges = ["#{log[%r[#{pattern}\n\s*(?i:co-authored-by:.*)*\s*\Z], 1]}..#{gem}/#{default_branch}"] end - + commits = commits_in_ranges(ranges) if commits.empty? puts "No commits to pick" return true end - puts "Try to pick these commits:" - puts commits.map{|commit| commit.join(": ")} - puts "----" - failed_commits = [] - - require 'shellwords' - filter = [ - ENV.fetch('RUBY', 'ruby').shellescape, - File.realpath(__FILE__).shellescape, - "--message-filter", - ] commits.each do |sha, subject| - puts "Pick #{sha} from #{repo}." + puts "----" + puts "Pick #{sha} #{subject}" case pickup_commit(gem, sha, edit) when false - next + # skipped when nil - failed_commits << sha - next - end - - puts "Update commit message: #{sha}" - - # Run this script itself (tool/sync_default_gems.rb --message-filter) as a message filter - IO.popen({"FILTER_BRANCH_SQUELCH_WARNING" => "1"}, - %W[git filter-branch -f --msg-filter #{[filter, repo, sha].join(' ')} -- HEAD~1..HEAD], - &:read) - unless $?.success? - puts "Failed to modify commit message of #{sha}" - break + failed_commits << [sha, subject] end end unless failed_commits.empty? puts "---- failed commits ----" - puts failed_commits + failed_commits.each do |sha, subject| + puts "#{sha} #{subject}" + end return false end return true @@ -744,9 +784,9 @@ def sync_lib(repo, upstream = nil) end def update_default_gems(gem, release: false) - - repository, default_branch = REPOSITORIES[gem] - author, repository = repository.split('/') + config = REPOSITORIES[gem] + author, repository = config.upstream.split('/') + default_branch = config.branch puts "Update #{author}/#{repository}" @@ -797,17 +837,10 @@ def update_default_gems(gem, release: false) when "list" ARGV.shift pattern = Regexp.new(ARGV.join('|')) - REPOSITORIES.each_pair do |name, (gem)| - next unless pattern =~ name or pattern =~ gem - printf "%-15s https://github.com/%s\n", name, gem + REPOSITORIES.each do |gem, config| + next unless pattern =~ gem or pattern =~ config.upstream + printf "%-15s https://github.com/%s\n", gem, config.upstream end - when "--message-filter" - ARGV.shift - if ARGV.size < 2 - abort "usage: #{$0} --message-filter repository commit-hash [input...]" - end - message_filter(*ARGV.shift(2)) - exit when "rdoc-ref" ARGV.shift pattern = ARGV.empty? ? %w[*.c *.rb *.rdoc] : ARGV diff --git a/tool/test/test_sync_default_gems.rb b/tool/test/test_sync_default_gems.rb index adbde66fbff132..f50be036fe16fd 100755 --- a/tool/test/test_sync_default_gems.rb +++ b/tool/test/test_sync_default_gems.rb @@ -19,14 +19,8 @@ def assert_message_filter(expected, trailers, input, repo = "ruby/test", sha = " expected.concat(trailers.map {_1+"\n"}) end - out, err = capture_output do - SyncDefaultGems.message_filter(repo, sha, input: StringIO.new(input, "r")) - end - - all_assertions do |a| - a.for("error") {assert_empty err} - a.for("result") {assert_pattern_list(expected, out)} - end + out = SyncDefaultGems.message_filter(repo, sha, input) + assert_pattern_list(expected, out) end def test_subject_only @@ -115,7 +109,16 @@ def setup git(*%W"config --global log.showSignature true") end @target = "sync-test" - SyncDefaultGems::REPOSITORIES[@target] = ["ruby/#{@target}", "default"] + SyncDefaultGems::REPOSITORIES[@target] = SyncDefaultGems.repo( + ["ruby/#{@target}", "default"], + [ + ["lib", "lib"], + ["test", "test"], + ], + exclude: [ + "test/fixtures/*", + ], + ) @sha = {} @origdir = Dir.pwd Dir.chdir(@testdir) @@ -314,5 +317,34 @@ def test_delete_after_conflict assert_equal(":ok\n""Should.be_merged\n", File.read("src/lib/common.rb"), out) assert_not_operator(File, :exist?, "src/lib/bad.rb", out) end + + def test_squash_merge + # 2---. <- branch + # / \ + # 1---3---3'<- merge commit with conflict resolution + File.write("#@target/lib/conflict.rb", "# 1\n") + git(*%W"add lib/conflict.rb", chdir: @target) + git(*%W"commit -q -m", "Add conflict.rb", chdir: @target) + + git(*%W"checkout -q -b branch", chdir: @target) + File.write("#@target/lib/conflict.rb", "# 2\n") + File.write("#@target/lib/new.rb", "# new\n") + git(*%W"add lib/conflict.rb lib/new.rb", chdir: @target) + git(*%W"commit -q -m", "Commit in branch", chdir: @target) + + git(*%W"checkout -q default", chdir: @target) + File.write("#@target/lib/conflict.rb", "# 3\n") + git(*%W"add lib/conflict.rb", chdir: @target) + git(*%W"commit -q -m", "Commit in default", chdir: @target) + + # How can I suppress "Auto-merging ..." message from git merge? + git(*%W"merge -X ours -m", "Merge commit", "branch", chdir: @target, out: IO::NULL) + + out = assert_sync() + assert_equal("# 3\n", File.read("src/lib/conflict.rb"), out) + subject, body = top_commit("src", format: "%B").split("\n\n", 2) + assert_equal("[ruby/#@target] Merge commit", subject, out) + assert_includes(body, "Commit in branch", out) + end end if /darwin|linux/ =~ RUBY_PLATFORM end diff --git a/variable.c b/variable.c index bab423e95029f7..f6cfef7b0725e6 100644 --- a/variable.c +++ b/variable.c @@ -1010,18 +1010,21 @@ rb_gvar_set(ID id, VALUE val) VALUE retval; struct rb_global_entry *entry; const rb_namespace_t *ns = rb_current_namespace(); + bool use_namespace_tbl = false; RB_VM_LOCKING() { entry = rb_global_entry(id); if (USE_NAMESPACE_GVAR_TBL(ns, entry)) { + use_namespace_tbl = true; rb_hash_aset(ns->gvar_tbl, rb_id2sym(entry->id), val); retval = val; // TODO: think about trace } - else { - retval = rb_gvar_set_entry(entry, val); - } + } + + if (!use_namespace_tbl) { + retval = rb_gvar_set_entry(entry, val); } return retval; } @@ -1037,28 +1040,36 @@ rb_gvar_get(ID id) { VALUE retval, gvars, key; const rb_namespace_t *ns = rb_current_namespace(); + bool use_namespace_tbl = false; + struct rb_global_entry *entry; + struct rb_global_variable *var; // TODO: use lock-free rb_id_table when it's available for use (doesn't yet exist) RB_VM_LOCKING() { - struct rb_global_entry *entry = rb_global_entry(id); - struct rb_global_variable *var = entry->var; + entry = rb_global_entry(id); + var = entry->var; if (USE_NAMESPACE_GVAR_TBL(ns, entry)) { + use_namespace_tbl = true; gvars = ns->gvar_tbl; key = rb_id2sym(entry->id); if (RTEST(rb_hash_has_key(gvars, key))) { // this gvar is already cached retval = rb_hash_aref(gvars, key); } else { - retval = (*var->getter)(entry->id, var->data); - if (rb_obj_respond_to(retval, rb_intern("clone"), 1)) { - retval = rb_funcall(retval, rb_intern("clone"), 0); + RB_VM_UNLOCK(); + { + retval = (*var->getter)(entry->id, var->data); + if (rb_obj_respond_to(retval, rb_intern("clone"), 1)) { + retval = rb_funcall(retval, rb_intern("clone"), 0); + } } + RB_VM_LOCK(); rb_hash_aset(gvars, key, retval); } } - else { - retval = (*var->getter)(entry->id, var->data); - } + } + if (!use_namespace_tbl) { + retval = (*var->getter)(entry->id, var->data); } return retval; } @@ -1159,6 +1170,7 @@ rb_alias_variable(ID name1, ID name2) else if ((entry1 = (struct rb_global_entry *)data1)->var != entry2->var) { struct rb_global_variable *var = entry1->var; if (var->block_trace) { + RB_VM_UNLOCK(); rb_raise(rb_eRuntimeError, "can't alias in tracer"); } var->counter--; diff --git a/zjit.rb b/zjit.rb index 72a6e586513b56..cfe8bcd6e2cfb1 100644 --- a/zjit.rb +++ b/zjit.rb @@ -165,9 +165,9 @@ def stats_string print_counters_with_prefix(prefix: 'send_fallback_', prompt: 'send fallback reasons', buf:, stats:, limit: 20) # Show most popular unsupported call features. Because each call can - # use multiple fancy features, a decrease in this number does not + # use multiple complex features, a decrease in this number does not # necessarily mean an increase in number of optimized calls. - print_counters_with_prefix(prefix: 'fancy_arg_pass_', prompt: 'popular unsupported argument-parameter features', buf:, stats:, limit: 10) + print_counters_with_prefix(prefix: 'complex_arg_pass_', prompt: 'popular complex argument-parameter features not optimized', buf:, stats:, limit: 10) # Show exit counters, ordered by the typical amount of exits for the prefix at the time print_counters_with_prefix(prefix: 'unhandled_yarv_insn_', prompt: 'unhandled YARV insns', buf:, stats:, limit: 20) diff --git a/zjit/src/asm/arm64/inst/load_store.rs b/zjit/src/asm/arm64/inst/load_store.rs index e27909ae355d10..e78edd56752aae 100644 --- a/zjit/src/asm/arm64/inst/load_store.rs +++ b/zjit/src/asm/arm64/inst/load_store.rs @@ -124,6 +124,12 @@ impl LoadStore { pub fn sturh(rt: u8, rn: u8, imm9: i16) -> Self { Self { rt, rn, idx: Index::None, imm9, opc: Opc::STR, size: Size::Size16 } } + + /// STURB (store register, byte, unscaled) + /// + pub fn sturb(rt: u8, rn: u8, imm9: i16) -> Self { + Self { rt, rn, idx: Index::None, imm9, opc: Opc::STR, size: Size::Size8 } + } } /// diff --git a/zjit/src/asm/arm64/mod.rs b/zjit/src/asm/arm64/mod.rs index ffb83cf16abd03..a4459117312f89 100644 --- a/zjit/src/asm/arm64/mod.rs +++ b/zjit/src/asm/arm64/mod.rs @@ -1026,7 +1026,21 @@ pub fn sturh(cb: &mut CodeBlock, rt: A64Opnd, rn: A64Opnd) { LoadStore::sturh(rt.reg_no, rn.base_reg_no, rn.disp as i16).into() }, - _ => panic!("Invalid operand combination to stur instruction.") + _ => panic!("Invalid operand combination to sturh instruction: {rt:?}, {rn:?}") + }; + + cb.write_bytes(&bytes); +} + +pub fn sturb(cb: &mut CodeBlock, rt: A64Opnd, rn: A64Opnd) { + let bytes: [u8; 4] = match (rt, rn) { + (A64Opnd::Reg(rt), A64Opnd::Mem(rn)) => { + assert!(rn.num_bits == 8); + assert!(mem_disp_fits_bits(rn.disp), "Expected displacement {} to be 9 bits or less", rn.disp); + + LoadStore::sturb(rt.reg_no, rn.base_reg_no, rn.disp as i16).into() + }, + _ => panic!("Invalid operand combination to sturb instruction: {rt:?}, {rn:?}") }; cb.write_bytes(&bytes); diff --git a/zjit/src/backend/arm64/mod.rs b/zjit/src/backend/arm64/mod.rs index 532570d732341e..428d4bff779084 100644 --- a/zjit/src/backend/arm64/mod.rs +++ b/zjit/src/backend/arm64/mod.rs @@ -800,6 +800,7 @@ impl Assembler { asm.push_insn(Insn::RShift { out: SCRATCH0_OPND, opnd: reg_out, shift: Opnd::UImm(63) }); } } + Insn::LShift { opnd, out, .. } | Insn::RShift { opnd, out, .. } => { *opnd = split_memory_read(asm, *opnd, SCRATCH0_OPND); let mem_out = split_memory_write(out, SCRATCH0_OPND); @@ -1297,7 +1298,8 @@ impl Assembler { match dest.rm_num_bits() { 64 | 32 => stur(cb, src, dest.into()), 16 => sturh(cb, src, dest.into()), - num_bits => panic!("unexpected dest num_bits: {} (src: {:#?}, dest: {:#?})", num_bits, src, dest), + 8 => sturb(cb, src, dest.into()), + num_bits => panic!("unexpected dest num_bits: {} (src: {:?}, dest: {:?})", num_bits, src, dest), } }, Insn::Load { opnd, out } | @@ -1674,6 +1676,7 @@ mod tests { static TEMP_REGS: [Reg; 5] = [X1_REG, X9_REG, X10_REG, X14_REG, X15_REG]; fn setup_asm() -> (Assembler, CodeBlock) { + crate::options::rb_zjit_prepare_options(); // Allow `get_option!` in Assembler (Assembler::new(), CodeBlock::new_dummy()) } @@ -2599,9 +2602,22 @@ mod tests { assert_snapshot!(cb.hexdump(), @"200500b1010400b1"); } + #[test] + fn test_store_spilled_byte() { + let (mut asm, mut cb) = setup_asm(); + + asm.store(Opnd::mem(8, C_RET_OPND, 0), Opnd::mem(8, C_RET_OPND, 8)); + asm.compile_with_num_regs(&mut cb, 0); // spill every VReg + + assert_disasm_snapshot!(cb.disasm(), @r" + 0x0: ldurb w16, [x0, #8] + 0x4: sturb w16, [x0] + "); + assert_snapshot!(cb.hexdump(), @"1080403810000038"); + } + #[test] fn test_ccall_resolve_parallel_moves_no_cycle() { - crate::options::rb_zjit_prepare_options(); let (mut asm, mut cb) = setup_asm(); asm.ccall(0 as _, vec![ @@ -2619,7 +2635,6 @@ mod tests { #[test] fn test_ccall_resolve_parallel_moves_single_cycle() { - crate::options::rb_zjit_prepare_options(); let (mut asm, mut cb) = setup_asm(); // x0 and x1 form a cycle @@ -2642,7 +2657,6 @@ mod tests { #[test] fn test_ccall_resolve_parallel_moves_two_cycles() { - crate::options::rb_zjit_prepare_options(); let (mut asm, mut cb) = setup_asm(); // x0 and x1 form a cycle, and x2 and rcx form another cycle @@ -2669,7 +2683,6 @@ mod tests { #[test] fn test_ccall_resolve_parallel_moves_large_cycle() { - crate::options::rb_zjit_prepare_options(); let (mut asm, mut cb) = setup_asm(); // x0, x1, and x2 form a cycle @@ -2690,4 +2703,24 @@ mod tests { "); assert_snapshot!(cb.hexdump(), @"ef0300aae00301aae10302aae2030faa100080d200023fd6"); } + + #[test] + fn test_split_spilled_lshift() { + let (mut asm, mut cb) = setup_asm(); + + let opnd_vreg = asm.load(1.into()); + let out_vreg = asm.lshift(opnd_vreg, Opnd::UImm(1)); + asm.mov(C_RET_OPND, out_vreg); + asm.compile_with_num_regs(&mut cb, 0); // spill every VReg + + assert_disasm_snapshot!(cb.disasm(), @r" + 0x0: mov x16, #1 + 0x4: stur x16, [x29, #-8] + 0x8: ldur x15, [x29, #-8] + 0xc: lsl x15, x15, #1 + 0x10: stur x15, [x29, #-8] + 0x14: ldur x0, [x29, #-8] + "); + assert_snapshot!(cb.hexdump(), @"300080d2b0831ff8af835ff8eff97fd3af831ff8a0835ff8"); + } } diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index 136f7b452fcd38..b26d2ffa047c30 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -579,7 +579,7 @@ pub enum SendFallbackReason { BmethodNonIseqProc, /// The call has at least one feature on the caller or callee side that the optimizer does not /// support. - FancyFeatureUse, + ComplexArgPass, /// Initial fallback reason for every instruction, which should be mutated to /// a more actionable reason when an attempt to specialize the instruction fails. NotOptimizedInstruction(ruby_vminsn_type), @@ -1417,12 +1417,12 @@ fn can_direct_send(function: &mut Function, block: BlockId, iseq: *const rb_iseq }; use Counter::*; - if unsafe { rb_get_iseq_flags_has_rest(iseq) } { count_failure(fancy_arg_pass_param_rest) } - if unsafe { rb_get_iseq_flags_has_opt(iseq) } { count_failure(fancy_arg_pass_param_opt) } - if unsafe { rb_get_iseq_flags_has_kw(iseq) } { count_failure(fancy_arg_pass_param_kw) } - if unsafe { rb_get_iseq_flags_has_kwrest(iseq) } { count_failure(fancy_arg_pass_param_kwrest) } - if unsafe { rb_get_iseq_flags_has_block(iseq) } { count_failure(fancy_arg_pass_param_block) } - if unsafe { rb_get_iseq_flags_forwardable(iseq) } { count_failure(fancy_arg_pass_param_forwardable) } + if unsafe { rb_get_iseq_flags_has_rest(iseq) } { count_failure(complex_arg_pass_param_rest) } + if unsafe { rb_get_iseq_flags_has_opt(iseq) } { count_failure(complex_arg_pass_param_opt) } + if unsafe { rb_get_iseq_flags_has_kw(iseq) } { count_failure(complex_arg_pass_param_kw) } + if unsafe { rb_get_iseq_flags_has_kwrest(iseq) } { count_failure(complex_arg_pass_param_kwrest) } + if unsafe { rb_get_iseq_flags_has_block(iseq) } { count_failure(complex_arg_pass_param_block) } + if unsafe { rb_get_iseq_flags_forwardable(iseq) } { count_failure(complex_arg_pass_param_forwardable) } can_send } @@ -2153,16 +2153,16 @@ impl Function { self.push_insn(block, Insn::GuardType { val, guard_type, state }) } - fn count_fancy_call_features(&mut self, block: BlockId, ci_flags: c_uint) { + fn count_complex_call_features(&mut self, block: BlockId, ci_flags: c_uint) { use Counter::*; - if 0 != ci_flags & VM_CALL_ARGS_SPLAT { self.push_insn(block, Insn::IncrCounter(fancy_arg_pass_caller_splat)); } - if 0 != ci_flags & VM_CALL_ARGS_BLOCKARG { self.push_insn(block, Insn::IncrCounter(fancy_arg_pass_caller_blockarg)); } - if 0 != ci_flags & VM_CALL_KWARG { self.push_insn(block, Insn::IncrCounter(fancy_arg_pass_caller_kwarg)); } - if 0 != ci_flags & VM_CALL_KW_SPLAT { self.push_insn(block, Insn::IncrCounter(fancy_arg_pass_caller_kw_splat)); } - if 0 != ci_flags & VM_CALL_TAILCALL { self.push_insn(block, Insn::IncrCounter(fancy_arg_pass_caller_tailcall)); } - if 0 != ci_flags & VM_CALL_SUPER { self.push_insn(block, Insn::IncrCounter(fancy_arg_pass_caller_super)); } - if 0 != ci_flags & VM_CALL_ZSUPER { self.push_insn(block, Insn::IncrCounter(fancy_arg_pass_caller_zsuper)); } - if 0 != ci_flags & VM_CALL_FORWARDING { self.push_insn(block, Insn::IncrCounter(fancy_arg_pass_caller_forwarding)); } + if 0 != ci_flags & VM_CALL_ARGS_SPLAT { self.push_insn(block, Insn::IncrCounter(complex_arg_pass_caller_splat)); } + if 0 != ci_flags & VM_CALL_ARGS_BLOCKARG { self.push_insn(block, Insn::IncrCounter(complex_arg_pass_caller_blockarg)); } + if 0 != ci_flags & VM_CALL_KWARG { self.push_insn(block, Insn::IncrCounter(complex_arg_pass_caller_kwarg)); } + if 0 != ci_flags & VM_CALL_KW_SPLAT { self.push_insn(block, Insn::IncrCounter(complex_arg_pass_caller_kw_splat)); } + if 0 != ci_flags & VM_CALL_TAILCALL { self.push_insn(block, Insn::IncrCounter(complex_arg_pass_caller_tailcall)); } + if 0 != ci_flags & VM_CALL_SUPER { self.push_insn(block, Insn::IncrCounter(complex_arg_pass_caller_super)); } + if 0 != ci_flags & VM_CALL_ZSUPER { self.push_insn(block, Insn::IncrCounter(complex_arg_pass_caller_zsuper)); } + if 0 != ci_flags & VM_CALL_FORWARDING { self.push_insn(block, Insn::IncrCounter(complex_arg_pass_caller_forwarding)); } } fn try_rewrite_fixnum_op(&mut self, block: BlockId, orig_insn_id: InsnId, f: &dyn Fn(InsnId, InsnId) -> Insn, bop: u32, left: InsnId, right: InsnId, state: InsnId) { @@ -2290,6 +2290,8 @@ impl Function { // do not optimize into a `SendWithoutBlockDirect`. let flags = unsafe { rb_vm_ci_flag(ci) }; if unspecializable_call_type(flags) { + self.count_complex_call_features(block, flags); + self.set_dynamic_send_reason(insn_id, ComplexArgPass); self.push_insn_id(block, insn_id); continue; } @@ -2314,7 +2316,7 @@ impl Function { // TODO(max): Handle other kinds of parameter passing let iseq = unsafe { get_def_iseq_ptr((*cme).def) }; if !can_direct_send(self, block, iseq) { - self.set_dynamic_send_reason(insn_id, FancyFeatureUse); + self.set_dynamic_send_reason(insn_id, ComplexArgPass); self.push_insn_id(block, insn_id); continue; } self.push_insn(block, Insn::PatchPoint { invariant: Invariant::MethodRedefined { klass, method: mid, cme }, state }); @@ -2340,7 +2342,7 @@ impl Function { let iseq = unsafe { *capture.code.iseq.as_ref() }; if !can_direct_send(self, block, iseq) { - self.set_dynamic_send_reason(insn_id, FancyFeatureUse); + self.set_dynamic_send_reason(insn_id, ComplexArgPass); self.push_insn_id(block, insn_id); continue; } // Can't pass a block to a block for now @@ -2770,6 +2772,8 @@ impl Function { // When seeing &block argument, fall back to dynamic dispatch for now // TODO: Support block forwarding if unspecializable_call_type(ci_flags) { + fun.count_complex_call_features(block, ci_flags); + fun.set_dynamic_send_reason(send_insn_id, ComplexArgPass); return Err(()); } @@ -2876,7 +2880,8 @@ impl Function { // Filter for simple call sites (i.e. no splats etc.) if ci_flags & VM_CALL_ARGS_SIMPLE == 0 { - fun.count_fancy_call_features(block, ci_flags); + fun.count_complex_call_features(block, ci_flags); + fun.set_dynamic_send_reason(send_insn_id, ComplexArgPass); return Err(()); } @@ -2948,7 +2953,10 @@ impl Function { // func(int argc, VALUE *argv, VALUE recv) let ci_flags = unsafe { vm_ci_flag(call_info) }; if ci_flags & VM_CALL_ARGS_SIMPLE == 0 { - fun.count_fancy_call_features(block, ci_flags); + // TODO(alan): Add fun.count_complex_call_features() here without double + // counting splat + fun.set_dynamic_send_reason(send_insn_id, ComplexArgPass); + return Err(()); } else { fun.gen_patch_points_for_optimized_ccall(block, recv_class, method_id, cme, state); diff --git a/zjit/src/hir/opt_tests.rs b/zjit/src/hir/opt_tests.rs index 07f80c06824b71..6fc890f385a46c 100644 --- a/zjit/src/hir/opt_tests.rs +++ b/zjit/src/hir/opt_tests.rs @@ -2549,7 +2549,7 @@ mod hir_opt_tests { Jump bb2(v4) bb2(v6:BasicObject): v10:Fixnum[1] = Const Value(1) - IncrCounter fancy_arg_pass_param_opt + IncrCounter complex_arg_pass_param_opt v12:BasicObject = SendWithoutBlock v6, :foo, v10 CheckInterrupts Return v12 @@ -2633,7 +2633,7 @@ mod hir_opt_tests { Jump bb2(v4) bb2(v6:BasicObject): v10:Fixnum[1] = Const Value(1) - IncrCounter fancy_arg_pass_param_rest + IncrCounter complex_arg_pass_param_rest v12:BasicObject = SendWithoutBlock v6, :foo, v10 CheckInterrupts Return v12 @@ -2940,9 +2940,9 @@ mod hir_opt_tests { v12:NilClass = Const Value(nil) PatchPoint MethodRedefined(Hash@0x1008, new@0x1010, cme:0x1018) v43:HashExact = ObjectAllocClass Hash:VALUE(0x1008) - IncrCounter fancy_arg_pass_param_opt - IncrCounter fancy_arg_pass_param_kw - IncrCounter fancy_arg_pass_param_block + IncrCounter complex_arg_pass_param_opt + IncrCounter complex_arg_pass_param_kw + IncrCounter complex_arg_pass_param_block v18:BasicObject = SendWithoutBlock v43, :initialize CheckInterrupts CheckInterrupts @@ -4815,6 +4815,7 @@ mod hir_opt_tests { v14:ArrayExact = NewArray GuardBlockParamProxy l0 v17:HeapObject[BlockParamProxy] = Const Value(VALUE(0x1000)) + IncrCounter complex_arg_pass_caller_blockarg v19:BasicObject = Send v14, 0x1008, :map, v17 CheckInterrupts Return v19 @@ -6841,6 +6842,51 @@ mod hir_opt_tests { "); } + #[test] + fn test_splat() { + eval(" + def foo = itself + + def test + # Use a local to inhibit compile.c peephole optimization to ensure callsites have VM_CALL_ARGS_SPLAT + empty = [] + foo(*empty) + ''.display(*empty) + itself(*empty) + end + test + "); + assert_snapshot!(hir_string("test"), @r" + fn test@:6: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:NilClass = Const Value(nil) + Jump bb2(v1, v2) + bb1(v5:BasicObject): + EntryPoint JIT(0) + v6:NilClass = Const Value(nil) + Jump bb2(v5, v6) + bb2(v8:BasicObject, v9:NilClass): + v14:ArrayExact = NewArray + v18:ArrayExact = ToArray v14 + IncrCounter complex_arg_pass_caller_splat + v20:BasicObject = SendWithoutBlock v8, :foo, v18 + v23:StringExact[VALUE(0x1000)] = Const Value(VALUE(0x1000)) + v25:StringExact = StringCopy v23 + PatchPoint NoEPEscape(test) + v29:ArrayExact = ToArray v14 + IncrCounter complex_arg_pass_caller_splat + v31:BasicObject = SendWithoutBlock v25, :display, v29 + PatchPoint NoEPEscape(test) + v37:ArrayExact = ToArray v14 + IncrCounter complex_arg_pass_caller_splat + v39:BasicObject = SendWithoutBlock v8, :itself, v37 + CheckInterrupts + Return v39 + "); + } + #[test] fn test_inline_symbol_to_sym() { eval(r#" @@ -7222,7 +7268,7 @@ mod hir_opt_tests { } #[test] - fn counting_fancy_feature_use_for_fallback() { + fn counting_complex_feature_use_for_fallback() { eval(" define_method(:fancy) { |_a, *_b, kw: 100, **kw_rest, &block| } def test = fancy(1) @@ -7239,10 +7285,10 @@ mod hir_opt_tests { Jump bb2(v4) bb2(v6:BasicObject): v10:Fixnum[1] = Const Value(1) - IncrCounter fancy_arg_pass_param_rest - IncrCounter fancy_arg_pass_param_kw - IncrCounter fancy_arg_pass_param_kwrest - IncrCounter fancy_arg_pass_param_block + IncrCounter complex_arg_pass_param_rest + IncrCounter complex_arg_pass_param_kw + IncrCounter complex_arg_pass_param_kwrest + IncrCounter complex_arg_pass_param_block v12:BasicObject = SendWithoutBlock v6, :fancy, v10 CheckInterrupts Return v12 @@ -7266,7 +7312,7 @@ mod hir_opt_tests { EntryPoint JIT(0) Jump bb2(v4) bb2(v6:BasicObject): - IncrCounter fancy_arg_pass_param_forwardable + IncrCounter complex_arg_pass_param_forwardable v11:BasicObject = SendWithoutBlock v6, :forwardable CheckInterrupts Return v11 diff --git a/zjit/src/stats.rs b/zjit/src/stats.rs index 35af2b1d9d3bea..30a09669940679 100644 --- a/zjit/src/stats.rs +++ b/zjit/src/stats.rs @@ -177,7 +177,7 @@ make_counters! { send_fallback_ccall_with_frame_too_many_args, // The call has at least one feature on the caller or callee side // that the optimizer does not support. - send_fallback_fancy_call_feature, + send_fallback_one_or_more_complex_arg_pass, send_fallback_bmethod_non_iseq_proc, send_fallback_obj_to_string_not_string, send_fallback_not_optimized_instruction, @@ -257,22 +257,22 @@ make_counters! { unspecialized_send_def_type_null, // Unsupported parameter features - fancy_arg_pass_param_rest, - fancy_arg_pass_param_opt, - fancy_arg_pass_param_kw, - fancy_arg_pass_param_kwrest, - fancy_arg_pass_param_block, - fancy_arg_pass_param_forwardable, + complex_arg_pass_param_rest, + complex_arg_pass_param_opt, + complex_arg_pass_param_kw, + complex_arg_pass_param_kwrest, + complex_arg_pass_param_block, + complex_arg_pass_param_forwardable, // Unsupported caller side features - fancy_arg_pass_caller_splat, - fancy_arg_pass_caller_blockarg, - fancy_arg_pass_caller_kwarg, - fancy_arg_pass_caller_kw_splat, - fancy_arg_pass_caller_tailcall, - fancy_arg_pass_caller_super, - fancy_arg_pass_caller_zsuper, - fancy_arg_pass_caller_forwarding, + complex_arg_pass_caller_splat, + complex_arg_pass_caller_blockarg, + complex_arg_pass_caller_kwarg, + complex_arg_pass_caller_kw_splat, + complex_arg_pass_caller_tailcall, + complex_arg_pass_caller_super, + complex_arg_pass_caller_zsuper, + complex_arg_pass_caller_forwarding, // Writes to the VM frame vm_write_pc_count, @@ -427,7 +427,7 @@ pub fn send_fallback_counter(reason: crate::hir::SendFallbackReason) -> Counter SendWithoutBlockDirectTooManyArgs => send_fallback_send_without_block_direct_too_many_args, SendPolymorphic => send_fallback_send_polymorphic, SendNoProfiles => send_fallback_send_no_profiles, - FancyFeatureUse => send_fallback_fancy_call_feature, + ComplexArgPass => send_fallback_one_or_more_complex_arg_pass, BmethodNonIseqProc => send_fallback_bmethod_non_iseq_proc, SendNotOptimizedMethodType(_) => send_fallback_send_not_optimized_method_type, CCallWithFrameTooManyArgs => send_fallback_ccall_with_frame_too_many_args,