From 56bbc2dc3a146502b4910df46656733badf9e982 Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Wed, 7 Aug 2019 17:37:25 -0700 Subject: [PATCH 01/28] Preserve BUNDLE_GEMFILE and add a test for it Starting your server with BUNDLE_GEMFILE=Gemfile.rails6 would delete this environment variable on app restart when using `--prune-bundler`, when all you really want is to restore the variable to what it was before Bundler was activated. I mimicked the existing code that preserves GEM_HOME. Co-authored-by: James Miller --- .gitignore | 2 ++ lib/puma/launcher.rb | 2 ++ test/rackup/hello-bundler-env.ru | 1 + test/test_integration.rb | 20 ++++++++++++++++---- 4 files changed, 21 insertions(+), 4 deletions(-) create mode 100644 test/rackup/hello-bundler-env.ru diff --git a/.gitignore b/.gitignore index 1afc74a409..2e31a6cb8c 100644 --- a/.gitignore +++ b/.gitignore @@ -12,6 +12,8 @@ tmp t/ .rbx/ Gemfile.lock +Gemfile.duplicate +Gemfile.duplicate.lock .idea/ /test/test_puma.state /test/test_server.sock diff --git a/lib/puma/launcher.rb b/lib/puma/launcher.rb index 1f275f6930..3622cf8f73 100644 --- a/lib/puma/launcher.rb +++ b/lib/puma/launcher.rb @@ -277,8 +277,10 @@ def prune_bundler log '* Pruning Bundler environment' home = ENV['GEM_HOME'] + gemfile = ENV['BUNDLE_GEMFILE'] Bundler.with_clean_env do ENV['GEM_HOME'] = home + ENV['BUNDLE_GEMFILE'] = gemfile ENV['PUMA_BUNDLER_PRUNED'] = '1' wild = File.expand_path(File.join(puma_lib_dir, "../bin/puma-wild")) args = [Gem.ruby, wild, '-I', dirs.join(':'), deps.join(',')] + @original_argv diff --git a/test/rackup/hello-bundler-env.ru b/test/rackup/hello-bundler-env.ru new file mode 100644 index 0000000000..bce7f344c1 --- /dev/null +++ b/test/rackup/hello-bundler-env.ru @@ -0,0 +1 @@ +run lambda { |env| [200, {"Content-Type" => "text/plain"}, ["Hello BUNDLE_GEMFILE #{ENV["BUNDLE_GEMFILE"]}"]] } diff --git a/test/test_integration.rb b/test/test_integration.rb index d886c323a1..8a565f7435 100644 --- a/test/test_integration.rb +++ b/test/test_integration.rb @@ -51,8 +51,8 @@ def server_cmd(argv) "#{base} -b tcp://127.0.0.1:#{@tcp_port} #{argv}" end - def server(argv) - @server = IO.popen(server_cmd(argv), "r") + def server(argv, env = {}) + @server = IO.popen(env, server_cmd(argv), "r") wait_for_server_to_boot(@server) @@ -75,8 +75,8 @@ def stop_forked_server(pid) Process.wait2(pid) end - def restart_server_and_listen(argv) - server(argv) + def restart_server_and_listen(argv, env = {}) + server(argv, env) connection = connect initial_reply = read_body(connection) restart_server(@server, connection) @@ -239,6 +239,18 @@ def test_restart_closes_keepalive_sockets_workers assert_equal "Hello World", new_reply end + def test_restart_with_prune_bundler_keeps_bundle_gemfile_env + gemfile_path = File.expand_path("../Gemfile", __dir__) + duplicate_gemfile_path = gemfile_path + ".duplicate" + FileUtils.cp(gemfile_path, gemfile_path + ".duplicate") + initial_reply, new_reply = restart_server_and_listen("-q -w 2 --prune-bundler test/rackup/hello-bundler-env.ru", "BUNDLE_GEMFILE" => "Gemfile.duplicate") + assert_match(/Hello BUNDLE_GEMFILE.*Gemfile\.duplicate/, initial_reply) + assert_match(/Hello BUNDLE_GEMFILE.*Gemfile\.duplicate/, new_reply) + ensure + FileUtils.rm_f(duplicate_gemfile_path) + FileUtils.rm_f(duplicate_gemfile_path + '.lock') + end + def test_sigterm_closes_listeners_on_forked_servers skip NO_FORK_MSG unless HAS_FORK pid = start_forked_server("-w 2 -q test/rackup/1second.ru") From a3ed99785528fd5ce8935a29059d715d16443e87 Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Thu, 8 Aug 2019 10:41:57 -0700 Subject: [PATCH 02/28] Go back to using Bundler.with_original_env Test that the environment is not lost when doing a restart by using the PATH env var as a canary. While we actually care about the BUNDLER_GEMFILE env var, that one is harder to set up in testing since it requires a full copy of Puma's Gemfile. This change also has the benefit of not only protecting the BUNDLE_GEMFILE var, but also the other variables that Bundler knows about. --- lib/puma/launcher.rb | 4 +--- test/rackup/hello-bundler-env.ru | 2 +- test/test_integration.rb | 12 +++--------- 3 files changed, 5 insertions(+), 13 deletions(-) diff --git a/lib/puma/launcher.rb b/lib/puma/launcher.rb index 3622cf8f73..da0214b8af 100644 --- a/lib/puma/launcher.rb +++ b/lib/puma/launcher.rb @@ -277,10 +277,8 @@ def prune_bundler log '* Pruning Bundler environment' home = ENV['GEM_HOME'] - gemfile = ENV['BUNDLE_GEMFILE'] - Bundler.with_clean_env do + Bundler.with_original_env do ENV['GEM_HOME'] = home - ENV['BUNDLE_GEMFILE'] = gemfile ENV['PUMA_BUNDLER_PRUNED'] = '1' wild = File.expand_path(File.join(puma_lib_dir, "../bin/puma-wild")) args = [Gem.ruby, wild, '-I', dirs.join(':'), deps.join(',')] + @original_argv diff --git a/test/rackup/hello-bundler-env.ru b/test/rackup/hello-bundler-env.ru index bce7f344c1..794d324f69 100644 --- a/test/rackup/hello-bundler-env.ru +++ b/test/rackup/hello-bundler-env.ru @@ -1 +1 @@ -run lambda { |env| [200, {"Content-Type" => "text/plain"}, ["Hello BUNDLE_GEMFILE #{ENV["BUNDLE_GEMFILE"]}"]] } +run lambda { |env| [200, {"Content-Type" => "text/plain"}, ["Hello PATH #{ENV["PATH"]}"]] } diff --git a/test/test_integration.rb b/test/test_integration.rb index 8a565f7435..65b30e6c88 100644 --- a/test/test_integration.rb +++ b/test/test_integration.rb @@ -240,15 +240,9 @@ def test_restart_closes_keepalive_sockets_workers end def test_restart_with_prune_bundler_keeps_bundle_gemfile_env - gemfile_path = File.expand_path("../Gemfile", __dir__) - duplicate_gemfile_path = gemfile_path + ".duplicate" - FileUtils.cp(gemfile_path, gemfile_path + ".duplicate") - initial_reply, new_reply = restart_server_and_listen("-q -w 2 --prune-bundler test/rackup/hello-bundler-env.ru", "BUNDLE_GEMFILE" => "Gemfile.duplicate") - assert_match(/Hello BUNDLE_GEMFILE.*Gemfile\.duplicate/, initial_reply) - assert_match(/Hello BUNDLE_GEMFILE.*Gemfile\.duplicate/, new_reply) - ensure - FileUtils.rm_f(duplicate_gemfile_path) - FileUtils.rm_f(duplicate_gemfile_path + '.lock') + initial_reply, new_reply = restart_server_and_listen("-q -w 2 --prune-bundler test/rackup/hello-bundler-env.ru", "PATH" => "hello-bundler:#{ENV["PATH"]}", "BUNDLER_ORIG_PATH" => nil) + assert_match(/Hello PATH.*hello-bundler/, initial_reply) + assert_match(/Hello PATH.*hello-bundler/, new_reply) end def test_sigterm_closes_listeners_on_forked_servers From 4b27762e73e946ad93ba7ee08d54635be24faf2d Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Thu, 8 Aug 2019 10:45:32 -0700 Subject: [PATCH 03/28] Gemfile.duplicate is no longer a thing --- .gitignore | 2 -- 1 file changed, 2 deletions(-) diff --git a/.gitignore b/.gitignore index 2e31a6cb8c..1afc74a409 100644 --- a/.gitignore +++ b/.gitignore @@ -12,8 +12,6 @@ tmp t/ .rbx/ Gemfile.lock -Gemfile.duplicate -Gemfile.duplicate.lock .idea/ /test/test_puma.state /test/test_server.sock From dad3e584c2312689803c9c1b8cb385d4187f9eb7 Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Thu, 8 Aug 2019 10:49:05 -0700 Subject: [PATCH 04/28] Rename test --- test/test_integration.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_integration.rb b/test/test_integration.rb index 65b30e6c88..b978520d5e 100644 --- a/test/test_integration.rb +++ b/test/test_integration.rb @@ -239,7 +239,7 @@ def test_restart_closes_keepalive_sockets_workers assert_equal "Hello World", new_reply end - def test_restart_with_prune_bundler_keeps_bundle_gemfile_env + def test_restart_with_prune_bundler_keeps_bundler_env initial_reply, new_reply = restart_server_and_listen("-q -w 2 --prune-bundler test/rackup/hello-bundler-env.ru", "PATH" => "hello-bundler:#{ENV["PATH"]}", "BUNDLER_ORIG_PATH" => nil) assert_match(/Hello PATH.*hello-bundler/, initial_reply) assert_match(/Hello PATH.*hello-bundler/, new_reply) From 9c2dd280c13d76a0a760b7079829f6516cba5ab0 Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Sun, 18 Aug 2019 21:40:42 -0500 Subject: [PATCH 05/28] Skip this test on Windows --- test/test_integration.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/test/test_integration.rb b/test/test_integration.rb index b978520d5e..2f0c85f19d 100644 --- a/test/test_integration.rb +++ b/test/test_integration.rb @@ -240,6 +240,7 @@ def test_restart_closes_keepalive_sockets_workers end def test_restart_with_prune_bundler_keeps_bundler_env + skip NO_FORK_MSG unless HAS_FORK initial_reply, new_reply = restart_server_and_listen("-q -w 2 --prune-bundler test/rackup/hello-bundler-env.ru", "PATH" => "hello-bundler:#{ENV["PATH"]}", "BUNDLER_ORIG_PATH" => nil) assert_match(/Hello PATH.*hello-bundler/, initial_reply) assert_match(/Hello PATH.*hello-bundler/, new_reply) From 5e7a6fd46ad26771d18245792b0bda3bc87c0574 Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Mon, 19 Aug 2019 09:26:46 -0500 Subject: [PATCH 06/28] No need to fork --- test/test_integration.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/test_integration.rb b/test/test_integration.rb index 2f0c85f19d..b2fc3d62f8 100644 --- a/test/test_integration.rb +++ b/test/test_integration.rb @@ -240,8 +240,7 @@ def test_restart_closes_keepalive_sockets_workers end def test_restart_with_prune_bundler_keeps_bundler_env - skip NO_FORK_MSG unless HAS_FORK - initial_reply, new_reply = restart_server_and_listen("-q -w 2 --prune-bundler test/rackup/hello-bundler-env.ru", "PATH" => "hello-bundler:#{ENV["PATH"]}", "BUNDLER_ORIG_PATH" => nil) + initial_reply, new_reply = restart_server_and_listen("-q --prune-bundler test/rackup/hello-bundler-env.ru", "PATH" => "hello-bundler:#{ENV["PATH"]}", "BUNDLER_ORIG_PATH" => nil) assert_match(/Hello PATH.*hello-bundler/, initial_reply) assert_match(/Hello PATH.*hello-bundler/, new_reply) end From 4cfd3b83027dceb92142d19ea6d5a5f69f72e514 Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Wed, 12 Feb 2020 15:37:10 -0600 Subject: [PATCH 07/28] Preserve BUNDLE_GEMFILE and other Bundler env vars Starting your server with BUNDLE_GEMFILE=Gemfile.rails6 would delete this environment variable on app restart when using --prune-bundler, when all you really want is to restore the variable to what it was before Bundler was activated. Writing a test for this proved to be difficult because the bug only seems to manifest itself when running the server under the following conditions: - Using `bundle exec` to run the puma command - Using a non-zero number of workers, e.g. `-w 2` - Setting `BUNDLE_GEMFILE` to something other than "Gemfile" --- test/bundle_preservation_test/.gitignore | 1 + .../Gemfile.bundle_env_preservation_test | 1 + test/bundle_preservation_test/config.ru | 1 + test/test_integration_cluster.rb | 26 +++++++++++++++++++ 4 files changed, 29 insertions(+) create mode 100644 test/bundle_preservation_test/.gitignore create mode 100644 test/bundle_preservation_test/Gemfile.bundle_env_preservation_test create mode 100644 test/bundle_preservation_test/config.ru diff --git a/test/bundle_preservation_test/.gitignore b/test/bundle_preservation_test/.gitignore new file mode 100644 index 0000000000..53c6471352 --- /dev/null +++ b/test/bundle_preservation_test/.gitignore @@ -0,0 +1 @@ +Gemfile.bundle_env_preservation_test.lock diff --git a/test/bundle_preservation_test/Gemfile.bundle_env_preservation_test b/test/bundle_preservation_test/Gemfile.bundle_env_preservation_test new file mode 100644 index 0000000000..e0695ee62c --- /dev/null +++ b/test/bundle_preservation_test/Gemfile.bundle_env_preservation_test @@ -0,0 +1 @@ +gem 'puma', path: '../..' diff --git a/test/bundle_preservation_test/config.ru b/test/bundle_preservation_test/config.ru new file mode 100644 index 0000000000..1f0f2cc619 --- /dev/null +++ b/test/bundle_preservation_test/config.ru @@ -0,0 +1 @@ +run lambda { |env| [200, {'Content-Type'=>'text/plain'}, [ENV['BUNDLE_GEMFILE'].inspect]] } diff --git a/test/test_integration_cluster.rb b/test/test_integration_cluster.rb index a7f58f84ec..559ca354fb 100644 --- a/test/test_integration_cluster.rb +++ b/test/test_integration_cluster.rb @@ -133,6 +133,32 @@ def test_stuck_phased_restart worker_respawn { |phase0_worker_pids| Process.kill :USR1, @pid } end + # It does not wipe out BUNDLE_GEMFILE et al + def test_usr2_restart_preserves_bundler_environment + skip_unless_signal_exist? :USR2 + + @tcp_port = UniquePort.call + env = { + # Intentionally set this to something we wish to keep intact on restarts + "BUNDLE_GEMFILE" => "Gemfile.bundle_env_preservation_test", + # Don't allow our (rake test's) original env to interfere with the child process + "BUNDLER_ORIG_BUNDLE_GEMFILE" => nil + } + # Must use `bundle exec puma` here, because otherwise Bundler may not be defined, which is required to trigger the bug + cmd = "bundle exec puma -q -w 1 --prune-bundler -b tcp://#{HOST}:#{@tcp_port}" + Dir.chdir(File.expand_path("bundle_preservation_test", __dir__)) do + @server = IO.popen(env, cmd, "r") + end + wait_for_server_to_boot + @pid = @server.pid + connection = connect + initial_reply = read_body(connection) + assert_match("Gemfile.bundle_env_preservation_test", initial_reply) + restart_server connection + new_reply = read_body(connection) + assert_match("Gemfile.bundle_env_preservation_test", new_reply) + end + private # Send requests 10 per second. Send 10, then :TERM server, then send another 30. From 242c1401bc5d03024aba1e05d1a0b21b9801b56c Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Wed, 12 Feb 2020 16:44:06 -0600 Subject: [PATCH 08/28] Remove unneeded test support file --- test/rackup/hello-bundler-env.ru | 1 - 1 file changed, 1 deletion(-) delete mode 100644 test/rackup/hello-bundler-env.ru diff --git a/test/rackup/hello-bundler-env.ru b/test/rackup/hello-bundler-env.ru deleted file mode 100644 index 794d324f69..0000000000 --- a/test/rackup/hello-bundler-env.ru +++ /dev/null @@ -1 +0,0 @@ -run lambda { |env| [200, {"Content-Type" => "text/plain"}, ["Hello PATH #{ENV["PATH"]}"]] } From f15121f1006b6e407a2fd52ec4d445e37c2df9c4 Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Wed, 12 Feb 2020 16:44:26 -0600 Subject: [PATCH 09/28] Update changelog --- History.md | 1 + 1 file changed, 1 insertion(+) diff --git a/History.md b/History.md index 91d737993f..ff3e5969e1 100644 --- a/History.md +++ b/History.md @@ -10,6 +10,7 @@ * Bugfixes * Your bugfix goes here (#Github Number) * Windows update extconf.rb for use with ssp and varied Ruby/MSYS2 combinations (#2069) + * Preserve `BUNDLE_GEMFILE` and other Bundler env vars on restart (#1893) * Refactor * Remove unused loader argument from Plugin initializer (#2095) From 302031627a4c7cbf34a30ede1ee252aed593cfd2 Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Thu, 13 Feb 2020 11:33:01 -0600 Subject: [PATCH 10/28] Run test command without bash On my machine, the USR2 signal was being sent to Bash, but not to to puma itself. I don't understand why this only affected Ruby 2.2 though! --- test/test_integration_cluster.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_integration_cluster.rb b/test/test_integration_cluster.rb index 559ca354fb..67805f2f65 100644 --- a/test/test_integration_cluster.rb +++ b/test/test_integration_cluster.rb @@ -147,7 +147,7 @@ def test_usr2_restart_preserves_bundler_environment # Must use `bundle exec puma` here, because otherwise Bundler may not be defined, which is required to trigger the bug cmd = "bundle exec puma -q -w 1 --prune-bundler -b tcp://#{HOST}:#{@tcp_port}" Dir.chdir(File.expand_path("bundle_preservation_test", __dir__)) do - @server = IO.popen(env, cmd, "r") + @server = IO.popen(env, cmd.split, "r") end wait_for_server_to_boot @pid = @server.pid From d89af859d39c6e4f9aaab3a5d926343a60a774c9 Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Thu, 13 Feb 2020 11:34:45 -0600 Subject: [PATCH 11/28] Remove skip for USR2 --- test/test_integration_cluster.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/test_integration_cluster.rb b/test/test_integration_cluster.rb index 67805f2f65..1e22e59e9d 100644 --- a/test/test_integration_cluster.rb +++ b/test/test_integration_cluster.rb @@ -135,8 +135,6 @@ def test_stuck_phased_restart # It does not wipe out BUNDLE_GEMFILE et al def test_usr2_restart_preserves_bundler_environment - skip_unless_signal_exist? :USR2 - @tcp_port = UniquePort.call env = { # Intentionally set this to something we wish to keep intact on restarts From c83401cfcf2cbe0ca23e1bef936d720ab5821291 Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Thu, 13 Feb 2020 13:59:16 -0600 Subject: [PATCH 12/28] Move test into separate file This test was failing on Travis when run in parallel with other tests in the TestIntegrationCluster test. Removing the line "parallelize_me!" from the top of the file worked, but I chose to move the test into its own file to keep the existing tests running in parallel. --- test/test_integration_cluster.rb | 24 ---------------------- test/test_preserve_bundler_env.rb | 33 +++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 24 deletions(-) create mode 100644 test/test_preserve_bundler_env.rb diff --git a/test/test_integration_cluster.rb b/test/test_integration_cluster.rb index 1e22e59e9d..a7f58f84ec 100644 --- a/test/test_integration_cluster.rb +++ b/test/test_integration_cluster.rb @@ -133,30 +133,6 @@ def test_stuck_phased_restart worker_respawn { |phase0_worker_pids| Process.kill :USR1, @pid } end - # It does not wipe out BUNDLE_GEMFILE et al - def test_usr2_restart_preserves_bundler_environment - @tcp_port = UniquePort.call - env = { - # Intentionally set this to something we wish to keep intact on restarts - "BUNDLE_GEMFILE" => "Gemfile.bundle_env_preservation_test", - # Don't allow our (rake test's) original env to interfere with the child process - "BUNDLER_ORIG_BUNDLE_GEMFILE" => nil - } - # Must use `bundle exec puma` here, because otherwise Bundler may not be defined, which is required to trigger the bug - cmd = "bundle exec puma -q -w 1 --prune-bundler -b tcp://#{HOST}:#{@tcp_port}" - Dir.chdir(File.expand_path("bundle_preservation_test", __dir__)) do - @server = IO.popen(env, cmd.split, "r") - end - wait_for_server_to_boot - @pid = @server.pid - connection = connect - initial_reply = read_body(connection) - assert_match("Gemfile.bundle_env_preservation_test", initial_reply) - restart_server connection - new_reply = read_body(connection) - assert_match("Gemfile.bundle_env_preservation_test", new_reply) - end - private # Send requests 10 per second. Send 10, then :TERM server, then send another 30. diff --git a/test/test_preserve_bundler_env.rb b/test/test_preserve_bundler_env.rb new file mode 100644 index 0000000000..35aaee7141 --- /dev/null +++ b/test/test_preserve_bundler_env.rb @@ -0,0 +1,33 @@ +require_relative "helper" +require_relative "helpers/integration" + +class TestPreserveBundlerEnv < TestIntegration + def setup + skip NO_FORK_MSG unless HAS_FORK + super + end + + # It does not wipe out BUNDLE_GEMFILE et al + def test_usr2_restart_preserves_bundler_environment + @tcp_port = UniquePort.call + env = { + # Intentionally set this to something we wish to keep intact on restarts + "BUNDLE_GEMFILE" => "Gemfile.bundle_env_preservation_test", + # Don't allow our (rake test's) original env to interfere with the child process + "BUNDLER_ORIG_BUNDLE_GEMFILE" => nil + } + # Must use `bundle exec puma` here, because otherwise Bundler may not be defined, which is required to trigger the bug + cmd = "bundle exec puma -q -w 1 --prune-bundler -b tcp://#{HOST}:#{@tcp_port}" + Dir.chdir(File.expand_path("bundle_preservation_test", __dir__)) do + @server = IO.popen(env, cmd.split, "r") + end + wait_for_server_to_boot + @pid = @server.pid + connection = connect + initial_reply = read_body(connection) + assert_match("Gemfile.bundle_env_preservation_test", initial_reply) + restart_server connection + new_reply = read_body(connection) + assert_match("Gemfile.bundle_env_preservation_test", new_reply) + end +end From cbf32d55400fb6ea217dfeb246f54cc8216ecffb Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Thu, 13 Feb 2020 14:32:10 -0600 Subject: [PATCH 13/28] Add some logging to help diagnose Travis build failure --- test/helpers/integration.rb | 18 ++++++++++++++---- test/test_preserve_bundler_env.rb | 4 ++-- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/test/helpers/integration.rb b/test/helpers/integration.rb index 7cd6b74e3d..173a5fc9a6 100644 --- a/test/helpers/integration.rb +++ b/test/helpers/integration.rb @@ -75,14 +75,24 @@ def restart_server_and_listen(argv) end # reuses an existing connection to make sure that works - def restart_server(connection) + def restart_server(connection, log: false) Process.kill :USR2, @pid connection.write "GET / HTTP/1.1\r\n\r\n" # trigger it to start by sending a new request - wait_for_server_to_boot + wait_for_server_to_boot(log: log) end - def wait_for_server_to_boot - true while @server.gets !~ /Ctrl-C/ # wait for server to say it booted + # wait for server to say it booted + def wait_for_server_to_boot(log: false) + if log + puts "Waiting for server to boot..." + begin + line = @server.gets + puts line if line && line.strip != '' + end while line !~ /Ctrl-C/ + puts "Server booted!" + else + true while @server.gets !~ /Ctrl-C/ + end end def connect(path = nil, unix: false) diff --git a/test/test_preserve_bundler_env.rb b/test/test_preserve_bundler_env.rb index 35aaee7141..07d2840e24 100644 --- a/test/test_preserve_bundler_env.rb +++ b/test/test_preserve_bundler_env.rb @@ -21,12 +21,12 @@ def test_usr2_restart_preserves_bundler_environment Dir.chdir(File.expand_path("bundle_preservation_test", __dir__)) do @server = IO.popen(env, cmd.split, "r") end - wait_for_server_to_boot + wait_for_server_to_boot(log: true) @pid = @server.pid connection = connect initial_reply = read_body(connection) assert_match("Gemfile.bundle_env_preservation_test", initial_reply) - restart_server connection + restart_server connection, log: true new_reply = read_body(connection) assert_match("Gemfile.bundle_env_preservation_test", new_reply) end From 95bcd057394a9f84ef3692269e9137328ed26ac6 Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Thu, 13 Feb 2020 14:45:38 -0600 Subject: [PATCH 14/28] Is that really running over and over??? --- lib/puma/launcher.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/puma/launcher.rb b/lib/puma/launcher.rb index a390cc0cb4..cc25b12682 100644 --- a/lib/puma/launcher.rb +++ b/lib/puma/launcher.rb @@ -295,7 +295,7 @@ def prune_bundler deps, dirs = dependencies_and_files_to_require_after_prune - log '* Pruning Bundler environment' + log "* Pruning Bundler environment #{rand 100}" home = ENV['GEM_HOME'] Bundler.with_original_env do ENV['GEM_HOME'] = home From 3be5f32e070df93132c78afb0efef9c003c33797 Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Thu, 13 Feb 2020 14:50:35 -0600 Subject: [PATCH 15/28] Yes! It is, but why? --- lib/puma/launcher.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/puma/launcher.rb b/lib/puma/launcher.rb index cc25b12682..de6b889c41 100644 --- a/lib/puma/launcher.rb +++ b/lib/puma/launcher.rb @@ -295,7 +295,7 @@ def prune_bundler deps, dirs = dependencies_and_files_to_require_after_prune - log "* Pruning Bundler environment #{rand 100}" + log "* Pruning Bundler environment #{caller.inspect}" home = ENV['GEM_HOME'] Bundler.with_original_env do ENV['GEM_HOME'] = home From 9dcd2e335db44506ec8c4da34a6c25dcf773ab4a Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Thu, 13 Feb 2020 15:12:20 -0600 Subject: [PATCH 16/28] Don't try to prune_bundler again --- lib/puma/launcher.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/puma/launcher.rb b/lib/puma/launcher.rb index de6b889c41..a45757f930 100644 --- a/lib/puma/launcher.rb +++ b/lib/puma/launcher.rb @@ -286,6 +286,7 @@ def puma_wild_location end def prune_bundler + return if ENV.key?('PUMA_BUNDLER_PRUNED') return unless defined?(Bundler) require_rubygems_min_version!(Gem::Version.new("2.2"), "prune_bundler") unless puma_wild_location @@ -295,7 +296,7 @@ def prune_bundler deps, dirs = dependencies_and_files_to_require_after_prune - log "* Pruning Bundler environment #{caller.inspect}" + log "* Pruning Bundler environment" home = ENV['GEM_HOME'] Bundler.with_original_env do ENV['GEM_HOME'] = home From 2ed158ecaca966a7b26bdf361d1b97c0174def68 Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Thu, 13 Feb 2020 15:18:51 -0600 Subject: [PATCH 17/28] OMG it works! Remove diagnostic logging. --- test/test_preserve_bundler_env.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_preserve_bundler_env.rb b/test/test_preserve_bundler_env.rb index 07d2840e24..35aaee7141 100644 --- a/test/test_preserve_bundler_env.rb +++ b/test/test_preserve_bundler_env.rb @@ -21,12 +21,12 @@ def test_usr2_restart_preserves_bundler_environment Dir.chdir(File.expand_path("bundle_preservation_test", __dir__)) do @server = IO.popen(env, cmd.split, "r") end - wait_for_server_to_boot(log: true) + wait_for_server_to_boot @pid = @server.pid connection = connect initial_reply = read_body(connection) assert_match("Gemfile.bundle_env_preservation_test", initial_reply) - restart_server connection, log: true + restart_server connection new_reply = read_body(connection) assert_match("Gemfile.bundle_env_preservation_test", new_reply) end From 359cb697db83d9db0558abce4aea7f84259cac1e Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Thu, 13 Feb 2020 15:39:49 -0600 Subject: [PATCH 18/28] When is Bundler loaded by puma-wild? --- bin/puma-wild | 1 + lib/puma/configuration.rb | 4 ++++ lib/puma/launcher.rb | 4 +++- 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/bin/puma-wild b/bin/puma-wild index b3ba0152e3..9bbd1e3567 100644 --- a/bin/puma-wild +++ b/bin/puma-wild @@ -28,4 +28,5 @@ require 'puma/cli' cli = Puma::CLI.new ARGV +puts "puma-wild defined?(Bundler) = #{defined?(Bundler).inspect}" cli.run diff --git a/lib/puma/configuration.rb b/lib/puma/configuration.rb index e8e546d69c..f587c0c8af 100644 --- a/lib/puma/configuration.rb +++ b/lib/puma/configuration.rb @@ -299,8 +299,12 @@ def rack_builder # a Gemfile if ENV.key? 'PUMA_BUNDLER_PRUNED' begin + puts '>>>> loading Bundler...' require 'bundler/setup' rescue LoadError + puts 'failed' + else + puts 'success' end end diff --git a/lib/puma/launcher.rb b/lib/puma/launcher.rb index a45757f930..98025d2b8b 100644 --- a/lib/puma/launcher.rb +++ b/lib/puma/launcher.rb @@ -286,7 +286,9 @@ def puma_wild_location end def prune_bundler - return if ENV.key?('PUMA_BUNDLER_PRUNED') + puts '>>>> prune_bundler starting...' + puts "prune_bundler defined?(Bundler) = #{defined?(Bundler).inspect}" + #return if ENV.key?('PUMA_BUNDLER_PRUNED') return unless defined?(Bundler) require_rubygems_min_version!(Gem::Version.new("2.2"), "prune_bundler") unless puma_wild_location From 08dc10f1afd4c88b5aeececbd4b330213bbdfc4a Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Thu, 13 Feb 2020 15:46:11 -0600 Subject: [PATCH 19/28] Reenable logging in the test --- test/test_preserve_bundler_env.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_preserve_bundler_env.rb b/test/test_preserve_bundler_env.rb index 35aaee7141..07d2840e24 100644 --- a/test/test_preserve_bundler_env.rb +++ b/test/test_preserve_bundler_env.rb @@ -21,12 +21,12 @@ def test_usr2_restart_preserves_bundler_environment Dir.chdir(File.expand_path("bundle_preservation_test", __dir__)) do @server = IO.popen(env, cmd.split, "r") end - wait_for_server_to_boot + wait_for_server_to_boot(log: true) @pid = @server.pid connection = connect initial_reply = read_body(connection) assert_match("Gemfile.bundle_env_preservation_test", initial_reply) - restart_server connection + restart_server connection, log: true new_reply = read_body(connection) assert_match("Gemfile.bundle_env_preservation_test", new_reply) end From 0d020df4e14ab84d900e69788246383854bb6b28 Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Thu, 13 Feb 2020 15:49:42 -0600 Subject: [PATCH 20/28] Is Bundler defined when using with_clean_env? --- lib/puma/launcher.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/puma/launcher.rb b/lib/puma/launcher.rb index 98025d2b8b..c457c7fdc6 100644 --- a/lib/puma/launcher.rb +++ b/lib/puma/launcher.rb @@ -300,7 +300,8 @@ def prune_bundler log "* Pruning Bundler environment" home = ENV['GEM_HOME'] - Bundler.with_original_env do + #Bundler.with_original_env do + Bundler.with_clean_env do ENV['GEM_HOME'] = home ENV['PUMA_BUNDLER_PRUNED'] = '1' args = [Gem.ruby, puma_wild_location, '-I', dirs.join(':'), deps.join(',')] + @original_argv From d8044958126984a48f21d0ce659bbc2f06b80cbc Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Thu, 13 Feb 2020 16:01:47 -0600 Subject: [PATCH 21/28] When is Bundler loaded? (this is nuts) --- bin/puma-wild | 7 ++++++- lib/puma/launcher.rb | 3 +-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/bin/puma-wild b/bin/puma-wild index 9bbd1e3567..8eb2902e7b 100644 --- a/bin/puma-wild +++ b/bin/puma-wild @@ -3,6 +3,7 @@ # Copyright (c) 2014 Evan Phoenix # +puts "puma-wild 111 defined?(Bundler) = #{defined?(Bundler).inspect}" require 'rubygems' gems = ARGV.shift @@ -15,18 +16,22 @@ if gems == "-I" gems = ARGV.shift end +p gems + gems.split(",").each do |s| name, ver = s.split(":",2) gem name, ver end +puts "puma-wild 222 defined?(Bundler) = #{defined?(Bundler).inspect}" + module Puma; end Puma.const_set("WILD_ARGS", ["-I", inc, gems]) require 'puma/cli' +puts "puma-wild 333 defined?(Bundler) = #{defined?(Bundler).inspect}" cli = Puma::CLI.new ARGV -puts "puma-wild defined?(Bundler) = #{defined?(Bundler).inspect}" cli.run diff --git a/lib/puma/launcher.rb b/lib/puma/launcher.rb index c457c7fdc6..98025d2b8b 100644 --- a/lib/puma/launcher.rb +++ b/lib/puma/launcher.rb @@ -300,8 +300,7 @@ def prune_bundler log "* Pruning Bundler environment" home = ENV['GEM_HOME'] - #Bundler.with_original_env do - Bundler.with_clean_env do + Bundler.with_original_env do ENV['GEM_HOME'] = home ENV['PUMA_BUNDLER_PRUNED'] = '1' args = [Gem.ruby, puma_wild_location, '-I', dirs.join(':'), deps.join(',')] + @original_argv From 10d4183d44893fc650e193bd2a74f5995d5bccb7 Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Thu, 13 Feb 2020 16:10:08 -0600 Subject: [PATCH 22/28] Let's see them args! --- lib/puma/launcher.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/puma/launcher.rb b/lib/puma/launcher.rb index 98025d2b8b..daaead4a84 100644 --- a/lib/puma/launcher.rb +++ b/lib/puma/launcher.rb @@ -306,6 +306,7 @@ def prune_bundler args = [Gem.ruby, puma_wild_location, '-I', dirs.join(':'), deps.join(',')] + @original_argv # Ruby 2.0+ defaults to true which breaks socket activation args += [{:close_others => false}] + p args Kernel.exec(*args) end end From d84e0ae15becfc1b85447ef2073b6a1b8136569c Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Thu, 13 Feb 2020 16:40:00 -0600 Subject: [PATCH 23/28] Now let's see the ENV! --- lib/puma/launcher.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/puma/launcher.rb b/lib/puma/launcher.rb index daaead4a84..81dea80fca 100644 --- a/lib/puma/launcher.rb +++ b/lib/puma/launcher.rb @@ -301,6 +301,9 @@ def prune_bundler log "* Pruning Bundler environment" home = ENV['GEM_HOME'] Bundler.with_original_env do + require 'pp' + puts 'ENV after Bundler.with_original_env:' + pp ENV ENV['GEM_HOME'] = home ENV['PUMA_BUNDLER_PRUNED'] = '1' args = [Gem.ruby, puma_wild_location, '-I', dirs.join(':'), deps.join(',')] + @original_argv From 3d1c5e00b459b3d9a02f4d1d3a3723d7904f9199 Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Thu, 13 Feb 2020 16:47:12 -0600 Subject: [PATCH 24/28] Bundler wins. Just set BUNDLE_GEMFILE manually ...and go back to using Bundler.with_clean_env. --- lib/puma/launcher.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/puma/launcher.rb b/lib/puma/launcher.rb index 81dea80fca..b0520235a4 100644 --- a/lib/puma/launcher.rb +++ b/lib/puma/launcher.rb @@ -300,11 +300,13 @@ def prune_bundler log "* Pruning Bundler environment" home = ENV['GEM_HOME'] - Bundler.with_original_env do + bundle_gemfile = ENV['BUNDLE_GEMFILE'] + Bundler.with_clean_env do require 'pp' puts 'ENV after Bundler.with_original_env:' pp ENV ENV['GEM_HOME'] = home + ENV['BUNDLE_GEMFILE'] = bundle_gemfile ENV['PUMA_BUNDLER_PRUNED'] = '1' args = [Gem.ruby, puma_wild_location, '-I', dirs.join(':'), deps.join(',')] + @original_argv # Ruby 2.0+ defaults to true which breaks socket activation From f73cd572332cafac66b2ff267df8beb596cceb32 Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Thu, 13 Feb 2020 16:55:35 -0600 Subject: [PATCH 25/28] Remove diagnostic logging --- bin/puma-wild | 6 ------ lib/puma/configuration.rb | 4 ---- lib/puma/launcher.rb | 9 +-------- test/test_preserve_bundler_env.rb | 4 ++-- 4 files changed, 3 insertions(+), 20 deletions(-) diff --git a/bin/puma-wild b/bin/puma-wild index 8eb2902e7b..b3ba0152e3 100644 --- a/bin/puma-wild +++ b/bin/puma-wild @@ -3,7 +3,6 @@ # Copyright (c) 2014 Evan Phoenix # -puts "puma-wild 111 defined?(Bundler) = #{defined?(Bundler).inspect}" require 'rubygems' gems = ARGV.shift @@ -16,22 +15,17 @@ if gems == "-I" gems = ARGV.shift end -p gems - gems.split(",").each do |s| name, ver = s.split(":",2) gem name, ver end -puts "puma-wild 222 defined?(Bundler) = #{defined?(Bundler).inspect}" - module Puma; end Puma.const_set("WILD_ARGS", ["-I", inc, gems]) require 'puma/cli' -puts "puma-wild 333 defined?(Bundler) = #{defined?(Bundler).inspect}" cli = Puma::CLI.new ARGV cli.run diff --git a/lib/puma/configuration.rb b/lib/puma/configuration.rb index f587c0c8af..e8e546d69c 100644 --- a/lib/puma/configuration.rb +++ b/lib/puma/configuration.rb @@ -299,12 +299,8 @@ def rack_builder # a Gemfile if ENV.key? 'PUMA_BUNDLER_PRUNED' begin - puts '>>>> loading Bundler...' require 'bundler/setup' rescue LoadError - puts 'failed' - else - puts 'success' end end diff --git a/lib/puma/launcher.rb b/lib/puma/launcher.rb index b0520235a4..f45e035edc 100644 --- a/lib/puma/launcher.rb +++ b/lib/puma/launcher.rb @@ -286,9 +286,6 @@ def puma_wild_location end def prune_bundler - puts '>>>> prune_bundler starting...' - puts "prune_bundler defined?(Bundler) = #{defined?(Bundler).inspect}" - #return if ENV.key?('PUMA_BUNDLER_PRUNED') return unless defined?(Bundler) require_rubygems_min_version!(Gem::Version.new("2.2"), "prune_bundler") unless puma_wild_location @@ -298,20 +295,16 @@ def prune_bundler deps, dirs = dependencies_and_files_to_require_after_prune - log "* Pruning Bundler environment" + log '* Pruning Bundler environment' home = ENV['GEM_HOME'] bundle_gemfile = ENV['BUNDLE_GEMFILE'] Bundler.with_clean_env do - require 'pp' - puts 'ENV after Bundler.with_original_env:' - pp ENV ENV['GEM_HOME'] = home ENV['BUNDLE_GEMFILE'] = bundle_gemfile ENV['PUMA_BUNDLER_PRUNED'] = '1' args = [Gem.ruby, puma_wild_location, '-I', dirs.join(':'), deps.join(',')] + @original_argv # Ruby 2.0+ defaults to true which breaks socket activation args += [{:close_others => false}] - p args Kernel.exec(*args) end end diff --git a/test/test_preserve_bundler_env.rb b/test/test_preserve_bundler_env.rb index 07d2840e24..35aaee7141 100644 --- a/test/test_preserve_bundler_env.rb +++ b/test/test_preserve_bundler_env.rb @@ -21,12 +21,12 @@ def test_usr2_restart_preserves_bundler_environment Dir.chdir(File.expand_path("bundle_preservation_test", __dir__)) do @server = IO.popen(env, cmd.split, "r") end - wait_for_server_to_boot(log: true) + wait_for_server_to_boot @pid = @server.pid connection = connect initial_reply = read_body(connection) assert_match("Gemfile.bundle_env_preservation_test", initial_reply) - restart_server connection, log: true + restart_server connection new_reply = read_body(connection) assert_match("Gemfile.bundle_env_preservation_test", new_reply) end From bc4217b688c4c7a3550f4dfd7b54b082758c8376 Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Thu, 13 Feb 2020 17:02:50 -0600 Subject: [PATCH 26/28] Update changelog --- History.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/History.md b/History.md index ff3e5969e1..f3c6c387a3 100644 --- a/History.md +++ b/History.md @@ -10,7 +10,7 @@ * Bugfixes * Your bugfix goes here (#Github Number) * Windows update extconf.rb for use with ssp and varied Ruby/MSYS2 combinations (#2069) - * Preserve `BUNDLE_GEMFILE` and other Bundler env vars on restart (#1893) + * Preserve `BUNDLE_GEMFILE` env var on restart (#1893) * Refactor * Remove unused loader argument from Plugin initializer (#2095) From 62f075f468dd6627cf53ce73a85a25557a6d1b37 Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Fri, 14 Feb 2020 10:48:23 -0600 Subject: [PATCH 27/28] Add back test skip for platforms without USR2 --- test/test_preserve_bundler_env.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/test_preserve_bundler_env.rb b/test/test_preserve_bundler_env.rb index 35aaee7141..1dd1b43447 100644 --- a/test/test_preserve_bundler_env.rb +++ b/test/test_preserve_bundler_env.rb @@ -9,6 +9,8 @@ def setup # It does not wipe out BUNDLE_GEMFILE et al def test_usr2_restart_preserves_bundler_environment + skip_unless_signal_exist? :USR2 + @tcp_port = UniquePort.call env = { # Intentionally set this to something we wish to keep intact on restarts From 1846ef3c56543b55358ba465938302536fdd9ab4 Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Tue, 18 Feb 2020 11:58:07 -0600 Subject: [PATCH 28/28] Clarify changelog entry --- History.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/History.md b/History.md index f3c6c387a3..991a053014 100644 --- a/History.md +++ b/History.md @@ -10,7 +10,7 @@ * Bugfixes * Your bugfix goes here (#Github Number) * Windows update extconf.rb for use with ssp and varied Ruby/MSYS2 combinations (#2069) - * Preserve `BUNDLE_GEMFILE` env var on restart (#1893) + * Preserve `BUNDLE_GEMFILE` env var when using `prune_bundler` (#1893) * Refactor * Remove unused loader argument from Plugin initializer (#2095)