From c87f088396a5f1d502f0dfc402d2160be8d54886 Mon Sep 17 00:00:00 2001 From: Chris LaRose Date: Tue, 10 Mar 2020 10:46:20 -0700 Subject: [PATCH] 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 * Use Bundler.original_env instead of raw environment variables Co-authored-by: Tim Morgan Co-authored-by: Nate Berkopec --- History.md | 3 +- lib/puma/launcher.rb | 2 +- .../bundle_preservation_test/version1/Gemfile | 1 + .../version1/config.ru | 1 + .../version1/config/puma.rb | 1 + .../bundle_preservation_test/version2/Gemfile | 1 + .../version2/config.ru | 1 + .../version2/config/puma.rb | 1 + test/test_preserve_bundler_env.rb | 54 +++++++++++++++++++ 9 files changed, 63 insertions(+), 2 deletions(-) create mode 100644 test/bundle_preservation_test/version1/Gemfile create mode 100644 test/bundle_preservation_test/version1/config.ru create mode 100644 test/bundle_preservation_test/version1/config/puma.rb create mode 100644 test/bundle_preservation_test/version2/Gemfile create mode 100644 test/bundle_preservation_test/version2/config.ru create mode 100644 test/bundle_preservation_test/version2/config/puma.rb diff --git a/History.md b/History.md index 49201d210d..7d5d569e00 100644 --- 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 index acff0f3d70..20a34c1a4e 100644 --- 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 new file mode 100644 index 0000000000..e1437de3f2 --- /dev/null +++ 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 new file mode 100644 index 0000000000..1f0f2cc619 --- /dev/null +++ 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 new file mode 100644 index 0000000000..ab019ff138 --- /dev/null +++ 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 new file mode 100644 index 0000000000..e1437de3f2 --- /dev/null +++ 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 new file mode 100644 index 0000000000..1f0f2cc619 --- /dev/null +++ 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 new file mode 100644 index 0000000000..ab019ff138 --- /dev/null +++ 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 index 1dd1b43447..6f082f3099 100644 --- 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