From 888c65d5c9f168cc0a601e250166ccb1b9da6f90 Mon Sep 17 00:00:00 2001 From: Brian John Date: Wed, 23 Nov 2022 15:43:49 -0600 Subject: [PATCH] [Fix #11188] Add a `--no-detach` option to `--start-server`. This makes it easy to run the RuboCop server process from within Docker, where detaching the process causes the container to exit. --- changelog/new_add_no_detach_option.md | 1 + docs/modules/ROOT/pages/usage/server.adoc | 9 ++++ lib/rubocop/options.rb | 4 ++ lib/rubocop/server/cli.rb | 52 +++++++++++++++------- lib/rubocop/server/client_command/start.rb | 7 ++- lib/rubocop/server/core.rb | 29 +++++++++--- spec/rubocop/options_spec.rb | 1 + spec/rubocop/server/cli_spec.rb | 35 +++++++++++++-- 8 files changed, 109 insertions(+), 29 deletions(-) create mode 100644 changelog/new_add_no_detach_option.md diff --git a/changelog/new_add_no_detach_option.md b/changelog/new_add_no_detach_option.md new file mode 100644 index 00000000000..bfc2ccd0fa1 --- /dev/null +++ b/changelog/new_add_no_detach_option.md @@ -0,0 +1 @@ +* [#11188](https://github.com/rubocop/rubocop/issues/11188): Add a `--no-detach` option for `--start-server`. This will start the server process in the foreground, which can be helpful when running within Docker where detaching the process terminates the container. ([@f1sherman][]) diff --git a/docs/modules/ROOT/pages/usage/server.adoc b/docs/modules/ROOT/pages/usage/server.adoc index 4f4d1c6139f..28ce6ec0e6c 100644 --- a/docs/modules/ROOT/pages/usage/server.adoc +++ b/docs/modules/ROOT/pages/usage/server.adoc @@ -57,6 +57,12 @@ RuboCop version incompatibility found, RuboCop server restarting... RuboCop server starting on 127.0.0.1:60665. ``` +If you would like to start the server in the foreground, which can be useful when running within Docker, you can pass the `--no-detach` option. + +```console +% rubocop --start-server --no-detach +``` + == Restart Server The started server does not reload the configuration file. @@ -92,6 +98,9 @@ These are the command-line options for server operations: | `--server-status` | Show server status. + +| `--no-detach` +| Run the server process in the foreground. |=== TIP: You can specify the server host and port with the $RUBOCOP_SERVER_HOST and the $RUBOCOP_SERVER_PORT environment variables. diff --git a/lib/rubocop/options.rb b/lib/rubocop/options.rb index 7055854912b..33f1f8a34e0 100644 --- a/lib/rubocop/options.rb +++ b/lib/rubocop/options.rb @@ -206,6 +206,7 @@ def add_server_options(opts) option(opts, '--start-server') option(opts, '--stop-server') option(opts, '--server-status') + option(opts, '--no-detach') end end @@ -465,6 +466,7 @@ def validate_cache_enabled_for_cache_root # This module contains help texts for command line options. # @api private + # rubocop:disable Metrics/ModuleLength module OptionsHelp MAX_EXCL = RuboCop::Options::DEFAULT_MAXIMUM_EXCLUSION_ITEMS.to_s FORMATTER_OPTION_LIST = RuboCop::Formatter::FormatterSet::BUILTIN_FORMATTERS_FOR_KEYS.keys @@ -599,9 +601,11 @@ module OptionsHelp start_server: 'Start server process.', stop_server: 'Stop server process.', server_status: 'Show server status.', + no_detach: 'Run the server process in the foreground.', raise_cop_error: ['Raise cop-related errors with cause and location.', 'This is used to prevent cops from failing silently.', 'Default is false.'] }.freeze end + # rubocop:enable Metrics/ModuleLength end diff --git a/lib/rubocop/server/cli.rb b/lib/rubocop/server/cli.rb index ff5ce4bea8e..46620ba70ef 100644 --- a/lib/rubocop/server/cli.rb +++ b/lib/rubocop/server/cli.rb @@ -23,15 +23,21 @@ class CLI STATUS_ERROR = 2 SERVER_OPTIONS = %w[ - --server --no-server --server-status --restart-server --start-server --stop-server + --server + --no-server + --server-status + --restart-server + --start-server + --stop-server + --no-detach ].freeze EXCLUSIVE_OPTIONS = (SERVER_OPTIONS - %w[--server --no-server]).freeze + NO_DETACH_OPTIONS = %w[--server --start-server --restart-server].freeze def initialize @exit = false end - # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/MethodLength def run(argv = ARGV) unless Server.support_server? return error('RuboCop server is not supported by this Ruby.') if use_server_option?(argv) @@ -40,16 +46,34 @@ def run(argv = ARGV) end Cache.cache_root_path = fetch_cache_root_path_from(argv) - deleted_server_arguments = delete_server_argument_from(argv) - if deleted_server_arguments.size >= 2 - return error("#{deleted_server_arguments.join(', ')} cannot be specified together.") + process_arguments(argv) + end + + def exit? + @exit + end + + private + + # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/MethodLength + def process_arguments(argv) + server_arguments = delete_server_argument_from(argv) + + detach = !server_arguments.delete('--no-detach') + + if server_arguments.size >= 2 + return error("#{server_arguments.join(', ')} cannot be specified together.") end - server_command = deleted_server_arguments.first + server_command = server_arguments.first + + unless detach || NO_DETACH_OPTIONS.include?(server_command) + return error("#{server_command} cannot be combined with --no-detach.") + end if EXCLUSIVE_OPTIONS.include?(server_command) && argv.count > allowed_option_count - return error("#{server_command} cannot be combined with other options.") + return error("#{server_command} cannot be combined with #{argv[0]}.") end if server_command.nil? @@ -57,23 +81,17 @@ def run(argv = ARGV) ArgumentsFile.read_as_arguments.delete('--server') end - run_command(server_command) + run_command(server_command, detach: detach) STATUS_SUCCESS end # rubocop:enable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/MethodLength - def exit? - @exit - end - - private - # rubocop:disable Metrics/CyclomaticComplexity, Metrics/MethodLength: - def run_command(server_command) + def run_command(server_command, detach:) case server_command when '--server' - Server::ClientCommand::Start.new.run unless Server.running? + Server::ClientCommand::Start.new(detach: detach).run unless Server.running? when '--no-server' Server::ClientCommand::Stop.new.run if Server.running? when '--restart-server' @@ -81,7 +99,7 @@ def run_command(server_command) Server::ClientCommand::Restart.new.run when '--start-server' @exit = true - Server::ClientCommand::Start.new.run + Server::ClientCommand::Start.new(detach: detach).run when '--stop-server' @exit = true Server::ClientCommand::Stop.new.run diff --git a/lib/rubocop/server/client_command/start.rb b/lib/rubocop/server/client_command/start.rb index 9cec4cd6ac2..1c675682f8f 100644 --- a/lib/rubocop/server/client_command/start.rb +++ b/lib/rubocop/server/client_command/start.rb @@ -15,6 +15,11 @@ module ClientCommand # This class is a client command to start server process. # @api private class Start < Base + def initialize(detach: true) + @detach = detach + super() + end + def run if Server.running? warn "RuboCop server (#{Cache.pid_path.read}) is already running." @@ -34,7 +39,7 @@ def run host = ENV.fetch('RUBOCOP_SERVER_HOST', '127.0.0.1') port = ENV.fetch('RUBOCOP_SERVER_PORT', 0) - Server::Core.new.start(host, port) + Server::Core.new.start(host, port, detach: @detach) end end end diff --git a/lib/rubocop/server/core.rb b/lib/rubocop/server/core.rb index 4fa24513af2..b47d3ae1572 100644 --- a/lib/rubocop/server/core.rb +++ b/lib/rubocop/server/core.rb @@ -27,31 +27,46 @@ def token self.class.token end - def start(host, port) + def start(host, port, detach: true) $PROGRAM_NAME = "rubocop --server #{Cache.project_dir}" require 'rubocop' start_server(host, port) - demonize if server_mode? + return unless server_mode? + + detach ? detach_server : run_server end private - def demonize - Cache.write_port_and_token_files(port: @server.addr[1], token: token) + def detach_server + write_port_and_token_files pid = fork do Process.daemon(true) $stderr.reopen(Cache.stderr_path, 'w') - Cache.write_pid_file do - read_socket(@server.accept) until @server.closed? - end + process_input end Process.waitpid(pid) end + def write_port_and_token_files + Cache.write_port_and_token_files(port: @server.addr[1], token: token) + end + + def process_input + Cache.write_pid_file do + read_socket(@server.accept) until @server.closed? + end + end + + def run_server + write_port_and_token_files + process_input + end + def server_mode? true end diff --git a/spec/rubocop/options_spec.rb b/spec/rubocop/options_spec.rb index 995aef4260d..2f7a8c51161 100644 --- a/spec/rubocop/options_spec.rb +++ b/spec/rubocop/options_spec.rb @@ -98,6 +98,7 @@ def abs(path) --start-server Start server process. --stop-server Stop server process. --server-status Show server status. + --no-detach Run the server process in the foreground. Output Options: -f, --format FORMATTER Choose an output formatter. This option diff --git a/spec/rubocop/server/cli_spec.rb b/spec/rubocop/server/cli_spec.rb index 4a74c0385a5..8f371a31170 100644 --- a/spec/rubocop/server/cli_spec.rb +++ b/spec/rubocop/server/cli_spec.rb @@ -53,6 +53,15 @@ end end + context 'when using `--start-server` option with `--no-detach`' do + it 'returns exit status 0 and display an information message' do + expect(cli.run(['--start-server', '--no-detach'])).to eq(0) + expect(cli.exit?).to be(true) + expect($stdout.string).to match(/RuboCop server starting on/) + expect($stderr.string).to eq '' + end + end + context 'when using `--stop-server` option' do it 'returns exit status 0 and display a warning message' do expect(cli.run(['--stop-server'])).to eq(0) @@ -71,6 +80,15 @@ end end + context 'when using `--restart-server` option with `--no-detach`' do + it 'returns exit status 0 and display an information message' do + expect(cli.run(['--restart-server', '--no-detach'])).to eq(0) + expect(cli.exit?).to be(true) + expect($stdout.string).to match(/RuboCop server starting on/) + expect($stderr.string).to eq "RuboCop server is not running.\n" + end + end + context 'when using `--server-status` option' do it 'returns exit status 0 and display an information message' do expect(cli.run(['--server-status'])).to eq(0) @@ -156,7 +174,7 @@ expect(cli.run(['--restart-server', '--format', 'simple'])).to eq(2) expect(cli.exit?).to be(true) expect($stdout.string).to eq '' - expect($stderr.string).to eq "--restart-server cannot be combined with other options.\n" + expect($stderr.string).to eq "--restart-server cannot be combined with --format.\n" end end @@ -165,7 +183,7 @@ expect(cli.run(['--start-server', '--format', 'simple'])).to eq(2) expect(cli.exit?).to be(true) expect($stdout.string).to eq '' - expect($stderr.string).to eq "--start-server cannot be combined with other options.\n" + expect($stderr.string).to eq "--start-server cannot be combined with --format.\n" end end @@ -174,7 +192,7 @@ expect(cli.run(['--stop-server', '--format', 'simple'])).to eq(2) expect(cli.exit?).to be(true) expect($stdout.string).to eq '' - expect($stderr.string).to eq "--stop-server cannot be combined with other options.\n" + expect($stderr.string).to eq "--stop-server cannot be combined with --format.\n" end end @@ -183,7 +201,16 @@ expect(cli.run(['--server-status', '--format', 'simple'])).to eq(2) expect(cli.exit?).to be(true) expect($stdout.string).to eq '' - expect($stderr.string).to eq "--server-status cannot be combined with other options.\n" + expect($stderr.string).to eq "--server-status cannot be combined with --format.\n" + end + end + + context 'when using server option with `--no-detach` option' do + it 'returns exit status 2 and display an error message' do + expect(cli.run(['--server-status', '--no-detach'])).to eq(2) + expect(cli.exit?).to be(true) + expect($stdout.string).to eq '' + expect($stderr.string).to eq "--server-status cannot be combined with --no-detach.\n" end end