Skip to content

Fix some invalid options to gem DSL not getting reported as invalid #8480

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 12, 2025
+111 −86
Merged

Fix some invalid options to gem DSL not getting reported as invalid #8480

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e7f5f06
Fix spec wording
deivid-rodriguez Feb 10, 2025
5b6077a
Fix `type` and `gemfile` not getting reported as invalid options
deivid-rodriguez Feb 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
159 changes: 82 additions & 77 deletions bundler/lib/bundler/dsl.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def self.evaluate(gemfile, lockfile, unlock)
VALID_PLATFORMS = Bundler::CurrentRuby::PLATFORM_MAP.keys.freeze

VALID_KEYS = %w[group groups git path glob name branch ref tag require submodules
platform platforms type source install_if gemfile force_ruby_platform].freeze
platform platforms source install_if force_ruby_platform].freeze

GITHUB_PULL_REQUEST_URL = %r{\Ahttps://github\.com/([A-Za-z0-9_\-\.]+/[A-Za-z0-9_\-\.]+)/pull/(\d+)\z}
GITLAB_MERGE_REQUEST_URL = %r{\Ahttps://gitlab\.com/([A-Za-z0-9_\-\./]+)/-/merge_requests/(\d+)\z}
Expand Down Expand Up @@ -77,12 +77,12 @@ def gemspec(opts = nil)

@gemspecs << spec

gem spec.name, name: spec.name, path: path, glob: glob
path path, "glob" => glob, "name" => spec.name do
add_dependency spec.name
end

group(development_group) do
spec.development_dependencies.each do |dep|
gem dep.name, *(dep.requirement.as_list + [type: :development])
end
spec.development_dependencies.each do |dep|
add_dependency dep.name, dep.requirement.as_list, "type" => :development, "group" => development_group
end
when 0
raise InvalidOption, "There are no gemspecs at #{expanded_path}"
Expand All @@ -94,79 +94,11 @@ def gemspec(opts = nil)

def gem(name, *args)
options = args.last.is_a?(Hash) ? args.pop.dup : {}
options["gemfile"] = @gemfile
version = args || [">= 0"]

normalize_options(name, version, options)

dep = Dependency.new(name, version, options)

# if there's already a dependency with this name we try to prefer one
if current = @dependencies.find {|d| d.name == dep.name }
if current.requirement != dep.requirement
current_requirement_open = current.requirements_list.include?(">= 0")

gemspec_dep = [dep, current].find(&:gemspec_dev_dep?)
if gemspec_dep
gemfile_dep = [dep, current].find(&:runtime?)

if gemfile_dep && !current_requirement_open
Bundler.ui.warn "A gemspec development dependency (#{gemspec_dep.name}, #{gemspec_dep.requirement}) is being overridden by a Gemfile dependency (#{gemfile_dep.name}, #{gemfile_dep.requirement}).\n" \
"This behaviour may change in the future. Please remove either of them, or make sure they both have the same requirement\n"
elsif gemfile_dep.nil?
require_relative "vendor/pub_grub/lib/pub_grub/version_range"
require_relative "vendor/pub_grub/lib/pub_grub/version_constraint"
require_relative "vendor/pub_grub/lib/pub_grub/version_union"
require_relative "vendor/pub_grub/lib/pub_grub/rubygems"

current_gemspec_range = PubGrub::RubyGems.requirement_to_range(current.requirement)
next_gemspec_range = PubGrub::RubyGems.requirement_to_range(dep.requirement)

if current_gemspec_range.intersects?(next_gemspec_range)
dep = Dependency.new(name, current.requirement.as_list + dep.requirement.as_list, options)
else
raise GemfileError, "Two gemspecs have conflicting requirements on the same gem: #{dep} and #{current}"
end
end
else
update_prompt = ""

if File.basename(@gemfile) == Injector::INJECTED_GEMS
if dep.requirements_list.include?(">= 0") && !current_requirement_open
update_prompt = ". Gem already added"
else
update_prompt = ". If you want to update the gem version, run `bundle update #{current.name}`"

update_prompt += ". You may also need to change the version requirement specified in the Gemfile if it's too restrictive." unless current_requirement_open
end
end

raise GemfileError, "You cannot specify the same gem twice with different version requirements.\n" \
"You specified: #{current.name} (#{current.requirement}) and #{dep.name} (#{dep.requirement})" \
"#{update_prompt}"
end
end

unless current.gemspec_dev_dep? && dep.gemspec_dev_dep?
# Always prefer the dependency from the Gemfile
if current.gemspec_dev_dep?
@dependencies.delete(current)
elsif dep.gemspec_dev_dep?
return
elsif current.source != dep.source
raise GemfileError, "You cannot specify the same gem twice coming from different sources.\n" \
"You specified that #{dep.name} (#{dep.requirement}) should come from " \
"#{current.source || "an unspecified source"} and #{dep.source}\n"
else
Bundler.ui.warn "Your Gemfile lists the gem #{current.name} (#{current.requirement}) more than once.\n" \
"You should probably keep only one of them.\n" \
"Remove any duplicate entries and specify the gem only once.\n" \
"While it's not a problem now, it could cause errors if you change the version of one of them later."
end
end
end

@dependencies << dep
add_dependency(name, version, options)
end

def source(source, *args, &blk)
Expand Down Expand Up @@ -301,6 +233,81 @@ def check_primary_source_safety

private

def add_dependency(name, version = nil, options = {})
options["gemfile"] = @gemfile
options["source"] ||= @source
options["env"] ||= @env

dep = Dependency.new(name, version, options)

# if there's already a dependency with this name we try to prefer one
if current = @dependencies.find {|d| d.name == dep.name }
if current.requirement != dep.requirement
current_requirement_open = current.requirements_list.include?(">= 0")

gemspec_dep = [dep, current].find(&:gemspec_dev_dep?)
if gemspec_dep
gemfile_dep = [dep, current].find(&:runtime?)

if gemfile_dep && !current_requirement_open
Bundler.ui.warn "A gemspec development dependency (#{gemspec_dep.name}, #{gemspec_dep.requirement}) is being overridden by a Gemfile dependency (#{gemfile_dep.name}, #{gemfile_dep.requirement}).\n" \
"This behaviour may change in the future. Please remove either of them, or make sure they both have the same requirement\n"
elsif gemfile_dep.nil?
require_relative "vendor/pub_grub/lib/pub_grub/version_range"
require_relative "vendor/pub_grub/lib/pub_grub/version_constraint"
require_relative "vendor/pub_grub/lib/pub_grub/version_union"
require_relative "vendor/pub_grub/lib/pub_grub/rubygems"

current_gemspec_range = PubGrub::RubyGems.requirement_to_range(current.requirement)
next_gemspec_range = PubGrub::RubyGems.requirement_to_range(dep.requirement)

if current_gemspec_range.intersects?(next_gemspec_range)
dep = Dependency.new(name, current.requirement.as_list + dep.requirement.as_list, options)
else
raise GemfileError, "Two gemspecs have conflicting requirements on the same gem: #{dep} and #{current}"
end
end
else
update_prompt = ""

if File.basename(@gemfile) == Injector::INJECTED_GEMS
if dep.requirements_list.include?(">= 0") && !current_requirement_open
update_prompt = ". Gem already added"
else
update_prompt = ". If you want to update the gem version, run `bundle update #{current.name}`"

update_prompt += ". You may also need to change the version requirement specified in the Gemfile if it's too restrictive." unless current_requirement_open
end
end

raise GemfileError, "You cannot specify the same gem twice with different version requirements.\n" \
"You specified: #{current.name} (#{current.requirement}) and #{dep.name} (#{dep.requirement})" \
"#{update_prompt}"
end
end

unless current.gemspec_dev_dep? && dep.gemspec_dev_dep?
# Always prefer the dependency from the Gemfile
if current.gemspec_dev_dep?
@dependencies.delete(current)
elsif dep.gemspec_dev_dep?
return
elsif current.source != dep.source
raise GemfileError, "You cannot specify the same gem twice coming from different sources.\n" \
"You specified that #{dep.name} (#{dep.requirement}) should come from " \
"#{current.source || "an unspecified source"} and #{dep.source}\n"
else
Bundler.ui.warn "Your Gemfile lists the gem #{current.name} (#{current.requirement}) more than once.\n" \
"You should probably keep only one of them.\n" \
"Remove any duplicate entries and specify the gem only once.\n" \
"While it's not a problem now, it could cause errors if you change the version of one of them later."
end
end
end

@dependencies << dep
end

def with_gemfile(gemfile)
expanded_gemfile_path = Pathname.new(gemfile).expand_path(@gemfile&.parent)
original_gemfile = @gemfile
Expand Down Expand Up @@ -433,8 +440,6 @@ def normalize_options(name, version, opts)
opts["source"] = source
end

opts["source"] ||= @source
opts["env"] ||= @env
opts["platforms"] = platforms.dup
opts["group"] = groups
opts["should_include"] = install_if
Expand Down
38 changes: 29 additions & 9 deletions bundler/spec/install/gemfile_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,17 +53,37 @@
end
end

context "with deprecated features" do
it "reports that lib is an invalid option" do
gemfile <<-G
source "https://gem.repo1"
it "reports that lib is an invalid option" do
gemfile <<-G
source "https://gem.repo1"

gem "myrack", :lib => "myrack"
G
gem "myrack", :lib => "myrack"
G

bundle :install, raise_on_error: false
expect(err).to match(/You passed :lib as an option for gem 'myrack', but it is invalid/)
end
bundle :install, raise_on_error: false
expect(err).to match(/You passed :lib as an option for gem 'myrack', but it is invalid/)
end

it "reports that type is an invalid option" do
gemfile <<-G
source "https://gem.repo1"

gem "myrack", :type => "development"
G

bundle :install, raise_on_error: false
expect(err).to match(/You passed :type as an option for gem 'myrack', but it is invalid/)
end

it "reports that gemfile is an invalid option" do
gemfile <<-G
source "https://gem.repo1"

gem "myrack", :gemfile => "foo"
G

bundle :install, raise_on_error: false
expect(err).to match(/You passed :gemfile as an option for gem 'myrack', but it is invalid/)
end

context "when an internal error happens" do
Expand Down
Loading