(MAINT) Make Bundler update_lock! helper more resilient

lib/pdk/cli/bundle.rb

@@ -20,13 +20,15 @@ module PDK::CLI
 
       PDK::CLI::Util.validate_puppet_version_opts({})
 
-      # Ensure that the bundled gems are up to date and correct Ruby is activated before running commend.
+      # Ensure that the correct Ruby is activated before running commend.
       puppet_env = PDK::CLI::Util.puppet_from_opts_or_env({})
       PDK::Util::RubyVersion.use(puppet_env[:ruby_version])
-      PDK::Util::Bundler.ensure_bundle!(puppet_env[:gemset])
+
+      gemfile_env = PDK::Util::Bundler::BundleHelper.gemfile_env(puppet_env[:gemset])
 
       command = PDK::CLI::Exec::Command.new(PDK::CLI::Exec.bundle_bin, *args).tap do |c|
         c.context = :module
+        c.update_environment(gemfile_env)
       end
 
       result = command.execute!

lib/pdk/util/bundler.rb

@@ -43,14 +43,10 @@ def self.ensure_bundle!(gem_overrides = nil)
           FileUtils.mv(temp_lockfile, original_lockfile, force: true)
         end
 
-        # Update puppet-related gem dependencies by re-resolving them specifically.
-        # If there are additional dependencies that aren't available locally, allow
-        # `bundle lock` to reach out to rubygems.org
-        bundle.update_lock!(gem_overrides, local: all_deps_available)
-
-        # If there were missing dependencies when we checked above, let `bundle install`
-        # go out and get them. For packaged installs, this should only be true if the user
-        # has added custom gems that we don't vendor.
+        bundle.update_lock!(with: gem_overrides, local: all_deps_available)
+
+        # If there are missing dependencies after updating the lockfile, let `bundle install`
+        # go out and get them.
         unless bundle.installed?
           bundle.install!(gem_overrides)
         end
@@ -151,24 +147,34 @@ def lock!
             # After initial lockfile generation, re-resolve json gem to built-in
             # version to avoid unncessary native compilation attempts. For packaged
             # installs this is done during the generation of the vendored Gemfile.lock
-            update_lock!({ json: nil }, local: true)
+            update_lock!(only: { json: nil }, local: true)
           end
 
           true
         end
 
-        def update_lock!(gem_overrides, options = {})
-          return true if gem_overrides.empty?
-
+        def update_lock!(options = {})
           PDK.logger.debug(_('Updating Gemfile dependencies.'))
 
-          update_gems = gem_overrides.keys.map(&:to_s)
+          argv = ['lock', '--update']
+
+          overrides = nil
+
+          if options && options[:only]
+            update_gems = options[:only].keys.map(&:to_s)
+            argv << update_gems
+            argv.flatten!
+
+            overrides = options[:only]
+          elsif options && options[:with]
+            overrides = options[:with]
+          end
 
-          argv = ['lock', '--update', update_gems].flatten
           argv << '--local' if options && options[:local]
+          argv << '--conservative' if options && options[:conservative]
 
           cmd = bundle_command(*argv).tap do |c|
-            c.update_environment(gemfile_env(gem_overrides))
+            c.update_environment(gemfile_env(overrides)) if overrides
           end
 
           result = cmd.execute!
@@ -215,19 +221,7 @@ def binstubs!(gems)
           true
         end
 
-        private
-
-        def bundle_command(*args)
-          PDK::CLI::Exec::Command.new(PDK::CLI::Exec.bundle_bin, *args).tap do |c|
-            c.context = :module
-          end
-        end
-
-        def bundle_cachedir
-          @bundle_cachedir ||= PDK::Util.package_install? ? PDK::Util.package_cachedir : File.join(PDK::Util.cachedir)
-        end
-
-        def gemfile_env(gem_overrides)
+        def self.gemfile_env(gem_overrides)
           gemfile_env = {}
 
           return gemfile_env unless gem_overrides.respond_to?(:each)
@@ -240,6 +234,22 @@ def gemfile_env(gem_overrides)
 
           gemfile_env
         end
+
+        private
+
+        def gemfile_env(gem_overrides)
+          self.class.gemfile_env(gem_overrides)
+        end
+
+        def bundle_command(*args)
+          PDK::CLI::Exec::Command.new(PDK::CLI::Exec.bundle_bin, *args).tap do |c|
+            c.context = :module
+          end
+        end
+
+        def bundle_cachedir
+          @bundle_cachedir ||= PDK::Util.package_install? ? PDK::Util.package_cachedir : File.join(PDK::Util.cachedir)
+        end
       end
     end
   end

spec/unit/pdk/util/bundler_spec.rb

@@ -65,7 +65,7 @@
         end
 
         it 'updates Gemfile.lock using default sources' do
-          expect(bundle_helper).to receive(:update_lock!).with(anything, hash_including(local: true))
+          expect(bundle_helper).to receive(:update_lock!).with(hash_including(local: true))
 
           described_class.ensure_bundle!
         end
@@ -74,7 +74,7 @@
           include_context 'packaged install'
 
           it 'updates Gemfile.lock using local gems' do
-            expect(bundle_helper).to receive(:update_lock!).with(anything, hash_including(local: true))
+            expect(bundle_helper).to receive(:update_lock!).with(hash_including(local: true))
 
             described_class.ensure_bundle!
           end
@@ -426,7 +426,7 @@ def bundle_regex
       it 'invokes #update_lock! to re-resolve json dependency locally' do
         allow_command([bundle_regex, 'lock'], exit_code: 0)
 
-        expect(instance).to receive(:update_lock!).with(hash_including(:json), hash_including(local: true)).and_return(true)
+        expect(instance).to receive(:update_lock!).with(hash_including(only: hash_including(:json), local: true)).and_return(true)
 
         instance.lock!
       end
@@ -491,57 +491,67 @@ def bundle_regex
       let(:overridden_gems) { overrides.keys.map(&:to_s) }
 
       it 'updates env before invoking `bundle lock --update`' do
-        cmd_double = allow_command([bundle_regex, 'lock', '--update', overridden_gems].flatten, exit_code: 0)
+        cmd_double = allow_command([bundle_regex, 'lock', '--update'], exit_code: 0)
 
         expect(cmd_double).to receive(:update_environment).with(hash_including('PUPPET_GEM_VERSION' => '1.2.3'))
 
-        instance.update_lock!(overrides)
+        instance.update_lock!(with: overrides)
       end
 
       it 'invokes `bundle lock --update`' do
-        expect_command([bundle_regex, 'lock', '--update', overridden_gems].flatten, exit_code: 0)
+        expect_command([bundle_regex, 'lock', '--update'], exit_code: 0)
 
-        instance.update_lock!(overrides)
+        instance.update_lock!(with: overrides)
       end
 
       context 'when `bundle lock --update` exits non-zero' do
         before(:each) do
-          allow_command([bundle_regex, 'lock', '--update', overridden_gems].flatten, exit_code: 1, stderr: 'bundle lock update error message')
+          allow_command([bundle_regex, 'lock', '--update'], exit_code: 1, stderr: 'bundle lock update error message')
         end
 
         it 'logs a fatal message with output and raises FatalError' do
           expect(logger).to receive(:fatal).with(%r{bundle lock update error message}i)
-          expect { instance.update_lock!(overrides) }.to raise_error(PDK::CLI::FatalError, %r{unable to resolve}i)
+          expect { instance.update_lock!(with: overrides) }.to raise_error(PDK::CLI::FatalError, %r{unable to resolve}i)
         end
       end
 
       context 'with multiple overrides' do
         let(:overrides) { { puppet: '1.2.3', facter: '2.3.4' } }
+        let(:expected_environment) do
+          {
+            'PUPPET_GEM_VERSION' => '1.2.3',
+            'FACTER_GEM_VERSION' => '2.3.4',
+          }
+        end
+
+        it 'includes all gem overrides in the command environment' do
+          cmd_double = allow_command([bundle_regex, 'lock', '--update'], exit_code: 0)
 
-        it 'includes all gem names in `bundle lock --update` invocation' do
-          expect_command([bundle_regex, 'lock', '--update', overridden_gems].flatten, exit_code: 0)
+          expect(cmd_double).to receive(:update_environment).with(hash_including(expected_environment))
 
-          instance.update_lock!(overrides)
+          instance.update_lock!(with: overrides)
         end
       end
 
       context 'with local option set' do
         let(:options) { { local: true } }
 
         it 'includes \'--local\' in `bundle lock --update` invocation' do
-          expect_command([bundle_regex, 'lock', '--update', overridden_gems, '--local'].flatten, exit_code: 0)
+          expect_command([bundle_regex, 'lock', '--update', '--local'], exit_code: 0)
 
-          instance.update_lock!(overrides, options)
+          instance.update_lock!(options.merge(with: overrides))
         end
       end
 
       context 'with no overrides' do
         let(:overrides) { {} }
 
-        it 'returns true early' do
-          expect(PDK::CLI::Exec::Command).not_to receive(:new)
+        it 'does not update the command environment' do
+          cmd_double = allow_command([bundle_regex, 'lock', '--update'], exit_code: 0)
+
+          expect(cmd_double).to receive(:update_environment).with({})
 
-          expect(instance.update_lock!(overrides)).to be true
+          instance.update_lock!(with: overrides)
         end
       end
     end