diff --git a/lib/bundler/cli/add.rb b/lib/bundler/cli/add.rb index 12a681a816886c..9f1760409617d2 100644 --- a/lib/bundler/cli/add.rb +++ b/lib/bundler/cli/add.rb @@ -36,6 +36,16 @@ def inject_dependencies end def validate_options! + raise InvalidOption, "You cannot specify `--git` and `--github` at the same time." if options["git"] && options["github"] + + unless options["git"] || options["github"] + raise InvalidOption, "You cannot specify `--branch` unless `--git` or `--github` is specified." if options["branch"] + + raise InvalidOption, "You cannot specify `--ref` unless `--git` or `--github` is specified." if options["ref"] + end + + raise InvalidOption, "You cannot specify `--branch` and `--ref` at the same time." if options["branch"] && options["ref"] + raise InvalidOption, "You cannot specify `--strict` and `--optimistic` at the same time." if options[:strict] && options[:optimistic] # raise error when no gems are specified diff --git a/lib/bundler/definition.rb b/lib/bundler/definition.rb index 2fa7d0d277943b..5ab577f504c39d 100644 --- a/lib/bundler/definition.rb +++ b/lib/bundler/definition.rb @@ -1066,7 +1066,22 @@ def converge_specs(specs) deps << dep if !replacement_source || lockfile_source.include?(replacement_source) || new_deps.include?(dep) else - replacement_source = sources.get(lockfile_source) + parent_dep = @dependencies.find do |d| + next unless d.source && d.source != lockfile_source + next if d.source.is_a?(Source::Gemspec) + + parent_locked_specs = @originally_locked_specs[d.name] + + parent_locked_specs.any? do |parent_spec| + parent_spec.runtime_dependencies.any? {|rd| rd.name == s.name } + end + end + + if parent_dep + replacement_source = parent_dep.source + else + replacement_source = sources.get(lockfile_source) + end end # Replace the locked dependency's source with the equivalent source from the Gemfile diff --git a/lib/bundler/errors.rb b/lib/bundler/errors.rb index 8cd1336356079e..d8df4d6ec5c553 100644 --- a/lib/bundler/errors.rb +++ b/lib/bundler/errors.rb @@ -131,7 +131,8 @@ class GemRequireError < BundlerError attr_reader :orig_exception def initialize(orig_exception, msg) - full_message = msg + "\nGem Load Error is: #{orig_exception.message}\n"\ + full_message = msg + "\nGem Load Error is: + #{orig_exception.full_message(highlight: false)}\n"\ "Backtrace for gem load error is:\n"\ "#{orig_exception.backtrace.join("\n")}\n"\ "Bundler Error Backtrace:\n" @@ -221,7 +222,9 @@ def initialize(underlying_error, message) class DirectoryRemovalError < BundlerError def initialize(orig_exception, msg) full_message = "#{msg}.\n" \ - "The underlying error was #{orig_exception.class}: #{orig_exception.message}, with backtrace:\n" \ + "The underlying error was #{orig_exception.class}: + #{orig_exception.full_message(highlight: false)}, + with backtrace:\n" \ " #{orig_exception.backtrace.join("\n ")}\n\n" \ "Bundler Error Backtrace:" super(full_message) diff --git a/spec/bundler/commands/add_spec.rb b/spec/bundler/commands/add_spec.rb index 00aa6415e11c97..ed98a914f326c4 100644 --- a/spec/bundler/commands/add_spec.rb +++ b/spec/bundler/commands/add_spec.rb @@ -236,6 +236,40 @@ end end + describe "with mismatched pair in --git/--github, --branch/--ref" do + describe "with --git and --github" do + it "throws error" do + bundle "add 'foo' --git x --github y", raise_on_error: false + + expect(err).to include("You cannot specify `--git` and `--github` at the same time.") + end + end + + describe "with --branch and --ref with --git" do + it "throws error" do + bundle "add 'foo' --branch x --ref y --git file://git", raise_on_error: false + + expect(err).to include("You cannot specify `--branch` and `--ref` at the same time.") + end + end + + describe "with --branch but without --git or --github" do + it "throws error" do + bundle "add 'foo' --branch x", raise_on_error: false + + expect(err).to include("You cannot specify `--branch` unless `--git` or `--github` is specified.") + end + end + + describe "with --ref but without --git or --github" do + it "throws error" do + bundle "add 'foo' --ref y", raise_on_error: false + + expect(err).to include("You cannot specify `--ref` unless `--git` or `--github` is specified.") + end + end + end + describe "with --skip-install" do it "adds gem to Gemfile but is not installed" do bundle "add foo --skip-install --version=2.0" diff --git a/spec/bundler/install/gemfile/sources_spec.rb b/spec/bundler/install/gemfile/sources_spec.rb index c0b4d98f1c5cc6..90f87ed0c5daea 100644 --- a/spec/bundler/install/gemfile/sources_spec.rb +++ b/spec/bundler/install/gemfile/sources_spec.rb @@ -1079,4 +1079,120 @@ expect(lockfile).to eq original_lockfile.gsub("bigdecimal (1.0.0)", "bigdecimal (3.3.1)") end end + + context "when switching a gem with components from rubygems to git source" do + before do + build_repo2 do + build_gem "rails", "7.0.0" do |s| + s.add_dependency "actionpack", "7.0.0" + s.add_dependency "activerecord", "7.0.0" + end + build_gem "actionpack", "7.0.0" + build_gem "activerecord", "7.0.0" + # propshaft also depends on actionpack, creating the conflict + build_gem "propshaft", "1.0.0" do |s| + s.add_dependency "actionpack", ">= 7.0.0" + end + end + + build_git "rails", "7.0.0", path: lib_path("rails") do |s| + s.add_dependency "actionpack", "7.0.0" + s.add_dependency "activerecord", "7.0.0" + end + + build_git "actionpack", "7.0.0", path: lib_path("rails") + build_git "activerecord", "7.0.0", path: lib_path("rails") + + install_gemfile <<-G + source "https://gem.repo2" + gem "rails", "7.0.0" + gem "propshaft" + G + end + + it "moves component gems to the git source in the lockfile" do + expect(lockfile).to include("remote: https://gem.repo2") + expect(lockfile).to include("rails (7.0.0)") + expect(lockfile).to include("actionpack (7.0.0)") + expect(lockfile).to include("activerecord (7.0.0)") + expect(lockfile).to include("propshaft (1.0.0)") + + gemfile <<-G + source "https://gem.repo2" + gem "rails", git: "#{lib_path("rails")}" + gem "propshaft" + G + + bundle "install" + + expect(lockfile).to include("remote: #{lib_path("rails")}") + expect(lockfile).to include("rails (7.0.0)") + expect(lockfile).to include("actionpack (7.0.0)") + expect(lockfile).to include("activerecord (7.0.0)") + + # Component gems should NOT remain in the GEM section + # Extract just the GEM section by splitting on GIT first, then GEM + gem_section = lockfile.split("GEM\n").last.split(/\n(PLATFORMS|DEPENDENCIES)/)[0] + expect(gem_section).not_to include("actionpack (7.0.0)") + expect(gem_section).not_to include("activerecord (7.0.0)") + end + end + + context "when switching a gem with components from rubygems to path source" do + before do + build_repo2 do + build_gem "rails", "7.0.0" do |s| + s.add_dependency "actionpack", "7.0.0" + s.add_dependency "activerecord", "7.0.0" + end + build_gem "actionpack", "7.0.0" + build_gem "activerecord", "7.0.0" + # propshaft also depends on actionpack, creating the conflict + build_gem "propshaft", "1.0.0" do |s| + s.add_dependency "actionpack", ">= 7.0.0" + end + end + + build_lib "rails", "7.0.0", path: lib_path("rails") do |s| + s.add_dependency "actionpack", "7.0.0" + s.add_dependency "activerecord", "7.0.0" + end + + build_lib "actionpack", "7.0.0", path: lib_path("rails") + build_lib "activerecord", "7.0.0", path: lib_path("rails") + + install_gemfile <<-G + source "https://gem.repo2" + gem "rails", "7.0.0" + gem "propshaft" + G + end + + it "moves component gems to the path source in the lockfile" do + expect(lockfile).to include("remote: https://gem.repo2") + expect(lockfile).to include("rails (7.0.0)") + expect(lockfile).to include("actionpack (7.0.0)") + expect(lockfile).to include("activerecord (7.0.0)") + expect(lockfile).to include("propshaft (1.0.0)") + + gemfile <<-G + source "https://gem.repo2" + gem "rails", path: "#{lib_path("rails")}" + gem "propshaft" + G + + bundle "install" + + expect(lockfile).to include("remote: #{lib_path("rails")}") + expect(lockfile).to include("rails (7.0.0)") + expect(lockfile).to include("actionpack (7.0.0)") + expect(lockfile).to include("activerecord (7.0.0)") + + # Component gems should NOT remain in the GEM section + # Extract just the GEM section by splitting appropriately + gem_section = lockfile.split("GEM\n").last.split(/\n(PLATFORMS|DEPENDENCIES)/)[0] + expect(gem_section).not_to include("actionpack (7.0.0)") + expect(gem_section).not_to include("activerecord (7.0.0)") + end + end end