diff --git a/lib/bundler/cli.rb b/lib/bundler/cli.rb index a1d3bc02c503c4..f82a94fe6fd80e 100644 --- a/lib/bundler/cli.rb +++ b/lib/bundler/cli.rb @@ -627,15 +627,15 @@ def inject(*) end desc "lock", "Creates a lockfile without installing" - method_option "update", type: :array, lazy_default: true, banner: "ignore the existing lockfile, update all gems by default, or update list of given gems" + method_option "update", type: :array, lazy_default: true, repeatable: true, banner: "ignore the existing lockfile, update all gems by default, or update list of given gems" method_option "local", type: :boolean, default: false, banner: "do not attempt to fetch remote gemspecs and use the local gem cache only" method_option "print", type: :boolean, default: false, banner: "print the lockfile to STDOUT instead of writing to the file system" method_option "gemfile", type: :string, banner: "Use the specified gemfile instead of Gemfile" method_option "lockfile", type: :string, default: nil, banner: "the path the lockfile should be written to" method_option "full-index", type: :boolean, default: false, banner: "Fall back to using the single-file index of all gems" method_option "add-checksums", type: :boolean, default: false, banner: "Adds checksums to the lockfile" - method_option "add-platform", type: :array, default: [], banner: "Add a new platform to the lockfile" - method_option "remove-platform", type: :array, default: [], banner: "Remove a platform from the lockfile" + method_option "add-platform", type: :array, default: [], repeatable: true, banner: "Add a new platform to the lockfile" + method_option "remove-platform", type: :array, default: [], repeatable: true, banner: "Remove a platform from the lockfile" method_option "normalize-platforms", type: :boolean, default: false, banner: "Normalize lockfile platforms" method_option "patch", type: :boolean, banner: "If updating, prefer updating only to next patch version" method_option "minor", type: :boolean, banner: "If updating, prefer updating only to next minor version" diff --git a/lib/bundler/cli/lock.rb b/lib/bundler/cli/lock.rb index eeea5ae1d51bd0..dcb79068557f1c 100644 --- a/lib/bundler/cli/lock.rb +++ b/lib/bundler/cli/lock.rb @@ -26,6 +26,9 @@ def run Bundler::Fetcher.disable_endpoint = options["full-index"] update = options[:update] + # --update is repeatable, so it parses as an array with one entry per + # occurrence, where a bare `--update` produces a `true` entry + update = update.include?(true) ? true : update.flatten if update.is_a?(Array) conservative = options[:conservative] bundler = options[:bundler] @@ -44,12 +47,12 @@ def run Bundler::CLI::Common.configure_gem_version_promoter(definition, options) if options[:update] - options["remove-platform"].each do |platform_string| + options["remove-platform"].flatten.each do |platform_string| platform = Gem::Platform.new(platform_string) definition.remove_platform(platform) end - options["add-platform"].each do |platform_string| + options["add-platform"].flatten.each do |platform_string| platform = Gem::Platform.new(platform_string) if platform.to_s == "unknown" Bundler.ui.error "The platform `#{platform_string}` is unknown to RubyGems and can't be added to the lockfile." diff --git a/lib/bundler/definition.rb b/lib/bundler/definition.rb index b997b7e4767bd5..822572af43e6ae 100644 --- a/lib/bundler/definition.rb +++ b/lib/bundler/definition.rb @@ -416,7 +416,7 @@ def write_lock(file, preserve_unknown_sections) updating_major = locked_major < current_major end - preserve_unknown_sections ||= !updating_major && (Bundler.frozen_bundle? || !(unlocking? || @unlocking_bundler)) + preserve_unknown_sections ||= Bundler.frozen_bundle? || (!updating_major && !(unlocking? || @unlocking_bundler)) if File.exist?(file) && lockfiles_equal?(@lockfile_contents, contents, preserve_unknown_sections) return if Bundler.frozen_bundle? @@ -425,8 +425,10 @@ def write_lock(file, preserve_unknown_sections) end if Bundler.frozen_bundle? - Bundler.ui.error "Cannot write a changed lockfile while frozen." - return + msg = lockfile_changes_summary("frozen mode is set") || + "Your lockfile needs to be updated, but it can't be because frozen mode is set.\n\n" \ + "Run `bundle install` elsewhere and add the updated #{SharedHelpers.relative_lockfile_path} to version control." + raise ProductionError, msg end # Convert to \r\n if the existing lock has them, i.e., Windows with diff --git a/lib/bundler/lockfile_generator.rb b/lib/bundler/lockfile_generator.rb index 8ed50139642040..f2b9afb388f620 100644 --- a/lib/bundler/lockfile_generator.rb +++ b/lib/bundler/lockfile_generator.rb @@ -103,6 +103,11 @@ def add_section(name, value) end def bundler_checksum + # In frozen mode the lockfile can't change, so reproduce whatever bundler + # entry is already locked instead of recording one for the running bundler + # version, which may legitimately differ from the locked one. + return locked_bundler_checksum if Bundler.frozen_bundle? + # `.dev` versions and `SKIP_BUNDLER_CHECKSUM` are deliberate opt-outs (used # by Bundler/RubyGems' own development and release tasks): never record a # checksum for Bundler itself in those cases. @@ -138,5 +143,16 @@ def bundled_with_changing? locked_gems.bundler_version != definition.bundler_version_to_lock end + + def locked_bundler_checksum + locked_version = definition.locked_gems&.bundler_version + return [] unless locked_version + + metadata_source = definition.sources.metadata_source + locked_spec = LazySpecification.new("bundler", locked_version, Gem::Platform::RUBY, metadata_source) + return [] if metadata_source.checksum_store.missing?(locked_spec) + + [metadata_source.checksum_store.to_lock(locked_spec)] + end end end diff --git a/spec/bundler/cache/git_spec.rb b/spec/bundler/cache/git_spec.rb index f0976ecac7c93e..f0bff333cb4bd0 100644 --- a/spec/bundler/cache/git_spec.rb +++ b/spec/bundler/cache/git_spec.rb @@ -402,7 +402,7 @@ G lockfile <<~L GIT - remote: #{git_path}/ + remote: #{git_path} revision: #{locked_revision} specs: foo (1.0) diff --git a/spec/bundler/commands/cache_spec.rb b/spec/bundler/commands/cache_spec.rb index b33a5a386c7449..ad0afe390eb7df 100644 --- a/spec/bundler/commands/cache_spec.rb +++ b/spec/bundler/commands/cache_spec.rb @@ -349,15 +349,15 @@ GEM remote: https://gem.repo4/ specs: - foo (1.0.0) bar (1.0.0) + foo (1.0.0) PLATFORMS #{lockfile_platforms} DEPENDENCIES - foo bar + foo BUNDLED WITH #{Bundler::VERSION} diff --git a/spec/bundler/commands/lock_spec.rb b/spec/bundler/commands/lock_spec.rb index 4914c11cc3f8b5..97a3989d4b831c 100644 --- a/spec/bundler/commands/lock_spec.rb +++ b/spec/bundler/commands/lock_spec.rb @@ -463,6 +463,49 @@ expect(read_lockfile).to eq(expected_lockfile) end + it "updates gems given through repeated --update options" do + build_repo4 do + build_gem "foo", "1.0" + build_gem "foo", "2.0" + build_gem "bar", "1.0" + build_gem "bar", "2.0" + build_gem "baz", "1.0" + build_gem "baz", "2.0" + end + + gemfile <<-G + source "https://gem.repo4" + + gem "foo" + gem "bar" + gem "baz" + G + + lockfile <<~L + GEM + remote: https://gem.repo4/ + specs: + bar (1.0) + baz (1.0) + foo (1.0) + + PLATFORMS + #{lockfile_platforms} + + DEPENDENCIES + bar + baz + foo + + BUNDLED WITH + #{Bundler::VERSION} + L + + bundle "lock --update foo --update bar" + + expect(lockfile).to include("foo (2.0)", "bar (2.0)", "baz (1.0)") + end + it "updates specific gems using --update, even if that requires unlocking other top level gems" do build_repo4 do build_gem "prism", "0.15.1" @@ -853,6 +896,15 @@ expect(the_bundle.locked_platforms).to match_array(default_platform_list("java", "x86-mingw32")) end + it "supports adding platforms through repeated --add-platform options" do + gemfile_with_rails_weakling_and_foo_from_repo4 + + bundle "lock --add-platform java --add-platform x86-mingw32" + + allow(Bundler::SharedHelpers).to receive(:find_gemfile).and_return(bundled_app_gemfile) + expect(the_bundle.locked_platforms).to match_array(default_platform_list("java", "x86-mingw32")) + end + it "supports adding new platforms when a previous lockfile exists" do gemfile_with_rails_weakling_and_foo_from_repo4 @@ -975,6 +1027,19 @@ expect(the_bundle.locked_platforms).to match_array(default_platform_list("x86-mingw32")) end + it "supports removing platforms through repeated --remove-platform options" do + gemfile_with_rails_weakling_and_foo_from_repo4 + + bundle "lock --add-platform java x86-mingw32" + + allow(Bundler::SharedHelpers).to receive(:find_gemfile).and_return(bundled_app_gemfile) + expect(the_bundle.locked_platforms).to match_array(default_platform_list("java", "x86-mingw32")) + + bundle "lock --remove-platform java --remove-platform x86-mingw32" + + expect(the_bundle.locked_platforms).to match_array(default_platform_list) + end + it "also cleans up redundant platform gems when removing platforms" do build_repo4 do build_gem "nokogiri", "1.12.0" diff --git a/spec/bundler/install/deploy_spec.rb b/spec/bundler/install/deploy_spec.rb index 5bdb8f6194f518..c8947393c0895c 100644 --- a/spec/bundler/install/deploy_spec.rb +++ b/spec/bundler/install/deploy_spec.rb @@ -173,6 +173,75 @@ end.not_to change { bundled_app_lock.mtime } end + it "explodes if regenerating the lockfile would change it" do + lockfile lockfile. + sub(" myrack (1.0.0)", " myrack-obama (1.0)\n myrack (1.0.0)"). + sub(/^ myrack \(1\.0\.0\) sha256=\S+$/) {|line| "#{line}\n #{checksum_to_lock(gem_repo1, "myrack-obama", "1.0")}" } + + bundle :install, env: { "BUNDLE_FROZEN" => "true" }, raise_on_error: false + expect(err).to include("Your lockfile needs to be updated, but it can't be because frozen mode is set") + expect(last_command).to be_failure + end + + it "explodes on `bundle check` if the lockfile contains a gem bundler would prune" do + lockfile lockfile.sub(" myrack (1.0.0)", " myrack (1.0.0)\n myrack-obama (1.0)") + + bundle :check, env: { "BUNDLE_FROZEN" => "true" }, raise_on_error: false + expect(err).to include("but can't be updated because frozen mode is set") + expect(last_command).to be_failure + end + + it "works when the lockfile includes a checksum entry for bundler itself" do + lockfile <<~L + GEM + remote: https://gem.repo1/ + specs: + myrack (1.0.0) + + PLATFORMS + #{lockfile_platforms} + + DEPENDENCIES + myrack + + CHECKSUMS + bundler (#{Bundler::VERSION}) sha256=#{"a" * 64} + #{checksum_to_lock gem_repo1, "myrack", "1.0.0"} + + BUNDLED WITH + #{Bundler::VERSION} + L + + bundle :install, env: { "BUNDLE_FROZEN" => "true" } + expect(err).to be_empty + end + + it "explodes if the lockfile checksum entry for bundler does not match the BUNDLED WITH version" do + lockfile <<~L + GEM + remote: https://gem.repo1/ + specs: + myrack (1.0.0) + + PLATFORMS + #{lockfile_platforms} + + DEPENDENCIES + myrack + + CHECKSUMS + bundler (4.0.16) sha256=#{"a" * 64} + #{checksum_to_lock gem_repo1, "myrack", "1.0.0"} + + BUNDLED WITH + #{Bundler::VERSION} + L + + bundle :install, env: { "BUNDLE_FROZEN" => "true" }, raise_on_error: false + expect(err).to include("Your lockfile needs to be updated, but it can't be because frozen mode is set") + expect(last_command).to be_failure + end + it "explodes with the `deployment` setting if you make a change and don't check in the lockfile" do gemfile <<-G source "https://gem.repo1" diff --git a/spec/bundler/install/gemfile/platform_spec.rb b/spec/bundler/install/gemfile/platform_spec.rb index c28af3d4abfe5f..87e9dcf77ec0dc 100644 --- a/spec/bundler/install/gemfile/platform_spec.rb +++ b/spec/bundler/install/gemfile/platform_spec.rb @@ -54,7 +54,7 @@ it "pulls the pure ruby version on jruby if the java platform is not present in the lockfile and bundler is run in frozen mode", :jruby_only do lockfile <<-G GEM - remote: https://gem.repo1 + remote: https://gem.repo1/ specs: platform_specific (1.0) diff --git a/transcode.c b/transcode.c index b657ff18708219..e4400f6622d86a 100644 --- a/transcode.c +++ b/transcode.c @@ -3013,7 +3013,7 @@ econv_free(void *ptr) static size_t econv_memsize(const void *ptr) { - return sizeof(rb_econv_t); + return ptr ? rb_econv_memsize((rb_econv_t *)ptr) : 0; } static const rb_data_type_t econv_data_type = { diff --git a/vm_insnhelper.c b/vm_insnhelper.c index b5297bba22750e..ce8e21d0fde2ff 100644 --- a/vm_insnhelper.c +++ b/vm_insnhelper.c @@ -2251,6 +2251,15 @@ rb_vm_search_method_slowpath(const struct rb_callinfo *ci, VALUE klass) return cc; } +#if VM_CHECK_MODE > 0 +static bool +vm_cd_owned_by_iseq_p(const struct rb_call_data *cd, const rb_iseq_t *iseq) +{ + const struct rb_iseq_constant_body *body = ISEQ_BODY(iseq); + return cd >= body->call_data && cd < body->call_data + body->ci_size; +} +#endif + static const struct rb_callcache * vm_search_method_slowpath0(VALUE cd_owner, struct rb_call_data *cd, VALUE klass) { @@ -2265,6 +2274,10 @@ vm_search_method_slowpath0(VALUE cd_owner, struct rb_call_data *cd, VALUE klass) const struct rb_callcache *empty_cc = &vm_empty_cc; if (cd_owner && cc != empty_cc) { + // invokesuper dispatches with a cd on the machine stack, which has no + // owning iseq to remember and keeps its ci out of the GC. + VM_ASSERT(!vm_ci_markable(cd->ci) || + vm_cd_owned_by_iseq_p(cd, (const rb_iseq_t *)cd_owner)); RB_OBJ_WRITTEN(cd_owner, Qundef, cc); } @@ -2295,12 +2308,10 @@ vm_search_method_slowpath0(VALUE cd_owner, struct rb_call_data *cd, VALUE klass) return cc; } -ALWAYS_INLINE(static const struct rb_callcache *vm_search_method_fastpath(const struct rb_control_frame_struct *reg_cfp, struct rb_call_data *cd, VALUE klass)); -static const struct rb_callcache * -vm_search_method_fastpath(const struct rb_control_frame_struct *reg_cfp, struct rb_call_data *cd, VALUE klass) +ALWAYS_INLINE(static bool vm_cc_hit_p(const struct rb_callcache *cc, const struct rb_call_data *cd, VALUE klass)); +static bool +vm_cc_hit_p(const struct rb_callcache *cc, const struct rb_call_data *cd, VALUE klass) { - const struct rb_callcache *cc = cd->cc; - VM_ASSERT_TYPE2(klass, T_CLASS, T_ICLASS); #if OPT_INLINE_METHOD_CACHE @@ -2312,7 +2323,7 @@ vm_search_method_fastpath(const struct rb_control_frame_struct *reg_cfp, struct (vm_ci_flag(cd->ci) & VM_CALL_SUPER) || // search_super w/ define_method vm_cc_cme(cc)->called_id == vm_ci_mid(cd->ci)); // cme->called_id == ci->mid - return cc; + return true; } RB_DEBUG_COUNTER_INC(mc_inline_miss_invalidated); } @@ -2321,6 +2332,18 @@ vm_search_method_fastpath(const struct rb_control_frame_struct *reg_cfp, struct } #endif + return false; +} + +ALWAYS_INLINE(static const struct rb_callcache *vm_search_method_fastpath(const struct rb_control_frame_struct *reg_cfp, struct rb_call_data *cd, VALUE klass)); +static const struct rb_callcache * +vm_search_method_fastpath(const struct rb_control_frame_struct *reg_cfp, struct rb_call_data *cd, VALUE klass) +{ + const struct rb_callcache *cc = cd->cc; + if (vm_cc_hit_p(cc, cd, klass)) { + return cc; + } + return vm_search_method_slowpath0((VALUE)CFP_ISEQ(reg_cfp), cd, klass); } @@ -2339,9 +2362,12 @@ const struct rb_callable_method_entry_struct * rb_zjit_vm_search_method(VALUE cd_owner, struct rb_call_data *cd, VALUE recv) { // Called from ZJIT with the compile-time iseq, which may differ from - // the iseq on the current CFP. Use the slowpath to avoid stale caches. + // the iseq on the current CFP. VALUE klass = CLASS_OF(recv); - const struct rb_callcache *cc = vm_search_method_slowpath0(cd_owner, cd, klass); + const struct rb_callcache *cc = cd->cc; + if (!vm_cc_hit_p(cc, cd, klass)) { + cc = vm_search_method_slowpath0(cd_owner, cd, klass); + } return vm_cc_cme(cc); } @@ -2418,11 +2444,7 @@ rb_zjit_cme_is_cfunc(const rb_callable_method_entry_t *me, const cfunc_type func int rb_vm_method_cfunc_is(const rb_iseq_t *iseq, CALL_DATA cd, VALUE recv, cfunc_type func) { - // Called from ZJIT with the compile-time iseq, which may differ from - // the iseq on the current CFP. Use the slowpath to avoid stale caches. - VALUE klass = CLASS_OF(recv); - const struct rb_callcache *cc = vm_search_method_slowpath0((VALUE)iseq, cd, klass); - const struct rb_callable_method_entry_struct *cme = vm_cc_cme(cc); + const struct rb_callable_method_entry_struct *cme = rb_zjit_vm_search_method((VALUE)iseq, cd, recv); return check_cfunc(cme, func); } diff --git a/zjit/src/backend/lir.rs b/zjit/src/backend/lir.rs index cf5f1a40d751f7..7c91d91691b072 100644 --- a/zjit/src/backend/lir.rs +++ b/zjit/src/backend/lir.rs @@ -1,4 +1,5 @@ use std::cell::{Cell, RefCell}; +use std::collections::hash_map::Entry; use std::collections::{HashMap, HashSet, VecDeque}; use std::fmt; use std::mem::take; @@ -2489,15 +2490,13 @@ impl Assembler // Count predecessors for each block let mut num_predecessors: HashMap = HashMap::new(); - for block_id in self.block_order() { + let block_order = self.block_order(); + for &block_id in &block_order { for succ in self.basic_blocks[block_id.0].successors() { *num_predecessors.entry(succ).or_insert(0) += 1; } } - // Collect block order upfront so we don't borrow self while mutating - let block_order = self.block_order(); - // This code is iterating over each block in our CFG and inserting // copy instructions at each edge. for &pred_id in &block_order { @@ -2653,7 +2652,7 @@ impl Assembler } } - self.rewrite_instructions(intervals, regs); + self.rewrite_instructions(&block_order, intervals, regs); } /// Handle caller-saved registers around CCall instructions. @@ -2953,8 +2952,8 @@ impl Assembler /// Walk every instruction and replace VReg operands with the physical /// register (or stack slot) assigned to the VReg's interval. - fn rewrite_instructions(&mut self, intervals: &[Interval], regs: &RegPool) { - for block_id in self.block_order() { + fn rewrite_instructions(&mut self, block_order: &[BlockId], intervals: &[Interval], regs: &RegPool) { + for &block_id in block_order { for insn in self.basic_blocks[block_id.0].insns.iter_mut() { insn.for_each_operand_mut(|opnd| { Self::rewrite_opnd(opnd, intervals, regs); @@ -3293,15 +3292,16 @@ impl Assembler }; // Compile the shared side exit if not compiled yet - let compiled_exit = if let Some(&compiled_exit) = compiled_exits.get(&exit) { - Target::Label(compiled_exit) - } else { - let new_exit = self.new_label("side_exit"); - self.write_label(new_exit.clone()); - asm_comment!(self, "Exit: {}", exit.pc); - compile_exit(self, &exit, None); - compiled_exits.insert(exit, new_exit.unwrap_label()); - new_exit + let compiled_exit = match compiled_exits.entry(exit) { + Entry::Occupied(entry) => Target::Label(*entry.get()), + Entry::Vacant(entry) => { + let new_exit = self.new_label("side_exit"); + self.write_label(new_exit.clone()); + asm_comment!(self, "Exit: {}", entry.key().pc); + compile_exit(self, entry.key(), None); + entry.insert(new_exit.unwrap_label()); + new_exit + } }; *self.basic_blocks[block_id].insns[idx].target_mut().unwrap() = counted_exit.unwrap_or(compiled_exit); diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index 86df1e3abf9045..b49753635e2b13 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -4979,7 +4979,8 @@ impl Function { } &Insn::IsMethodCfunc { val, cd, cfunc, state } if self.type_of(val).ruby_object_known() => { let class = self.type_of(val).ruby_object().unwrap(); - let cme = unsafe { rb_zjit_vm_search_method(self.iseq.into(), cd as *mut rb_call_data, class) }; + let cd_owner = self.frame_state_iseq(state); + let cme = unsafe { rb_zjit_vm_search_method(cd_owner.into(), cd as *mut rb_call_data, class) }; let is_expected_cfunc = unsafe { rb_zjit_cme_is_cfunc(cme, cfunc as *const c_void) }; let method = unsafe { rb_vm_ci_mid((*cd).ci) }; self.push_insn(block, Insn::PatchPoint { invariant: Invariant::MethodRedefined { klass: class, method, cme }, state });