From c5a5b3d0b20aa2e82b334d05d775fd388005486c Mon Sep 17 00:00:00 2001 From: sophia Date: Thu, 26 Mar 2020 15:44:16 -0400 Subject: [PATCH 1/8] Yield output based on ui opts --- lib/vagrant/ui.rb | 8 +++++++- lib/vagrant/util/curl_helper.rb | 7 ++++--- test/unit/vagrant/ui_test.rb | 12 ++++++++++++ 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/lib/vagrant/ui.rb b/lib/vagrant/ui.rb index ee3547fc8..2a8c8de05 100644 --- a/lib/vagrant/ui.rb +++ b/lib/vagrant/ui.rb @@ -32,7 +32,7 @@ module Vagrant def initialize @logger = Log4r::Logger.new("vagrant::ui::interface") - @opts = {} + @opts = { :show_progress => true } @stdin = $stdin @stdout = $stdout @@ -75,6 +75,12 @@ module Vagrant def machine(type, *data) @logger.info("Machine: #{type} #{data.inspect}") end + + def output_if_showing_progress + if @opts[:show_progress] == true + yield self + end + end end # This is a UI implementation that does nothing. diff --git a/lib/vagrant/util/curl_helper.rb b/lib/vagrant/util/curl_helper.rb index 67def5a82..07afa3e70 100644 --- a/lib/vagrant/util/curl_helper.rb +++ b/lib/vagrant/util/curl_helper.rb @@ -82,10 +82,11 @@ module Vagrant # 9 - Time spent # 10 - Time left # 11 - Current speed - output = "Progress: #{columns[0]}% (Rate: #{columns[11]}/s, Estimated time remaining: #{columns[10]})" - ui.clear_line - ui.detail(output, new_line: false) + ui.output_if_showing_progress do |ui| + ui.clear_line + ui.detail(output, new_line: false) + end end end diff --git a/test/unit/vagrant/ui_test.rb b/test/unit/vagrant/ui_test.rb index 5cc1722d8..e84871078 100644 --- a/test/unit/vagrant/ui_test.rb +++ b/test/unit/vagrant/ui_test.rb @@ -110,6 +110,18 @@ describe Vagrant::UI::Basic do subject.detail(output) end end + + context "#output_if_showing_progress" do + it "yields an output" do + subject.opts[:show_progress] = true + expect { |b| subject.output_if_showing_progress(&b) }.to yield_control + end + + it "does not yield an output" do + subject.opts[:show_progress] = false + expect { |b| subject.output_if_showing_progress(&b) }.to_not yield_control + end + end end describe Vagrant::UI::Colored do From d6d6e2c36200b7401caacc841e472c497163ded0 Mon Sep 17 00:00:00 2001 From: sophia Date: Thu, 26 Mar 2020 17:04:33 -0400 Subject: [PATCH 2/8] Surface cli option to quiet progress --- bin/vagrant | 6 ++++++ lib/vagrant/ui.rb | 9 +++++++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/bin/vagrant b/bin/vagrant index 5f6725b50..042def12b 100755 --- a/bin/vagrant +++ b/bin/vagrant @@ -78,6 +78,12 @@ if argv.include?("--debug-timestamp") ENV["VAGRANT_LOG_TIMESTAMP"] = "1" end +# Setting to enable/disable showing progress bars +if argv.include?("--quiet-progress") + argv.delete("--quiet-progress") + ENV["VAGRANT_QUIET_PROGRESS"] = "1" +end + # Stdout/stderr should not buffer output $stdout.sync = true $stderr.sync = true diff --git a/lib/vagrant/ui.rb b/lib/vagrant/ui.rb index 2a8c8de05..2a7f5c06f 100644 --- a/lib/vagrant/ui.rb +++ b/lib/vagrant/ui.rb @@ -31,9 +31,14 @@ module Vagrant attr_accessor :stderr def initialize - @logger = Log4r::Logger.new("vagrant::ui::interface") - @opts = { :show_progress => true } + if ENV["VAGRANT_QUIET_PROGRESS"] + show_progress = false + else + show_progress = true + end + @logger = Log4r::Logger.new("vagrant::ui::interface") + @opts = { :show_progress => show_progress } @stdin = $stdin @stdout = $stdout @stderr = $stderr From baaaf8e7e6d16f480316e5a074ee6f6a202b326b Mon Sep 17 00:00:00 2001 From: sophia Date: Thu, 26 Mar 2020 17:28:19 -0400 Subject: [PATCH 3/8] Output report progress if show progress is enabled --- lib/vagrant/ui.rb | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/lib/vagrant/ui.rb b/lib/vagrant/ui.rb index 2a7f5c06f..5812ebd50 100644 --- a/lib/vagrant/ui.rb +++ b/lib/vagrant/ui.rb @@ -211,15 +211,17 @@ module Vagrant # to the UI. Send `clear_line` to clear the line to show # a continuous progress meter. def report_progress(progress, total, show_parts=true) - if total && total > 0 - percent = (progress.to_f / total.to_f) * 100 - line = "Progress: #{percent.to_i}%" - line << " (#{progress} / #{total})" if show_parts - else - line = "Progress: #{progress}" - end + if @opts[:show_progress] == true + if total && total > 0 + percent = (progress.to_f / total.to_f) * 100 + line = "Progress: #{percent.to_i}%" + line << " (#{progress} / #{total})" if show_parts + else + line = "Progress: #{progress}" + end - info(line, new_line: false) + info(line, new_line: false) + end end def clear_line @@ -301,7 +303,11 @@ module Vagrant [:clear_line, :report_progress].each do |method| # By default do nothing, these aren't formatted - define_method(method) { |*args| @ui.send(method, *args) } + define_method(method) do |*args| + @ui.output_if_showing_progress do |ui| + ui.send(method, *args) + end + end end # For machine-readable output, set the prefix in the From 428fad9291c8763f61b52d398d04b682a19868aa Mon Sep 17 00:00:00 2001 From: sophia Date: Thu, 26 Mar 2020 17:31:13 -0400 Subject: [PATCH 4/8] "Test `-quiet-progress` flag"` --- test/unit/bin/vagrant_test.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/unit/bin/vagrant_test.rb b/test/unit/bin/vagrant_test.rb index 42245631b..aafaed475 100644 --- a/test/unit/bin/vagrant_test.rb +++ b/test/unit/bin/vagrant_test.rb @@ -91,6 +91,14 @@ describe "vagrant bin" do with(hash_including(ui_class: Vagrant::UI::Colored)) end end + + describe "--quiet-progress" do + let(:argv) { ["--quiet-progress"] } + + it "should enable less verbose progress output" do + expect(ENV).to receive(:[]=).with("VAGRANT_QUIET_PROGRESS", "1") + end + end end context "default CLI flags" do From d2302720625f25759666bb648743e4ed168c701e Mon Sep 17 00:00:00 2001 From: sophia Date: Fri, 27 Mar 2020 12:09:25 -0400 Subject: [PATCH 5/8] Add NonInteractive UI --- lib/vagrant/ui.rb | 13 +++++++++++++ test/unit/vagrant/ui_test.rb | 17 +++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/lib/vagrant/ui.rb b/lib/vagrant/ui.rb index 5812ebd50..884ddcd08 100644 --- a/lib/vagrant/ui.rb +++ b/lib/vagrant/ui.rb @@ -265,6 +265,19 @@ module Vagrant end end + + class NonInteractive < Basic + def initialize + super + @opts = { :show_progress => false } + end + + def ask(*args) + # Non interactive can't ask for input + raise Errors::UIExpectsTTY + end + end + # Prefixed wraps an existing UI and adds a prefix to it. class Prefixed < Interface # The prefix for `output` messages. diff --git a/test/unit/vagrant/ui_test.rb b/test/unit/vagrant/ui_test.rb index e84871078..b94aa28a5 100644 --- a/test/unit/vagrant/ui_test.rb +++ b/test/unit/vagrant/ui_test.rb @@ -124,6 +124,23 @@ describe Vagrant::UI::Basic do end end +describe Vagrant::UI::NonInteractive do + # let(:ui) { Vagrant::UI::NonInteractive.new } + + describe "#ask" do + it "raises an exception" do + expect{subject.ask("foo")}.to raise_error(Vagrant::Errors::UIExpectsTTY) + end + end + + describe "#report_progress" do + it "does not output progress" do + expect(subject).to_not receive(:info) + subject.report_progress(1, 1) + end + end +end + describe Vagrant::UI::Colored do include_context "unit" From 0296e59baff5f74df30c3a658492d1d916326911 Mon Sep 17 00:00:00 2001 From: sophia Date: Mon, 30 Mar 2020 09:48:44 -0400 Subject: [PATCH 6/8] Use :ui_class opt to set ui class --- bin/vagrant | 13 +++++++------ lib/vagrant/ui.rb | 8 +------- test/unit/bin/vagrant_test.rb | 7 ++++--- 3 files changed, 12 insertions(+), 16 deletions(-) diff --git a/bin/vagrant b/bin/vagrant index 042def12b..ba7e40076 100755 --- a/bin/vagrant +++ b/bin/vagrant @@ -78,12 +78,6 @@ if argv.include?("--debug-timestamp") ENV["VAGRANT_LOG_TIMESTAMP"] = "1" end -# Setting to enable/disable showing progress bars -if argv.include?("--quiet-progress") - argv.delete("--quiet-progress") - ENV["VAGRANT_QUIET_PROGRESS"] = "1" -end - # Stdout/stderr should not buffer output $stdout.sync = true $stderr.sync = true @@ -114,6 +108,7 @@ begin o.on("--debug", "Enable debug output") o.on("--timestamp", "Enable timestamps on log output") o.on("--debug-timestamp", "Enable debug output with timestamps") + o.on("--no-tty", "Enable non-interactive output") }) # Create a logger right away @@ -152,6 +147,12 @@ begin opts[:ui_class] = Vagrant::UI::MachineReadable end + # Setting to enable/disable showing progress bars + if argv.include?("--no-tty") + argv.delete("--no-tty") + opts[:ui_class] = Vagrant::UI::NonInteractive + end + # Default to colored output opts[:ui_class] ||= Vagrant::UI::Colored diff --git a/lib/vagrant/ui.rb b/lib/vagrant/ui.rb index 884ddcd08..3301b8f2b 100644 --- a/lib/vagrant/ui.rb +++ b/lib/vagrant/ui.rb @@ -31,14 +31,8 @@ module Vagrant attr_accessor :stderr def initialize - if ENV["VAGRANT_QUIET_PROGRESS"] - show_progress = false - else - show_progress = true - end - @logger = Log4r::Logger.new("vagrant::ui::interface") - @opts = { :show_progress => show_progress } + @opts = {} @stdin = $stdin @stdout = $stdout @stderr = $stderr diff --git a/test/unit/bin/vagrant_test.rb b/test/unit/bin/vagrant_test.rb index aafaed475..bc11309aa 100644 --- a/test/unit/bin/vagrant_test.rb +++ b/test/unit/bin/vagrant_test.rb @@ -92,11 +92,12 @@ describe "vagrant bin" do end end - describe "--quiet-progress" do - let(:argv) { ["--quiet-progress"] } + describe "--no-tty" do + let(:argv) { ["--no-tty"] } it "should enable less verbose progress output" do - expect(ENV).to receive(:[]=).with("VAGRANT_QUIET_PROGRESS", "1") + expect(Vagrant::Environment).to receive(:new). + with(hash_including(ui_class: Vagrant::UI::NonInteractive)) end end end From e6c387cdcedc9fcc2fbfff8f03449d43f6fbed6a Mon Sep 17 00:00:00 2001 From: sophia Date: Mon, 30 Mar 2020 10:03:49 -0400 Subject: [PATCH 7/8] Refactor non interactive UI --- lib/vagrant/ui.rb | 50 +++++++++++++++++++++------------ lib/vagrant/util/curl_helper.rb | 2 +- test/unit/vagrant/ui_test.rb | 20 ++++++------- 3 files changed, 42 insertions(+), 30 deletions(-) diff --git a/lib/vagrant/ui.rb b/lib/vagrant/ui.rb index 3301b8f2b..aa2acf374 100644 --- a/lib/vagrant/ui.rb +++ b/lib/vagrant/ui.rb @@ -33,6 +33,7 @@ module Vagrant def initialize @logger = Log4r::Logger.new("vagrant::ui::interface") @opts = {} + @stdin = $stdin @stdout = $stdout @stderr = $stderr @@ -75,10 +76,8 @@ module Vagrant @logger.info("Machine: #{type} #{data.inspect}") end - def output_if_showing_progress - if @opts[:show_progress] == true - yield self - end + def rewriting + yield self end end @@ -205,17 +204,15 @@ module Vagrant # to the UI. Send `clear_line` to clear the line to show # a continuous progress meter. def report_progress(progress, total, show_parts=true) - if @opts[:show_progress] == true - if total && total > 0 - percent = (progress.to_f / total.to_f) * 100 - line = "Progress: #{percent.to_i}%" - line << " (#{progress} / #{total})" if show_parts - else - line = "Progress: #{progress}" - end - - info(line, new_line: false) + if total && total > 0 + percent = (progress.to_f / total.to_f) * 100 + line = "Progress: #{percent.to_i}%" + line << " (#{progress} / #{total})" if show_parts + else + line = "Progress: #{progress}" end + + info(line, new_line: false) end def clear_line @@ -263,7 +260,19 @@ module Vagrant class NonInteractive < Basic def initialize super - @opts = { :show_progress => false } + end + + def rewriting + # no-op + end + + def report_progress(progress, total, show_parts=true) + # no-op + end + + def clear_line + @logger.warn("Using `clear line` in a non interactive ui") + say(:info, "\n", opts) end def ask(*args) @@ -311,9 +320,7 @@ module Vagrant [:clear_line, :report_progress].each do |method| # By default do nothing, these aren't formatted define_method(method) do |*args| - @ui.output_if_showing_progress do |ui| - ui.send(method, *args) - end + @ui.send(method, *args) end end @@ -368,6 +375,13 @@ module Vagrant "#{prefix}#{target} #{line}" end.join("\n") end + + def rewriting + @ui.rewriting do |ui| + yield ui + end + end + end # This is a UI implementation that outputs color for various types diff --git a/lib/vagrant/util/curl_helper.rb b/lib/vagrant/util/curl_helper.rb index 07afa3e70..67b772b54 100644 --- a/lib/vagrant/util/curl_helper.rb +++ b/lib/vagrant/util/curl_helper.rb @@ -83,7 +83,7 @@ module Vagrant # 10 - Time left # 11 - Current speed output = "Progress: #{columns[0]}% (Rate: #{columns[11]}/s, Estimated time remaining: #{columns[10]})" - ui.output_if_showing_progress do |ui| + ui.rewriting do |ui| ui.clear_line ui.detail(output, new_line: false) end diff --git a/test/unit/vagrant/ui_test.rb b/test/unit/vagrant/ui_test.rb index b94aa28a5..4d1ded16e 100644 --- a/test/unit/vagrant/ui_test.rb +++ b/test/unit/vagrant/ui_test.rb @@ -111,22 +111,14 @@ describe Vagrant::UI::Basic do end end - context "#output_if_showing_progress" do - it "yields an output" do - subject.opts[:show_progress] = true - expect { |b| subject.output_if_showing_progress(&b) }.to yield_control - end - - it "does not yield an output" do - subject.opts[:show_progress] = false - expect { |b| subject.output_if_showing_progress(&b) }.to_not yield_control + context "#rewriting" do + it "does output progress" do + expect { |b| subject.rewriting(&b) }.to yield_control end end end describe Vagrant::UI::NonInteractive do - # let(:ui) { Vagrant::UI::NonInteractive.new } - describe "#ask" do it "raises an exception" do expect{subject.ask("foo")}.to raise_error(Vagrant::Errors::UIExpectsTTY) @@ -139,6 +131,12 @@ describe Vagrant::UI::NonInteractive do subject.report_progress(1, 1) end end + + describe "#rewriting" do + it "does not output progress" do + expect { |b| subject.rewriting(&b) }.to_not yield_control + end + end end describe Vagrant::UI::Colored do From 39d7cd8997a97198d67a26ce2f383f97d3d319b4 Mon Sep 17 00:00:00 2001 From: sophia Date: Fri, 3 Apr 2020 15:51:35 -0400 Subject: [PATCH 8/8] Wrap all progress type outputs with rewriting to enable --no-tty --- lib/vagrant/ui.rb | 3 +++ lib/vagrant/util/curl_helper.rb | 6 ++++-- plugins/providers/hyperv/action/export.rb | 6 ++++-- plugins/providers/virtualbox/action/export.rb | 6 ++++-- plugins/providers/virtualbox/action/import.rb | 12 ++++++++---- .../virtualbox/action/prepare_clone_snapshot.rb | 6 ++++-- .../providers/virtualbox/action/snapshot_delete.rb | 6 ++++-- .../providers/virtualbox/action/snapshot_restore.rb | 6 ++++-- test/unit/vagrant/util/downloader_test.rb | 1 + 9 files changed, 36 insertions(+), 16 deletions(-) diff --git a/lib/vagrant/ui.rb b/lib/vagrant/ui.rb index aa2acf374..a521fd105 100644 --- a/lib/vagrant/ui.rb +++ b/lib/vagrant/ui.rb @@ -76,6 +76,9 @@ module Vagrant @logger.info("Machine: #{type} #{data.inspect}") end + # Yields self (UI) + # Provides a way for selectively displaying or not displaying + # updating content like download progress. def rewriting yield self end diff --git a/lib/vagrant/util/curl_helper.rb b/lib/vagrant/util/curl_helper.rb index 67b772b54..8c3d205d9 100644 --- a/lib/vagrant/util/curl_helper.rb +++ b/lib/vagrant/util/curl_helper.rb @@ -37,8 +37,10 @@ module Vagrant source_host = source_uri.host.to_s.split(".", 2).last location_host = location_uri.host.to_s.split(".", 2).last if !redirect_notify && location_host != source_host && !SILENCED_HOSTS.include?(location_host) - ui.clear_line - ui.detail "Download redirected to host: #{location_uri.host}" + ui.rewriting do |ui| + ui.clear_line + ui.detail "Download redirected to host: #{location_uri.host}" + end end redirect_notify = true end diff --git a/plugins/providers/hyperv/action/export.rb b/plugins/providers/hyperv/action/export.rb index 28b5ab4be..34a540409 100644 --- a/plugins/providers/hyperv/action/export.rb +++ b/plugins/providers/hyperv/action/export.rb @@ -25,8 +25,10 @@ module VagrantPlugins @env[:ui].info I18n.t("vagrant.actions.vm.export.exporting") export_tmp_dir = Vagrant::Util::Platform.wsl_to_windows_path(@env["export.temp_dir"]) @env[:machine].provider.driver.export(export_tmp_dir) do |progress| - @env[:ui].clear_line - @env[:ui].report_progress(progress.percent, 100, false) + @env[:ui].rewriting do |ui| + ui.clear_line + ui.report_progress(progress.percent, 100, false) + end end # Clear the line a final time so the next data can appear diff --git a/plugins/providers/virtualbox/action/export.rb b/plugins/providers/virtualbox/action/export.rb index f9132c3c4..51c1621f3 100644 --- a/plugins/providers/virtualbox/action/export.rb +++ b/plugins/providers/virtualbox/action/export.rb @@ -23,8 +23,10 @@ module VagrantPlugins def export @env[:ui].info I18n.t("vagrant.actions.vm.export.exporting") @env[:machine].provider.driver.export(ovf_path) do |progress| - @env[:ui].clear_line - @env[:ui].report_progress(progress.percent, 100, false) + @env[:ui].rewriting do |ui| + ui.clear_line + ui.report_progress(progress.percent, 100, false) + end end # Clear the line a final time so the next data can appear diff --git a/plugins/providers/virtualbox/action/import.rb b/plugins/providers/virtualbox/action/import.rb index fb540f50a..704814b61 100644 --- a/plugins/providers/virtualbox/action/import.rb +++ b/plugins/providers/virtualbox/action/import.rb @@ -19,8 +19,10 @@ module VagrantPlugins env[:ui].info I18n.t("vagrant.actions.vm.clone.creating") env[:machine].id = env[:machine].provider.driver.clonevm( env[:clone_id], env[:clone_snapshot]) do |progress| - env[:ui].clear_line - env[:ui].report_progress(progress, 100, false) + env[:ui].rewriting do |ui| + ui.clear_line + ui.report_progress(progress, 100, false) + end end # Clear the line one last time since the progress meter doesn't @@ -51,8 +53,10 @@ module VagrantPlugins # Import the virtual machine ovf_file = env[:machine].box.directory.join("box.ovf").to_s id = env[:machine].provider.driver.import(ovf_file) do |progress| - env[:ui].clear_line - env[:ui].report_progress(progress, 100, false) + env[:ui].rewriting do |ui| + ui.clear_line + ui.report_progress(progress, 100, false) + end end # Set the machine ID diff --git a/plugins/providers/virtualbox/action/prepare_clone_snapshot.rb b/plugins/providers/virtualbox/action/prepare_clone_snapshot.rb index 08ce16a6d..653e61a91 100644 --- a/plugins/providers/virtualbox/action/prepare_clone_snapshot.rb +++ b/plugins/providers/virtualbox/action/prepare_clone_snapshot.rb @@ -55,8 +55,10 @@ module VagrantPlugins @logger.info("Creating base snapshot for master VM.") env[:machine].provider.driver.create_snapshot( env[:clone_id], name) do |progress| - env[:ui].clear_line - env[:ui].report_progress(progress, 100, false) + env[:ui].rewriting do |ui| + ui.clear_line + ui.report_progress(progress, 100, false) + end end end end diff --git a/plugins/providers/virtualbox/action/snapshot_delete.rb b/plugins/providers/virtualbox/action/snapshot_delete.rb index 1d8cecc73..74fac9440 100644 --- a/plugins/providers/virtualbox/action/snapshot_delete.rb +++ b/plugins/providers/virtualbox/action/snapshot_delete.rb @@ -12,8 +12,10 @@ module VagrantPlugins name: env[:snapshot_name])) env[:machine].provider.driver.delete_snapshot( env[:machine].id, env[:snapshot_name]) do |progress| - env[:ui].clear_line - env[:ui].report_progress(progress, 100, false) + env[:ui].rewriting do |ui| + ui.clear_line + ui.report_progress(progress, 100, false) + end end # Clear the line one last time since the progress meter doesn't disappear diff --git a/plugins/providers/virtualbox/action/snapshot_restore.rb b/plugins/providers/virtualbox/action/snapshot_restore.rb index f655471c1..922d51d84 100644 --- a/plugins/providers/virtualbox/action/snapshot_restore.rb +++ b/plugins/providers/virtualbox/action/snapshot_restore.rb @@ -12,8 +12,10 @@ module VagrantPlugins name: env[:snapshot_name])) env[:machine].provider.driver.restore_snapshot( env[:machine].id, env[:snapshot_name]) do |progress| - env[:ui].clear_line - env[:ui].report_progress(progress, 100, false) + env[:ui].rewriting do |ui| + ui.clear_line + ui.report_progress(progress, 100, false) + end end # Clear the line one last time since the progress meter doesn't disappear diff --git a/test/unit/vagrant/util/downloader_test.rb b/test/unit/vagrant/util/downloader_test.rb index 0aa29f5fd..973510259 100644 --- a/test/unit/vagrant/util/downloader_test.rb +++ b/test/unit/vagrant/util/downloader_test.rb @@ -38,6 +38,7 @@ describe Vagrant::Util::Downloader do before do allow(ui).to receive(:clear_line) allow(ui).to receive(:detail) + allow(ui).to receive(:rewriting).and_yield(ui) end after do