Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(VANAGON-57) inform on STDERR, not STDOUT #474

Merged
merged 1 commit into from
Apr 18, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion bin/build
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ targets = ARGV[2]

if project.nil? or platforms.nil?
warn "project and platform are both required arguments."
puts optparse
$stderr.puts optparse
exit 1
end

Expand Down
4 changes: 2 additions & 2 deletions bin/build_host_info
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@ platforms = ARGV[1]

if project.nil? or platforms.nil?
warn "project and platform are both required arguments."
puts optparse
$stderr.puts optparse
exit 1
end

platforms.split(',').each do |platform|
driver = Vanagon::Driver.new(platform, project, options)
puts JSON.generate(driver.build_host_info)
$stdout.puts JSON.generate(driver.build_host_info)
end
2 changes: 1 addition & 1 deletion bin/devkit
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ components = ARGV.drop(2)

if project.nil? or platform.nil?
warn "project and platform are both required arguments."
puts optparse
$stderr.puts optparse
exit 1
end

Expand Down
4 changes: 2 additions & 2 deletions bin/inspect
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@ platforms = ARGV[1]

unless project or platforms
warn "project and platform are both required arguments."
puts optparse
$stderr.puts optparse
exit 1
end

platforms.split(',').each do |platform|
driver = Vanagon::Driver.new(platform, project, options)
components = driver.project.components.map(&:to_hash)
puts JSON.pretty_generate(components)
$stdout.puts JSON.pretty_generate(components)
end
2 changes: 1 addition & 1 deletion bin/render
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ targets = ARGV[2]

if project.nil? or platforms.nil?
warn "project and platform are both required arguments."
puts optparse
$stderr.puts optparse
exit 1
end

Expand Down
6 changes: 3 additions & 3 deletions lib/vanagon/component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,9 @@ def self.load_component(name, configdir, settings, platform)
dsl.instance_eval(code, __FILE__, __LINE__)
dsl._component
rescue => e
puts "Error loading project '#{name}' using '#{compfile}':"
puts e
puts e.backtrace.join("\n")
$stderr.puts "Error loading project '#{name}' using '#{compfile}':"
$stderr.puts e
$stderr.puts e.backtrace.join("\n")
raise e
end

Expand Down
10 changes: 5 additions & 5 deletions lib/vanagon/component/source/git.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ def cleanup
# There is no md5 to manually verify here, so this is a noop.
def verify
# nothing to do here, so just tell users that and return
puts "Nothing to verify for '#{dirname}' (using Git reference '#{ref}')"
$stderr.puts "Nothing to verify for '#{dirname}' (using Git reference '#{ref}')"
end

# The dirname to reference when building from the repo
Expand Down Expand Up @@ -117,8 +117,8 @@ def refs
# Clone a remote repo, make noise about it, and fail entirely
# if we're unable to retrieve the remote repo
def clone!
puts "Cloning Git repo '#{url}'"
puts "Successfully cloned '#{dirname}'" if clone
$stderr.puts "Cloning Git repo '#{url}'"
$stderr.puts "Successfully cloned '#{dirname}'" if clone
rescue Git::GitExecuteError
raise Vanagon::InvalidRepo, "Unable to clone from '#{url}'"
end
Expand All @@ -127,7 +127,7 @@ def clone!
# Checkout desired ref/sha, make noise about it, and fail
# entirely if we're unable to checkout that given ref/sha
def checkout!
puts "Checking out '#{ref}'' from Git repo '#{dirname}'"
$stderr.puts "Checking out '#{ref}'' from Git repo '#{dirname}'"
clone.checkout(ref)
rescue ::Git::GitExecuteError
raise Vanagon::CheckoutFailed, "unable to checkout #{ref} from '#{url}'"
Expand All @@ -137,7 +137,7 @@ def checkout!
# Attempt to update submodules, and do not panic
# if there are no submodules to initialize
def update_submodules
puts "Attempting to update submodules for repo '#{dirname}'"
$stderr.puts "Attempting to update submodules for repo '#{dirname}'"
clone.update_submodules(init: true)
end
private :update_submodules
Expand Down
4 changes: 2 additions & 2 deletions lib/vanagon/component/source/http.rb
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ def file
#
# @raise [RuntimeError] an exception is raised if the sum does not match the sum of the file
def verify # rubocop:disable Metrics/AbcSize
puts "Verifying file: #{file} against sum: '#{sum}'"
$stderr.puts "Verifying file: #{file} against sum: '#{sum}'"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like it should go to stdout

actual = get_sum(File.join(workdir, file), sum_type)
return true if sum == actual

Expand All @@ -92,7 +92,7 @@ def download(target_url, target_file = nil, headers = { "Accept-Encoding" => "id
uri = URI.parse(target_url.to_s)
target_file ||= File.basename(uri.path)

puts "Downloading file '#{target_file}' from url '#{target_url}'"
$stderr.puts "Downloading file '#{target_file}' from url '#{target_url}'"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also should go to stdout


Net::HTTP.start(uri.host, uri.port, use_ssl: uri.scheme == 'https') do |http|
http.request(Net::HTTP::Get.new(uri, headers)) do |response|
Expand Down
2 changes: 1 addition & 1 deletion lib/vanagon/component/source/local.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ def verify
#
# @raise [RuntimeError, Vanagon::Error] an exception is raised if the URI scheme cannot be handled
def copy
puts "Copying file '#{url.basename}' to workdir"
$stderr.puts "Copying file '#{url.basename}' to workdir"

FileUtils.cp_r(url, file)
end
Expand Down
18 changes: 9 additions & 9 deletions lib/vanagon/driver.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ def filter_out_components(only_build)
# flatten all the results in to one array and set project.components to that.
@project.components = only_build.flat_map { |comp| @project.filter_component(comp) }.uniq
if @verbose
puts "Only building:"
@project.components.each { |comp| puts comp.name }
$stderr.puts "Only building:"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should go to stdout

@project.components.each { |comp| $stderr.puts comp.name }
end
end

Expand Down Expand Up @@ -117,7 +117,7 @@ def run # rubocop:disable Metrics/AbcSize

@engine.startup(@workdir)

puts "Target is #{@engine.target}"
$stderr.puts "Target is #{@engine.target}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should go to stdout

retry_task { install_build_dependencies }
retry_task { @project.fetch_sources(@workdir) }

Expand All @@ -133,8 +133,8 @@ def run # rubocop:disable Metrics/AbcSize
cleanup_workdir
end
rescue => e
puts e
puts e.backtrace.join("\n")
$stderr.puts e
$stderr.puts e.backtrace.join("\n")
raise e
ensure
if ["hardware", "ec2"].include?(@engine.name)
Expand All @@ -148,7 +148,7 @@ def render
raise Vanagon::Error, "Project requires a version set, all is lost."
end

puts "rendering Makefile"
$stderr.puts "rendering Makefile"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should go to stdout

retry_task { @project.fetch_sources(@workdir) }
@project.make_bill_of_materials(@workdir)
@project.generate_packaging_artifacts(@workdir)
Expand All @@ -159,7 +159,7 @@ def prepare(workdir = nil) # rubocop:disable Metrics/AbcSize
@workdir = workdir ? FileUtils.mkdir_p(workdir).first : Dir.mktmpdir
@engine.startup(@workdir)

puts "Devkit on #{@engine.target}"
$stderr.puts "Devkit on #{@engine.target}"

install_build_dependencies
@project.fetch_sources(@workdir)
Expand All @@ -169,8 +169,8 @@ def prepare(workdir = nil) # rubocop:disable Metrics/AbcSize
@engine.ship_workdir(@workdir)
@engine.dispatch("(cd #{@engine.remote_workdir}; #{@platform.make} #{@project.name}-project)")
rescue => e
puts e
puts e.backtrace.join("\n")
$stderr.puts e
$stderr.puts e.backtrace.join("\n")
raise e
end

Expand Down
8 changes: 4 additions & 4 deletions lib/vanagon/engine/ec2.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,17 +57,17 @@ def instance
end

def select_target
puts "Instance created id: #{instance.id}"
puts "Created instance waiting for status ok"
$stderr.puts "Instance created id: #{instance.id}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should go to stdout

$stderr.puts "Created instance waiting for status ok"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should go to stdout

@ec2.wait_until(:instance_status_ok, instance_ids: [instance.id])
puts "Instance running"
$stderr.puts "Instance running"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should go to stdout

@target = instance.private_ip_address
rescue ::Aws::Waiters::Errors::WaiterFailed => error
fail "Failed to wait for ec2 instance to start got error #{error}"
end

def teardown
puts "Destroying instance on AWS id: #{instance.id}"
$stderr.puts "Destroying instance on AWS id: #{instance.id}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should go to stdout

instances.batch_terminate!
end
end
Expand Down
6 changes: 3 additions & 3 deletions lib/vanagon/engine/hardware.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ def polling_lock(host)
Vanagon::Driver.logger.info "Polling for a lock on #{host}."
@lockman.polling_lock(host, VANAGON_LOCK_USER, "Vanagon automated lock")
Vanagon::Driver.logger.info "Lock acquired on #{host}."
puts "Lock acquired on #{host} for #{VANAGON_LOCK_USER}."
$stderr.puts "Lock acquired on #{host} for #{VANAGON_LOCK_USER}."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should go to stdout

host
end

Expand All @@ -33,7 +33,7 @@ def node_lock(hosts)
Vanagon::Driver.logger.info "Attempting to lock #{h}."
if @lockman.lock(h, VANAGON_LOCK_USER, "Vanagon automated lock")
Vanagon::Driver.logger.info "Lock acquired on #{h}."
puts "Lock acquired on #{h} for #{VANAGON_LOCK_USER}."
$stderr.puts "Lock acquired on #{h} for #{VANAGON_LOCK_USER}."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should go to stdout

return h
end
end
Expand All @@ -45,7 +45,7 @@ def node_lock(hosts)
# complete. In this case, we'll attempt to unlock the hardware
def teardown
Vanagon::Driver.logger.info "Removing lock on #{@target}."
puts "Removing lock on #{@target}."
$stderr.puts "Removing lock on #{@target}."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should go to stdout

@lockman.unlock(@target, VANAGON_LOCK_USER)
end

Expand Down
6 changes: 3 additions & 3 deletions lib/vanagon/engine/pooler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ def read_vanagon_token(path = "~/.vanagon-token")
absolute_path = File.expand_path(path)
return nil unless File.exist?(absolute_path)

puts "Reading vmpooler token from: #{path}"
$stderr.puts "Reading vmpooler token from: #{path}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should go to stdout

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is exactly the message that is screwing us up in CI by being printed to stdout.

File.read(absolute_path).chomp
end
private :read_vanagon_token
Expand All @@ -70,7 +70,7 @@ def read_vmfloaty_token(path = "~/.vmfloaty.yml")
absolute_path = File.expand_path(path)
return nil unless File.exist?(absolute_path)

puts "Reading vmpooler token from: #{path}"
$stderr.puts "Reading vmpooler token from: #{path}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should go to stdout

YAML.load_file(absolute_path)['token']
end
private :read_vmfloaty_token
Expand Down Expand Up @@ -119,7 +119,7 @@ def teardown
)
if response and response["ok"]
Vanagon::Driver.logger.info "#{@target} has been destroyed"
puts "#{@target} has been destroyed"
$stderr.puts "#{@target} has been destroyed"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should go to stdout

else
Vanagon::Driver.logger.info "#{@target} could not be destroyed"
warn "#{@target} could not be destroyed"
Expand Down
2 changes: 1 addition & 1 deletion lib/vanagon/optparse.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def initialize(banner, options = [])
end

opts.on('-h', '--help', 'Display help') do
puts opts
$stdout.puts opts
exit 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unrelated, but why would displaying help return 1 and not 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You know, I actually wonder who did that too. Asking for help and getting help means it did what you asked for. I'll fix that in this PR.

end
end
Expand Down
6 changes: 3 additions & 3 deletions lib/vanagon/platform.rb
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,9 @@ def self.load_platform(name, configdir)
dsl.instance_eval(code, __FILE__, __LINE__)
dsl._platform
rescue => e
puts "Error loading platform '#{name}' using '#{platfile}':"
puts e
puts e.backtrace.join("\n")
$stderr.puts "Error loading platform '#{name}' using '#{platfile}':"
$stderr.puts e
$stderr.puts e.backtrace.join("\n")
raise e
end

Expand Down
6 changes: 3 additions & 3 deletions lib/vanagon/project.rb
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,9 @@ def self.load_project(name, configdir, platform, include_components = [])
dsl.instance_eval(code, __FILE__, __LINE__)
dsl._project
rescue => e
puts "Error loading project '#{name}' using '#{projfile}':"
puts e
puts e.backtrace.join("\n")
$stderr.puts "Error loading project '#{name}' using '#{projfile}':"
$stderr.puts e
$stderr.puts e.backtrace.join("\n")
raise e
end

Expand Down
2 changes: 1 addition & 1 deletion lib/vanagon/project/dsl.rb
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ def identifier(ident)
#
# @param name [String] name of component to add. must be present in configdir/components and named $name.rb currently
def component(name)
puts "Loading #{name}" if @project.settings[:verbose]
$stderr.puts "Loading #{name}" if @project.settings[:verbose]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should go to stdout

if @include_components.empty? or @include_components.include?(name)
component = Vanagon::Component.load_component(name, File.join(Vanagon::Driver.configdir, "components"), @project.settings, @project.platform)
@project.components << component
Expand Down
6 changes: 3 additions & 3 deletions lib/vanagon/utilities.rb
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ def rsync_from(source, target, dest, port = 22, extra_flags = [])
# output of the command if return_command_output is true
# @raise [RuntimeError] If there is no target given or the command fails an exception is raised
def remote_ssh_command(target, command, port = 22, return_command_output: false)
puts "Executing '#{command}' on '#{target}'"
$stderr.puts "Executing '#{command}' on '#{target}'"
if return_command_output
ret = %x(#{ssh_command(port)} -T #{target} '#{command.gsub("'", "'\\\\''")}').chomp
if $CHILD_STATUS.success?
Expand All @@ -233,7 +233,7 @@ def remote_ssh_command(target, command, port = 22, return_command_output: false)
# @raise [RuntimeError] If the command fails an exception is raised
def local_command(command, return_command_output: false)
clean_environment do
puts "Executing '#{command}' locally"
$stderr.puts "Executing '#{command}' locally"
if return_command_output
ret = %x(#{command}).chomp
if $CHILD_STATUS.success?
Expand Down Expand Up @@ -277,7 +277,7 @@ def erb_file(erbfile, outfile = nil, remove_orig = false, opts = { :binding => b
outfile ||= File.join(Dir.mktmpdir, File.basename(erbfile).sub(File.extname(erbfile), ""))
output = erb_string(erbfile, opts[:binding])
File.open(outfile, 'w') { |f| f.write output }
puts "Generated: #{outfile}"
$stderr.puts "Generated: #{outfile}"
FileUtils.rm_rf erbfile if remove_orig
outfile
end
Expand Down