Merge pull request #8666 from Shopify/rwstauner/check-forces-dep-check

Validate lockfile dependencies with bundle install

Author: deivid-rodriguez

bundler/lib/bundler/cli/install.rb

@@ -66,7 +66,9 @@ def run
 
       Plugin.gemfile_install(Bundler.default_gemfile) if Bundler.feature_flag.plugins?
 
-      definition = Bundler.definition
+      # For install we want to enable strict validation
+      # (rather than some optimizations we perform at app runtime).
+      definition = Bundler.definition(strict: true)
       definition.validate_runtime!
 
       installer = Installer.install(Bundler.root, definition, options)

bundler/lib/bundler/definition.rb

@@ -60,6 +60,7 @@ def initialize(lockfile, dependencies, sources, unlock, ruby_version = nil, opti
 
       if unlock == true
         @unlocking_all = true
+        strict = false
         @unlocking_bundler = false
         @unlocking = unlock
         @sources_to_unlock = []
@@ -68,6 +69,7 @@ def initialize(lockfile, dependencies, sources, unlock, ruby_version = nil, opti
         conservative = false
       else
         @unlocking_all = false
+        strict = unlock.delete(:strict)
         @unlocking_bundler = unlock.delete(:bundler)
         @unlocking = unlock.any? {|_k, v| !Array(v).empty? }
         @sources_to_unlock = unlock.delete(:sources) || []
@@ -97,7 +99,7 @@ def initialize(lockfile, dependencies, sources, unlock, ruby_version = nil, opti
 
       if lockfile_exists?
         @lockfile_contents = Bundler.read_file(lockfile)
-        @locked_gems = LockfileParser.new(@lockfile_contents)
+        @locked_gems = LockfileParser.new(@lockfile_contents, strict: strict)
         @locked_platforms = @locked_gems.platforms
         @most_specific_locked_platform = @locked_gems.most_specific_locked_platform
         @platforms = @locked_platforms.dup

bundler/lib/bundler/lazy_specification.rb

@@ -33,7 +33,7 @@ def self.from_spec(s)
       lazy_spec
     end
 
-    def initialize(name, version, platform, source = nil)
+    def initialize(name, version, platform, source = nil, **materialization_options)
       @name          = name
       @version       = version
       @dependencies  = []
@@ -43,6 +43,7 @@ def initialize(name, version, platform, source = nil)
 
       @original_source = source
       @source = source
+      @materialization_options = materialization_options
 
       @force_ruby_platform = default_force_ruby_platform
       @most_specific_locked_platform = nil
@@ -226,12 +227,13 @@ def choose_compatible(candidates, fallback_to_non_installable: Bundler.frozen_bu
     # Validate dependencies of this locked spec are consistent with dependencies
     # of the actual spec that was materialized.
     #
-    # Note that we don't validate dependencies of locally installed gems but
+    # Note that unless we are in strict mode (which we set during installation)
+    # we don't validate dependencies of locally installed gems but
     # accept what's in the lockfile instead for performance, since loading
     # dependencies of locally installed gems would mean evaluating all gemspecs,
     # which would affect `bundler/setup` performance.
     def validate_dependencies(spec)
-      if spec.is_a?(StubSpecification)
+      if !@materialization_options[:strict] && spec.is_a?(StubSpecification)
         spec.dependencies = dependencies
       else
         if !source.is_a?(Source::Path) && spec.runtime_dependencies.sort != dependencies.sort

bundler/lib/bundler/lockfile_parser.rb

@@ -94,7 +94,7 @@ def self.bundled_with
       lockfile_contents.split(BUNDLED).last.strip
     end
 
-    def initialize(lockfile)
+    def initialize(lockfile, strict: false)
       @platforms    = []
       @sources      = []
       @dependencies = {}
@@ -106,6 +106,7 @@ def initialize(lockfile)
         "Gemfile.lock"
       end
       @pos = Position.new(1, 1)
+      @strict = strict
 
       if lockfile.match?(/<<<<<<<|=======|>>>>>>>|\|\|\|\|\|\|\|/)
         raise LockfileError, "Your #{@lockfile_path} contains merge conflicts.\n" \
@@ -286,7 +287,7 @@ def parse_spec(line)
 
         version = Gem::Version.new(version)
         platform = platform ? Gem::Platform.new(platform) : Gem::Platform::RUBY
-        @current_spec = LazySpecification.new(name, version, platform, @current_source)
+        @current_spec = LazySpecification.new(name, version, platform, @current_source, strict: @strict)
         @current_source.add_dependency_names(name)
 
         @specs[@current_spec.full_name] = @current_spec

bundler/spec/commands/install_spec.rb

@@ -1500,6 +1500,55 @@ def run
     end
   end
 
+  context "when lockfile has incorrect dependencies" do
+    before do
+      build_repo2
+
+      gemfile <<-G
+        source "https://gem.repo2"
+        gem "myrack_middleware"
+      G
+
+      system_gems "myrack_middleware-1.0", path: default_bundle_path
+
+      # we want to raise when the 1.0 line should be followed by "            myrack (= 0.9.1)" but isn't
+      lockfile <<-L
+        GEM
+          remote: https://gem.repo2/
+          specs:
+            myrack_middleware (1.0)
+
+        PLATFORMS
+          #{lockfile_platforms}
+
+        DEPENDENCIES
+          myrack_middleware
+
+        BUNDLED WITH
+          #{Bundler::VERSION}
+      L
+    end
+
+    it "raises a clear error message when frozen" do
+      bundle "config set frozen true"
+      bundle "install", raise_on_error: false
+
+      expect(exitstatus).to eq(41)
+      expect(err).to eq("Bundler found incorrect dependencies in the lockfile for myrack_middleware-1.0")
+    end
+
+    it "updates the lockfile when not frozen" do
+      missing_dep = "myrack (0.9.1)"
+      expect(lockfile).not_to include(missing_dep)
+
+      bundle "config set frozen false"
+      bundle :install
+
+      expect(lockfile).to include(missing_dep)
+      expect(out).to include("now installed")
+    end
+  end
+
   context "with --local flag" do
     before do
       system_gems "myrack-1.0.0", path: default_bundle_path