Leave BUNDLE_GEMFILE unset in workers if it was unset in the master (#…

…2154) * Leave BUNDLE_GEMFILE unset in workers if it was unset in the master Co-Authored-By: Tim Morgan <tim@timmorgan.org> * Use Bundler.original_env instead of raw environment variables Co-authored-by: Tim Morgan <tim@timmorgan.org> Co-authored-by: Nate Berkopec <nate.berkopec@gmail.com>
puma · Mar 10, 2020 · c87f088 · c87f088
1 parent 7827a6c
commit c87f088
Show file tree

Hide file tree

Showing 9 changed files with 63 additions and 2 deletions.
diff --git a/History.md b/History.md
@@ -21,8 +21,9 @@
   * Preserve `BUNDLE_GEMFILE` env var when using `prune_bundler` (#1893)
   * Send 408 request timeout even when queue requests is disabled (#2119)
   * Rescue IO::WaitReadable instead of EAGAIN for blocking read (#2121)
+  * Ensure `BUNDLE_GEMFILE` is unspecified in workers if unspecified in master when using `prune_bundler` (#2154)
   * Rescue and log exceptions in hooks defined by users (on_worker_boot, after_worker_fork etc) (#1551)
-
+  
 * Refactor
   * Remove unused loader argument from Plugin initializer (#2095)
   * Simplify `Configuration.random_token` and remove insecure fallback (#2102)

diff --git a/lib/puma/launcher.rb b/lib/puma/launcher.rb
@@ -300,7 +300,7 @@ def prune_bundler
 
       log '* Pruning Bundler environment'
       home = ENV['GEM_HOME']
-      bundle_gemfile = ENV['BUNDLE_GEMFILE']
+      bundle_gemfile = Bundler.original_env['BUNDLE_GEMFILE']
       with_unbundled_env do
         ENV['GEM_HOME'] = home
         ENV['BUNDLE_GEMFILE'] = bundle_gemfile

diff --git a/test/bundle_preservation_test/version1/Gemfile b/test/bundle_preservation_test/version1/Gemfile
@@ -0,0 +1 @@
+gem 'puma', path: '../../..'
diff --git a/test/bundle_preservation_test/version1/config.ru b/test/bundle_preservation_test/version1/config.ru
@@ -0,0 +1 @@
+run lambda { |env| [200, {'Content-Type'=>'text/plain'}, [ENV['BUNDLE_GEMFILE'].inspect]] }
diff --git a/test/bundle_preservation_test/version1/config/puma.rb b/test/bundle_preservation_test/version1/config/puma.rb
@@ -0,0 +1 @@
+directory File.expand_path("../../current", __dir__)
diff --git a/test/bundle_preservation_test/version2/Gemfile b/test/bundle_preservation_test/version2/Gemfile
@@ -0,0 +1 @@
+gem 'puma', path: '../../..'
diff --git a/test/bundle_preservation_test/version2/config.ru b/test/bundle_preservation_test/version2/config.ru
@@ -0,0 +1 @@
+run lambda { |env| [200, {'Content-Type'=>'text/plain'}, [ENV['BUNDLE_GEMFILE'].inspect]] }
diff --git a/test/bundle_preservation_test/version2/config/puma.rb b/test/bundle_preservation_test/version2/config/puma.rb
@@ -0,0 +1 @@
+directory File.expand_path("../../current", __dir__)
diff --git a/test/test_preserve_bundler_env.rb b/test/test_preserve_bundler_env.rb
@@ -7,6 +7,11 @@ def setup
     super
   end
 
+  def teardown
+    FileUtils.rm current_release_symlink, force: true
+    super
+  end
+
   # It does not wipe out BUNDLE_GEMFILE et al
   def test_usr2_restart_preserves_bundler_environment
     skip_unless_signal_exist? :USR2
@@ -32,4 +37,53 @@ def test_usr2_restart_preserves_bundler_environment
     new_reply = read_body(connection)
     assert_match("Gemfile.bundle_env_preservation_test", new_reply)
   end
+
+  def test_phased_restart_preserves_unspecified_bundle_gemfile
+    skip_unless_signal_exist? :USR1
+
+    @tcp_port = UniquePort.call
+    env = {
+      "BUNDLE_GEMFILE" => nil,
+      "BUNDLER_ORIG_BUNDLE_GEMFILE" => nil
+    }
+    set_release_symlink File.expand_path("bundle_preservation_test/version1", __dir__)
+    cmd = "bundle exec puma -q -w 1 --prune-bundler -b tcp://#{HOST}:#{@tcp_port}"
+    Dir.chdir(current_release_symlink) do
+      @server = IO.popen(env, cmd.split, "r")
+    end
+    wait_for_server_to_boot
+    @pid = @server.pid
+    connection = connect
+
+    # Bundler itself sets ENV['BUNDLE_GEMFILE'] to the Gemfile it finds if ENV['BUNDLE_GEMFILE'] was unspecified
+    initial_reply = read_body(connection)
+    expected_gemfile = File.expand_path("bundle_preservation_test/version1/Gemfile", __dir__).inspect
+    assert_equal(expected_gemfile, initial_reply)
+
+    set_release_symlink File.expand_path("bundle_preservation_test/version2", __dir__)
+    start_phased_restart
+
+    connection = connect
+    connection.write "GET / HTTP/1.1\r\n\r\n"
+    new_reply = read_body(connection)
+    expected_gemfile = File.expand_path("bundle_preservation_test/version2/Gemfile", __dir__).inspect
+    assert_equal(expected_gemfile, new_reply)
+  end
+
+  private
+
+  def current_release_symlink
+    File.expand_path "bundle_preservation_test/current", __dir__
+  end
+
+  def set_release_symlink(target_dir)
+    FileUtils.rm current_release_symlink, force: true
+    FileUtils.symlink target_dir, current_release_symlink, force: true
+  end
+
+  def start_phased_restart
+    Process.kill :USR1, @pid
+
+    true while @server.gets !~ /booted, phase: 1/
+  end
 end