Skip to content

Commit a2a44cf

Browse files
hsbtclaude
andcommitted
Warn when duplicate source declarations conflict on cooldown
Declaring the same source URL twice with different cooldown values silently drops one of them, because duplicate declarations are deduped into a single source and cooldown applies per server, not per gem. The reporter of #9723 used a second source block with cooldown: 0 as a per-gem exemption and got no indication it had no effect. Keep the dedup behavior but warn when a conflicting cooldown value is discarded. #9723 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 6901334 commit a2a44cf

4 files changed

Lines changed: 120 additions & 1 deletion

File tree

lib/bundler/source_list.rb

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,13 @@ def add_git_source(options = {})
5050

5151
def add_rubygems_source(options = {})
5252
new_source = Source::Rubygems.new(options)
53-
return @global_rubygems_source if @global_rubygems_source == new_source
53+
if @global_rubygems_source == new_source
54+
warn_on_cooldown_conflict(new_source, @global_rubygems_source)
55+
return @global_rubygems_source
56+
end
57+
58+
existing_source = @rubygems_sources.find {|s| s == new_source }
59+
warn_on_cooldown_conflict(new_source, existing_source) if existing_source
5460

5561
add_source_to_list new_source, @rubygems_sources
5662
end
@@ -60,6 +66,13 @@ def add_plugin_source(source, options = {})
6066
end
6167

6268
def add_global_rubygems_remote(uri, cooldown: nil)
69+
unless cooldown.nil?
70+
new_source = source_class.new("remotes" => uri, "cooldown" => cooldown)
71+
[global_rubygems_source, *@rubygems_sources].find do |existing_source|
72+
warn_on_cooldown_conflict(new_source, existing_source)
73+
end
74+
end
75+
6376
global_rubygems_source.add_remote(uri, cooldown: cooldown)
6477
global_rubygems_source
6578
end
@@ -222,6 +235,22 @@ def source_list_for(source)
222235
end
223236
end
224237

238+
def warn_on_cooldown_conflict(new_source, existing_source)
239+
new_source.remote_cooldowns.any? do |uri, cooldown|
240+
next false unless existing_source.remotes.include?(uri)
241+
242+
existing_cooldown = existing_source.cooldown_for(uri)
243+
next false if existing_cooldown == cooldown
244+
245+
previous = existing_cooldown ? "`cooldown: #{existing_cooldown}`" : "no cooldown"
246+
Bundler.ui.warn "The source #{uri} is declared more than once with different cooldown " \
247+
"values (`cooldown: #{cooldown}` here, #{previous} previously). All declarations of " \
248+
"the same source URL share a single cooldown, so only one of these values will apply " \
249+
"to all gems from this source."
250+
true
251+
end
252+
end
253+
225254
def warn_on_git_protocol(source)
226255
return if Bundler.settings["git.allow_insecure"]
227256

spec/bundler/dsl_spec.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,7 @@
370370
describe "#source with cooldown" do
371371
before do
372372
allow(@rubygems).to receive(:add_remote)
373+
allow(@rubygems).to receive(:remote_cooldowns).and_return({})
373374
end
374375

375376
it "accepts a non-negative integer" do

spec/bundler/source_list_spec.rb

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,56 @@
161161
end
162162
end
163163

164+
describe "cooldown conflicts between duplicate source declarations" do
165+
it "warns when a block source duplicates the global source with a different cooldown" do
166+
source_list.add_global_rubygems_remote("https://rubygems.org", cooldown: 7)
167+
expect(Bundler.ui).to receive(:warn).with(/declared more than once with different cooldown values \(`cooldown: 0` here, `cooldown: 7` previously\)/)
168+
source_list.add_rubygems_source("remotes" => ["https://rubygems.org"], "cooldown" => 0)
169+
end
170+
171+
it "warns when a global source duplicates a block source with a different cooldown" do
172+
source_list.add_rubygems_source("remotes" => ["https://rubygems.org"], "cooldown" => 0)
173+
expect(Bundler.ui).to receive(:warn).with(/declared more than once with different cooldown values \(`cooldown: 7` here, `cooldown: 0` previously\)/)
174+
source_list.add_global_rubygems_remote("https://rubygems.org", cooldown: 7)
175+
end
176+
177+
it "warns when a block source duplicates another block source with a different cooldown" do
178+
source_list.add_rubygems_source("remotes" => ["https://rubygems.org"], "cooldown" => 7)
179+
expect(Bundler.ui).to receive(:warn).with(/declared more than once with different cooldown values/)
180+
source_list.add_rubygems_source("remotes" => ["https://rubygems.org"], "cooldown" => 0)
181+
end
182+
183+
it "warns when a global remote is declared again with a different cooldown" do
184+
source_list.add_global_rubygems_remote("https://rubygems.org", cooldown: 7)
185+
expect(Bundler.ui).to receive(:warn).with(/declared more than once with different cooldown values/)
186+
source_list.add_global_rubygems_remote("https://rubygems.org", cooldown: 0)
187+
end
188+
189+
it "warns when a duplicate declaration adds a cooldown to a source declared without one" do
190+
source_list.add_global_rubygems_remote("https://rubygems.org")
191+
expect(Bundler.ui).to receive(:warn).with(/declared more than once with different cooldown values \(`cooldown: 7` here, no cooldown previously\)/)
192+
source_list.add_rubygems_source("remotes" => ["https://rubygems.org"], "cooldown" => 7)
193+
end
194+
195+
it "does not warn when the duplicate declaration uses the same cooldown" do
196+
source_list.add_global_rubygems_remote("https://rubygems.org", cooldown: 7)
197+
expect(Bundler.ui).not_to receive(:warn)
198+
source_list.add_rubygems_source("remotes" => ["https://rubygems.org"], "cooldown" => 7)
199+
end
200+
201+
it "does not warn when the duplicate declaration has no cooldown" do
202+
source_list.add_global_rubygems_remote("https://rubygems.org", cooldown: 7)
203+
expect(Bundler.ui).not_to receive(:warn)
204+
source_list.add_rubygems_source("remotes" => ["https://rubygems.org"])
205+
end
206+
207+
it "does not warn when different sources declare different cooldowns" do
208+
source_list.add_global_rubygems_remote("https://rubygems.org", cooldown: 7)
209+
expect(Bundler.ui).not_to receive(:warn)
210+
source_list.add_rubygems_source("remotes" => ["https://other-rubygems.org"], "cooldown" => 0)
211+
end
212+
end
213+
164214
describe "#all_sources" do
165215
it "includes the global rubygems source when rubygems sources have been added" do
166216
source_list.add_git_source("uri" => "git://host/path.git")

spec/install/cooldown_spec.rb

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,45 @@
244244
expect(the_bundle).to include_gems("ripe_gem 1.0.0", "child 1.0.0")
245245
end
246246

247+
it "warns when the same source is declared again with a different cooldown and keeps the first value" do
248+
# https://github.com/rubygems/rubygems/issues/9723: a second declaration
249+
# of the same URL is deduped into the first one, so its cooldown cannot
250+
# act as a per-gem exemption.
251+
install_gemfile <<-G, artifice: "compact_index_cooldown"
252+
source "https://gem.repo3", cooldown: 7
253+
source "https://gem.repo3", cooldown: 0 do
254+
gem "ripe_gem"
255+
end
256+
G
257+
258+
expect(err).to include("The source https://gem.repo3/ is declared more than once with different cooldown values (`cooldown: 0` here, `cooldown: 7` previously).")
259+
expect(the_bundle).to include_gems("ripe_gem 1.0.0")
260+
end
261+
262+
it "does not warn when the same source is declared again without a cooldown" do
263+
install_gemfile <<-G, artifice: "compact_index_cooldown"
264+
source "https://gem.repo3", cooldown: 7
265+
source "https://gem.repo3" do
266+
gem "ripe_gem"
267+
end
268+
G
269+
270+
expect(err).not_to include("cooldown")
271+
expect(the_bundle).to include_gems("ripe_gem 1.0.0")
272+
end
273+
274+
it "does not warn when the same source is declared again with the same cooldown" do
275+
install_gemfile <<-G, artifice: "compact_index_cooldown"
276+
source "https://gem.repo3", cooldown: 7
277+
source "https://gem.repo3", cooldown: 7 do
278+
gem "ripe_gem"
279+
end
280+
G
281+
282+
expect(err).not_to include("cooldown")
283+
expect(the_bundle).to include_gems("ripe_gem 1.0.0")
284+
end
285+
247286
it "is overridden by CLI --cooldown when Gemfile sets a different per-source value" do
248287
gemfile <<-G
249288
source "https://gem.repo3", cooldown: 0

0 commit comments

Comments
 (0)