use backticks instead of vanilla system in Rails to gather more debug output?

A team member had a problem where due to a config change, pg_dump was not on path, so when he hit:

command = “pg_dump -i -s -x -O -f #{Shellwords.escape(filename)} #{search_path} #{Shellwords.escape(configuration[‘database’])}”

raise ‘Error dumping database’ unless Kernel.system(command)

in https://github.com/rails/rails/blob/master/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb, it just failed with “Error dumping database” without the stdout/stderr output that would have helped diagnose the error.

Most of the time all the output from command execution isn’t needed, but when it fails, it seems like it might be good to use something that could log output to debug.

Backticks would be a simple solution, or Open3/Open4 if shell more interaction might be required (and if desired).

I’m not sure exactly why Kernel.system is used as much as it is in Rails, but there is one mention in about why it isn’t used in a particular case:

https://github.com/rails/rails/blob/master/railties/lib/rails/generators/app_base.rb#L320

We use backticks and #print here instead of vanilla #system because it

is easier to silence stdout in the existing test suite this way. The

end-user gets the bundler commands called anyway, so no big deal.

So, before submitting a patch to use backticks, I was curious as to why vanilla system calls are used in Rails at all?

Just for reference, here are the places where system is used currently for command execution:

activerecord/lib/active_record/migration.rb: system(“bin/rake db:test:prepare”)

activerecord/lib/active_record/tasks/mysql_database_tasks.rb: unless Kernel.system(*args)

activerecord/lib/active_record/tasks/mysql_database_tasks.rb: Kernel.system(*args)

activerecord/lib/active_record/tasks/postgresql_database_tasks.rb: raise ‘Error dumping database’ unless Kernel.system(command)

activerecord/lib/active_record/tasks/postgresql_database_tasks.rb: Kernel.system(“psql -q -f #{Shellwords.escape(filename)} #{configuration[‘database’]}”)

railties/lib/rails/generators/app_base.rb: # We use backticks and #print here instead of vanilla #system because it

railties/lib/rails/generators/rails/app/templates/bin/setup: system ‘gem install bundler --conservative’

railties/lib/rails/generators/rails/app/templates/bin/setup: system(‘bundle check’) or system(‘bundle install’)

railties/lib/rails/generators/rails/app/templates/bin/setup: system ‘ruby bin/rake db:setup’

And places where backticks are used currently for command execution:

railties/lib/rails/api/task.rb: “master@#{git rev-parse HEAD[0, 7]}”

railties/lib/rails/generators/app_base.rb: output = "#{Gem.ruby}" "#{_bundle_command}" #{command}

railties/lib/rails/generators/rails/plugin/plugin_generator.rb: @author = git config user.name.chomp rescue default

railties/lib/rails/generators/rails/plugin/plugin_generator.rb: @email = git config user.email.chomp rescue default

And popen uses:

actionpack/lib/action_dispatch/journey/gtg/transition_table.rb: svg = IO.popen(‘dot -Tsvg’, ‘w+’) { |f|

activesupport/lib/active_support/testing/isolation.rb: child = IO.popen([env, command])

Thanks in advance,

Gary

I’d be up for submitting a PR to replace all shelling out with a method like e.g. Rails.sh the takes a string command and optional block so that shelling out can be better controlled.