From b70ff3a823751be6f45c38de041fac727930c1bc Mon Sep 17 00:00:00 2001 From: Ian MacDonald Date: Mon, 5 Oct 2026 11:10:01 -0700 Subject: [PATCH] Bundler: update git pins declared on git/github blocks GitPinReplacer only rewrote `tag:`/`ref:` on `gem "x", git: ..., tag: ...` declarations. When a gem is declared inside a `git "...", tag: "..." do` (or `github`) block, the pin lives on the block's call, so the Gemfile was left unchanged, the lockfile re-resolved to identical content, and LockfileUpdater raised "Expected content to change!". Rewrite the pin on a git/github block when its body declares the targeted gem. Fixes #16435 Co-Authored-By: Claude Opus 5.5 --- .../bundler/file_updater/git_pin_replacer.rb | 39 ++++++++++- .../file_updater/git_pin_replacer_spec.rb | 65 +++++++++++++++++++ .../dependabot/bundler/file_updater_spec.rb | 52 +++++++++++++++ .../bundler2/git_source_block/Gemfile | 8 +++ .../bundler2/git_source_block/Gemfile.lock | 21 ++++++ 5 files changed, 182 insertions(+), 3 deletions(-) create mode 100644 bundler/spec/fixtures/projects/bundler2/git_source_block/Gemfile create mode 100644 bundler/spec/fixtures/projects/bundler2/git_source_block/Gemfile.lock diff --git a/bundler/lib/dependabot/bundler/file_updater/git_pin_replacer.rb b/bundler/lib/dependabot/bundler/file_updater/git_pin_replacer.rb index 2eb1a76fa27..597dbc15657 100644 --- a/bundler/lib/dependabot/bundler/file_updater/git_pin_replacer.rb +++ b/bundler/lib/dependabot/bundler/file_updater/git_pin_replacer.rb @@ -39,6 +39,7 @@ class Rewriter < Parser::TreeRewriter extend T::Sig PIN_KEYS = %i(ref tag).freeze + GIT_BLOCK_METHODS = %i(git github).freeze sig { returns(Dependabot::Dependency) } attr_reader :dependency @@ -56,9 +57,24 @@ def initialize(dependency:, new_pin:) sig { params(node: Parser::AST::Node).void } def on_send(node) return unless declares_targeted_gem?(node) - return unless node.children.last.type == :hash + update_pins(node) + end + + sig { params(node: Parser::AST::Node).void } + def on_block(node) + update_pins(node.children.first) if git_block_declaring_targeted_gem?(node) + + super + end + + private + + sig { params(node: Parser::AST::Node).void } + def update_pins(node) kwargs_node = node.children.last + return unless kwargs_node.is_a?(Parser::AST::Node) && kwargs_node.type == :hash + kwargs_node.children.each do |hash_pair| next unless PIN_KEYS.include?(key_from_hash_pair(hash_pair)) @@ -66,8 +82,6 @@ def on_send(node) end end - private - sig { params(node: Parser::AST::Node).returns(T::Boolean) } def declares_targeted_gem?(node) return false unless node.children[1] == :gem @@ -75,6 +89,25 @@ def declares_targeted_gem?(node) node.children[2].children.first == dependency.name end + sig { params(node: Parser::AST::Node).returns(T::Boolean) } + def git_block_declaring_targeted_gem?(node) + send_node, _args, body = node.children + return false unless send_node.type == :send + return false unless send_node.children[0].nil? && GIT_BLOCK_METHODS.include?(send_node.children[1]) + + block_statements(body).any? do |statement| + statement.type == :send && declares_targeted_gem?(statement) + end + end + + sig { params(body: T.nilable(Parser::AST::Node)).returns(T::Array[Parser::AST::Node]) } + def block_statements(body) + return [] if body.nil? + return body.children.grep(Parser::AST::Node) if body.type == :begin + + [body] + end + sig { params(node: Parser::AST::Node).returns(Symbol) } def key_from_hash_pair(node) node.children.first.children.first.to_sym diff --git a/bundler/spec/dependabot/bundler/file_updater/git_pin_replacer_spec.rb b/bundler/spec/dependabot/bundler/file_updater/git_pin_replacer_spec.rb index 956d2d38192..9f6e201ba9c 100644 --- a/bundler/spec/dependabot/bundler/file_updater/git_pin_replacer_spec.rb +++ b/bundler/spec/dependabot/bundler/file_updater/git_pin_replacer_spec.rb @@ -68,5 +68,70 @@ expect(rewrite).to include(%(tag: %(new_ref))) end end + + context "with a dependency declared inside a git block" do + let(:content) do + <<~GEMFILE + git "https://x.com/monorepo", tag: "v1", glob: "gems/*/*.gemspec" do + gem "business" + end + GEMFILE + end + + it "replaces the tag on the git block" do + expect(rewrite).to include(%(git "https://x.com/monorepo", tag: "new_ref", glob: "gems/*/*.gemspec" do)) + end + end + + context "with a dependency declared inside a git block alongside other gems" do + let(:content) do + <<~GEMFILE + git "https://x.com/monorepo", ref: "a1b78a9" do + gem "statesman" + gem "business" + end + + git "https://x.com/other", tag: "v0.11.6" do + gem "que" + end + GEMFILE + end + + it "replaces the ref on the git block that declares the dependency" do + expect(rewrite).to include(%(git "https://x.com/monorepo", ref: "new_ref" do)) + end + + it "leaves other git blocks alone" do + expect(rewrite).to include(%(git "https://x.com/other", tag: "v0.11.6" do)) + end + end + + context "with a dependency declared inside a github block" do + let(:content) do + <<~GEMFILE + github "org/monorepo", tag: 'v1' do + gem "business" + end + GEMFILE + end + + it "replaces the tag on the github block" do + expect(rewrite).to include(%(github "org/monorepo", tag: 'new_ref' do)) + end + end + + context "with a dependency declared inside a non-git block" do + let(:content) do + <<~GEMFILE + group :development, tag: "v1" do + gem "business" + end + GEMFILE + end + + it "leaves the block alone" do + expect(rewrite).to eq(content) + end + end end end diff --git a/bundler/spec/dependabot/bundler/file_updater_spec.rb b/bundler/spec/dependabot/bundler/file_updater_spec.rb index 3796c6b431d..e7bc36d88e5 100644 --- a/bundler/spec/dependabot/bundler/file_updater_spec.rb +++ b/bundler/spec/dependabot/bundler/file_updater_spec.rb @@ -808,6 +808,58 @@ .to eq(old_lock.index(original_remote_line)) end + context "when the dependency is declared inside a tag-pinned git block" do + let(:project_name) { "git_source_block" } + let(:dependency) do + Dependabot::Dependency.new( + name: "que", + version: "07f74f25cc1432acc84a857d2b356ec5c9a37d48", + previous_version: "997d1a6ee76a1f254fd72ce16acbc8d347fcaee3", + requirements: requirements, + previous_requirements: previous_requirements, + package_manager: "bundler" + ) + end + let(:requirements) do + [{ + file: "Gemfile", + requirement: ">= 0", + groups: [], + source: { + type: "git", + url: "https://github.com/dependabot-fixtures/que", + branch: nil, + ref: "v0.12.0" + } + }] + end + let(:previous_requirements) do + [{ + file: "Gemfile", + requirement: ">= 0", + groups: [], + source: { + type: "git", + url: "https://github.com/dependabot-fixtures/que", + branch: nil, + ref: "v0.11.6" + } + }] + end + + it "updates the tag on the git block" do + gemfile = updated_files.find { |f| f.name == "Gemfile" } + + expect(gemfile.content) + .to include(%(git "https://github.com/dependabot-fixtures/que", tag: "v0.12.0" do)) + end + + it "updates the dependency's tag and revision in the lockfile" do + expect(file.content).to include("tag: v0.12.0") + expect(file.content).not_to include("997d1a6ee76a1f254fd72ce16acbc8d347fcaee3") + end + end + context "when a git source is specified that multiple deps use" do let(:project_name) { "git_source_with_multiple_deps" } let(:dependency) do diff --git a/bundler/spec/fixtures/projects/bundler2/git_source_block/Gemfile b/bundler/spec/fixtures/projects/bundler2/git_source_block/Gemfile new file mode 100644 index 00000000000..f3740138d43 --- /dev/null +++ b/bundler/spec/fixtures/projects/bundler2/git_source_block/Gemfile @@ -0,0 +1,8 @@ +# frozen_string_literal: true +source "https://rubygems.org" + +gem "statesman", "~> 1.2.0" + +git "https://github.com/dependabot-fixtures/que", tag: "v0.11.6" do + gem "que" +end diff --git a/bundler/spec/fixtures/projects/bundler2/git_source_block/Gemfile.lock b/bundler/spec/fixtures/projects/bundler2/git_source_block/Gemfile.lock new file mode 100644 index 00000000000..84d67d6bcf7 --- /dev/null +++ b/bundler/spec/fixtures/projects/bundler2/git_source_block/Gemfile.lock @@ -0,0 +1,21 @@ +GIT + remote: https://github.com/dependabot-fixtures/que + revision: 997d1a6ee76a1f254fd72ce16acbc8d347fcaee3 + tag: v0.11.6 + specs: + que (0.11.6) + +GEM + remote: https://rubygems.org/ + specs: + statesman (1.2.5) + +PLATFORMS + ruby + +DEPENDENCIES + que! + statesman (~> 1.2.0) + +BUNDLED WITH + 2.2.0