From fb4e4320b2d2f8261855dfd9b8c8a5221db3c717 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Mon, 24 Oct 2016 10:06:59 -0700 Subject: [PATCH] Remove `set -e` usage for better shell compatibility --- plugins/guests/arch/cap/change_host_name.rb | 10 +++---- plugins/guests/arch/cap/configure_networks.rb | 11 ++++---- plugins/guests/arch/cap/nfs.rb | 6 ++--- plugins/guests/arch/cap/rsync.rb | 2 +- plugins/guests/atomic/cap/change_host_name.rb | 10 +++---- plugins/guests/bsd/cap/nfs.rb | 10 +++---- plugins/guests/bsd/cap/public_key.rb | 22 ++++++++-------- plugins/guests/darwin/cap/change_host_name.rb | 23 ++++++++-------- plugins/guests/debian/cap/change_host_name.rb | 3 --- .../guests/debian/cap/configure_networks.rb | 2 +- plugins/guests/debian/cap/nfs.rb | 2 +- plugins/guests/debian/cap/rsync.rb | 1 - plugins/guests/gentoo/cap/change_host_name.rb | 6 ++--- plugins/guests/linux/cap/nfs.rb | 26 +++++++++---------- plugins/guests/linux/cap/public_key.rb | 20 ++++++-------- plugins/guests/netbsd/cap/change_host_name.rb | 5 ++-- test/unit/plugins/guests/bsd/cap/nfs_test.rb | 11 ++++---- .../guests/linux/cap/mount_nfs_test.rb | 10 +++---- 18 files changed, 80 insertions(+), 100 deletions(-) diff --git a/plugins/guests/arch/cap/change_host_name.rb b/plugins/guests/arch/cap/change_host_name.rb index 0b10112e1..39bc5e586 100644 --- a/plugins/guests/arch/cap/change_host_name.rb +++ b/plugins/guests/arch/cap/change_host_name.rb @@ -8,18 +8,16 @@ module VagrantPlugins if !comm.test("hostname -f | grep '^#{name}$'", sudo: false) basename = name.split(".", 2)[0] comm.sudo <<-EOH.gsub(/^ {14}/, "") - set -e + # Remove comments and blank lines from /etc/hosts + sed -i'' -e 's/#.*$//' -e '/^$/d' /etc/hosts # Set hostname hostnamectl set-hostname '#{basename}' - # Remove comments and blank lines from /etc/hosts - sed -i'' -e 's/#.*$//' -e '/^$/d' /etc/hosts - # Prepend ourselves to /etc/hosts - grep -w '#{name}' /etc/hosts || { + test $? -eq 0 && (grep -w '#{name}' /etc/hosts || { sed -i'' '1i 127.0.0.1\\t#{name}\\t#{basename}' /etc/hosts - } + }) EOH end end diff --git a/plugins/guests/arch/cap/configure_networks.rb b/plugins/guests/arch/cap/configure_networks.rb index 2fa9c8017..0c748b3d8 100644 --- a/plugins/guests/arch/cap/configure_networks.rb +++ b/plugins/guests/arch/cap/configure_networks.rb @@ -12,10 +12,9 @@ module VagrantPlugins def self.configure_networks(machine, networks) comm = machine.communicate + commands = [] - commands = ["set -e"] interfaces = machine.guest.capability(:network_interfaces) - networks.each.with_index do |network, i| network[:device] = interfaces[network[:interface]] @@ -42,15 +41,15 @@ module VagrantPlugins commands << <<-EOH.gsub(/^ {14}/, '') # Configure #{network[:device]} - mv '#{remote_path}' '/etc/netctl/#{network[:device]}' - ip link set '#{network[:device]}' down - netctl restart '#{network[:device]}' + mv '#{remote_path}' '/etc/netctl/#{network[:device]}' && + ip link set '#{network[:device]}' down && + netctl restart '#{network[:device]}' && netctl enable '#{network[:device]}' EOH end # Run all the network modification commands in one communicator call. - comm.sudo(commands.join("\n")) + comm.sudo(commands.join(" && \n")) end end end diff --git a/plugins/guests/arch/cap/nfs.rb b/plugins/guests/arch/cap/nfs.rb index 116256ed2..5ac5dfe67 100644 --- a/plugins/guests/arch/cap/nfs.rb +++ b/plugins/guests/arch/cap/nfs.rb @@ -15,8 +15,7 @@ module VagrantPlugins # https://bbs.archlinux.org/viewtopic.php?id=193410 # comm.sudo <<-EOH.gsub(/^ {12}/, "") - set -e - systemctl enable rpcbind + systemctl enable rpcbind && systemctl start rpcbind EOH end @@ -24,8 +23,7 @@ module VagrantPlugins def self.nfs_client_install(machine) comm = machine.communicate comm.sudo <<-EOH.gsub(/^ {12}/, "") - set -e - pacman --noconfirm -Syy + pacman --noconfirm -Syy && pacman --noconfirm -S nfs-utils ntp EOH end diff --git a/plugins/guests/arch/cap/rsync.rb b/plugins/guests/arch/cap/rsync.rb index 99aefa208..991a19478 100644 --- a/plugins/guests/arch/cap/rsync.rb +++ b/plugins/guests/arch/cap/rsync.rb @@ -5,9 +5,9 @@ module VagrantPlugins def self.rsync_install(machine) comm = machine.communicate comm.sudo <<-EOH.gsub(/^ {12}/, '') - set -e pacman -Sy --noconfirm pacman -S --noconfirm rsync + exit $? EOH end end diff --git a/plugins/guests/atomic/cap/change_host_name.rb b/plugins/guests/atomic/cap/change_host_name.rb index 2c501c8dd..1833cf32a 100644 --- a/plugins/guests/atomic/cap/change_host_name.rb +++ b/plugins/guests/atomic/cap/change_host_name.rb @@ -8,18 +8,16 @@ module VagrantPlugins if !comm.test("hostname -f | grep '^#{name}$'", sudo: false) basename = name.split(".", 2)[0] comm.sudo <<-EOH.gsub(/^ {14}/, "") - set -e + # Remove comments and blank lines from /etc/hosts + sed -i'' -e 's/#.*$//' -e '/^$/d' /etc/hosts # Set hostname hostnamectl set-hostname '#{basename}' - # Remove comments and blank lines from /etc/hosts - sed -i'' -e 's/#.*$//' -e '/^$/d' /etc/hosts - # Prepend ourselves to /etc/hosts - grep -w '#{name}' /etc/hosts || { + test $? -eq 0 && (grep -w '#{name}' /etc/hosts || { sed -i'' '1i 127.0.0.1\\t#{name}\\t#{basename}' /etc/hosts - } + }) EOH end end diff --git a/plugins/guests/bsd/cap/nfs.rb b/plugins/guests/bsd/cap/nfs.rb index a54835e89..458ebcfbe 100644 --- a/plugins/guests/bsd/cap/nfs.rb +++ b/plugins/guests/bsd/cap/nfs.rb @@ -11,10 +11,8 @@ module VagrantPlugins def self.mount_nfs_folder(machine, ip, folders) comm = machine.communicate + # Mount each folder separately so we can retry. folders.each do |name, opts| - # Mount each folder separately so we can retry. - commands = ["set -e"] - # Shellescape the paths in case they do not have special characters. guest_path = Shellwords.escape(opts[:guestpath]) host_path = Shellwords.escape(opts[:hostpath]) @@ -29,14 +27,14 @@ module VagrantPlugins mount_opts = mount_opts.join(",") # Make the directory on the guest. - commands << "mkdir -p #{guest_path}" + machine.communicate.sudo("mkdir -p #{guest_path}") # Perform the mount operation. - commands << "/sbin/mount -t nfs -o '#{mount_opts}' #{ip}:#{host_path} #{guest_path}" + command = "/sbin/mount -t nfs -o '#{mount_opts}' #{ip}:#{host_path} #{guest_path}" # Run the command, raising a specific error. retryable(on: Vagrant::Errors::NFSMountFailed, tries: 3, sleep: 5) do - machine.communicate.sudo(commands.join("\n"), + machine.communicate.sudo(command, error_class: Vagrant::Errors::NFSMountFailed, shell: "sh", ) diff --git a/plugins/guests/bsd/cap/public_key.rb b/plugins/guests/bsd/cap/public_key.rb index 8e84c0204..4f25aa967 100644 --- a/plugins/guests/bsd/cap/public_key.rb +++ b/plugins/guests/bsd/cap/public_key.rb @@ -22,14 +22,13 @@ module VagrantPlugins # Use execute (not sudo) because we want to execute this as the SSH # user (which is "vagrant" by default). comm.execute <<-EOH.gsub(/^ {12}/, "") - set -e - mkdir -p ~/.ssh - chmod 0700 ~/.ssh - cat '#{remote_path}' >> ~/.ssh/authorized_keys - chmod 0600 ~/.ssh/authorized_keys - + chmod 0700 ~/.ssh && + cat '#{remote_path}' >> ~/.ssh/authorized_keys && + chmod 0600 ~/.ssh/authorized_keys + result=$? rm -f '#{remote_path}' + exit $result EOH end @@ -49,15 +48,16 @@ module VagrantPlugins # Use execute (not sudo) because we want to execute this as the SSH # user (which is "vagrant" by default). comm.execute <<-EOH.sub(/^ {12}/, "") - set -e - + result=0 if test -f ~/.ssh/authorized_keys; then - grep -v -x -f '#{remote_path}' ~/.ssh/authorized_keys > ~/.ssh/authorized_keys.tmp - mv ~/.ssh/authorized_keys.tmp ~/.ssh/authorized_keys - chmod 0600 ~/.ssh/authorized_keys + grep -v -x -f '#{remote_path}' ~/.ssh/authorized_keys > ~/.ssh/authorized_keys.tmp && + mv ~/.ssh/authorized_keys.tmp ~/.ssh/authorized_keys && + chmod 0600 ~/.ssh/authorized_keys + result=$? fi rm -f '#{remote_path}' + exit $result EOH end end diff --git a/plugins/guests/darwin/cap/change_host_name.rb b/plugins/guests/darwin/cap/change_host_name.rb index 5d5902d9e..03bc70840 100644 --- a/plugins/guests/darwin/cap/change_host_name.rb +++ b/plugins/guests/darwin/cap/change_host_name.rb @@ -8,16 +8,17 @@ module VagrantPlugins if !comm.test("hostname -f | grep '^#{name}$'", sudo: false) basename = name.split(".", 2)[0] - comm.sudo <<-EOH.gsub(/^ {14}/, '') - set -e - + # LocalHostName should not contain dots - it is used by Bonjour and + # visible through file sharing services. + comm.sudo <<-EOH.gsub(/^ */, '') # Set hostname - scutil --set ComputerName '#{name}' - scutil --set HostName '#{name}' - - # LocalHostName should not contain dots - it is used by Bonjour and - # visible through file sharing services. - scutil --set LocalHostName '#{basename}' + scutil --set ComputerName '#{name}' && + scutil --set HostName '#{name}' && + scutil --set LocalHostName '#{basename}' + result=$? + if [ $result -ne 0 ]; then + exit $result + fi hostname '#{name}' @@ -27,8 +28,8 @@ module VagrantPlugins # Prepend ourselves to /etc/hosts - sed on bsd is sad grep -w '#{name}' /etc/hosts || { - echo -e '127.0.0.1\\t#{name}\\t#{basename}' | cat - /etc/hosts > /tmp/tmp-hosts - mv /tmp/tmp-hosts /etc/hosts + echo -e '127.0.0.1\\t#{name}\\t#{basename}' | cat - /etc/hosts > /tmp/tmp-hosts && + mv /tmp/tmp-hosts /etc/hosts } EOH end diff --git a/plugins/guests/debian/cap/change_host_name.rb b/plugins/guests/debian/cap/change_host_name.rb index a1ce9195a..e496004ad 100644 --- a/plugins/guests/debian/cap/change_host_name.rb +++ b/plugins/guests/debian/cap/change_host_name.rb @@ -8,9 +8,6 @@ module VagrantPlugins if !comm.test("hostname -f | grep '^#{name}$'", sudo: false) basename = name.split(".", 2)[0] comm.sudo <<-EOH.gsub(/^ {14}/, '') - # Ensure exit on command error - set -e - # Set the hostname echo '#{basename}' > /etc/hostname hostname -F /etc/hostname diff --git a/plugins/guests/debian/cap/configure_networks.rb b/plugins/guests/debian/cap/configure_networks.rb index 2553516a8..263fe924a 100644 --- a/plugins/guests/debian/cap/configure_networks.rb +++ b/plugins/guests/debian/cap/configure_networks.rb @@ -11,7 +11,7 @@ module VagrantPlugins def self.configure_networks(machine, networks) comm = machine.communicate - commands = ["set -e"] + commands = [] entries = [] interfaces = machine.guest.capability(:network_interfaces) diff --git a/plugins/guests/debian/cap/nfs.rb b/plugins/guests/debian/cap/nfs.rb index 881469906..de9da6667 100644 --- a/plugins/guests/debian/cap/nfs.rb +++ b/plugins/guests/debian/cap/nfs.rb @@ -5,9 +5,9 @@ module VagrantPlugins def self.nfs_client_install(machine) comm = machine.communicate comm.sudo <<-EOH.gsub(/^ {12}/, '') - set -e apt-get -yqq update apt-get -yqq install nfs-common portmap + exit $? EOH end end diff --git a/plugins/guests/debian/cap/rsync.rb b/plugins/guests/debian/cap/rsync.rb index b503f8cf3..6fca5d4f1 100644 --- a/plugins/guests/debian/cap/rsync.rb +++ b/plugins/guests/debian/cap/rsync.rb @@ -5,7 +5,6 @@ module VagrantPlugins def self.rsync_install(machine) comm = machine.communicate comm.sudo <<-EOH.gsub(/^ {14}/, '') - set -e apt-get -yqq update apt-get -yqq install rsync EOH diff --git a/plugins/guests/gentoo/cap/change_host_name.rb b/plugins/guests/gentoo/cap/change_host_name.rb index 7d8ca3ac5..36377e077 100644 --- a/plugins/guests/gentoo/cap/change_host_name.rb +++ b/plugins/guests/gentoo/cap/change_host_name.rb @@ -8,8 +8,6 @@ module VagrantPlugins if !comm.test("hostname -f | grep '^#{name}$'", sudo: false) basename = name.split(".", 2)[0] comm.sudo <<-EOH.gsub(/^ {14}/, "") - set -e - # Set the hostname hostname '#{basename}' echo "hostname=#{basename}" > /etc/conf.d/hostname @@ -20,8 +18,8 @@ module VagrantPlugins # Prepend ourselves to /etc/hosts grep -w '#{name}' /etc/hosts || { - echo -e '127.0.0.1\\t#{name}\\t#{basename}' | cat - /etc/hosts > /tmp/tmp-hosts - mv /tmp/tmp-hosts /etc/hosts + echo -e '127.0.0.1\\t#{name}\\t#{basename}' | cat - /etc/hosts > /tmp/tmp-hosts && + mv /tmp/tmp-hosts /etc/hosts } EOH end diff --git a/plugins/guests/linux/cap/nfs.rb b/plugins/guests/linux/cap/nfs.rb index f017499aa..28e25c461 100644 --- a/plugins/guests/linux/cap/nfs.rb +++ b/plugins/guests/linux/cap/nfs.rb @@ -13,10 +13,8 @@ module VagrantPlugins def self.mount_nfs_folder(machine, ip, folders) comm = machine.communicate + # Mount each folder separately so we can retry. folders.each do |name, opts| - # Mount each folder separately so we can retry. - commands = ["set -e"] - # Shellescape the paths in case they do not have special characters. guest_path = Shellwords.escape(opts[:guestpath]) host_path = Shellwords.escape(opts[:hostpath]) @@ -30,22 +28,24 @@ module VagrantPlugins end mount_opts = mount_opts.join(",") - # Make the directory on the guest. - commands << "mkdir -p #{guest_path}" + machine.communicate.sudo("mkdir -p #{guest_path}") - # Perform the mount operation. - commands << "mount -o #{mount_opts} #{ip}:#{host_path} #{guest_path}" - - # Emit a mount event - commands << <<-EOH.gsub(/^ {14}/, '') - if test -x /sbin/initctl && command -v /sbin/init && /sbin/init --version | grep upstart; then - /sbin/initctl emit --no-wait vagrant-mounted MOUNTPOINT=#{guest_path} + # Perform the mount operation and emit mount event if applicable + command = <<-EOH.gsub(/^ */, '') + mount -o #{mount_opts} #{ip}:#{host_path} #{guest_path} + result=$? + if test $result -eq 0; then + if test -x /sbin/initctl && command -v /sbin/init && /sbin/init --version | grep upstart; then + /sbin/initctl emit --no-wait vagrant-mounted MOUNTPOINT=#{guest_path} + fi + else + exit $result fi EOH # Run the command, raising a specific error. retryable(on: Vagrant::Errors::NFSMountFailed, tries: 3, sleep: 5) do - machine.communicate.sudo(commands.join("\n"), + machine.communicate.sudo(command, error_class: Vagrant::Errors::NFSMountFailed, ) end diff --git a/plugins/guests/linux/cap/public_key.rb b/plugins/guests/linux/cap/public_key.rb index c75251dd3..fb8301cae 100644 --- a/plugins/guests/linux/cap/public_key.rb +++ b/plugins/guests/linux/cap/public_key.rb @@ -21,15 +21,13 @@ module VagrantPlugins # Use execute (not sudo) because we want to execute this as the SSH # user (which is "vagrant" by default). - comm.execute <<-EOH.gsub(/^ {12}/, "") - set -e - + comm.execute <<-EOH.gsub(/^ */, "") mkdir -p ~/.ssh chmod 0700 ~/.ssh - cat '#{remote_path}' >> ~/.ssh/authorized_keys - chmod 0600 ~/.ssh/authorized_keys - + cat '#{remote_path}' >> ~/.ssh/authorized_keys && chmod 0600 ~/.ssh/authorized_keys + result=$? rm -f '#{remote_path}' + exit $result EOH end @@ -48,16 +46,14 @@ module VagrantPlugins # Use execute (not sudo) because we want to execute this as the SSH # user (which is "vagrant" by default). - comm.execute <<-EOH.sub(/^ {12}/, "") - set -e - + comm.execute <<-EOH.sub(/^ */, "") if test -f ~/.ssh/authorized_keys; then grep -v -x -f '#{remote_path}' ~/.ssh/authorized_keys > ~/.ssh/authorized_keys.tmp - mv ~/.ssh/authorized_keys.tmp ~/.ssh/authorized_keys - chmod 0600 ~/.ssh/authorized_keys + mv ~/.ssh/authorized_keys.tmp ~/.ssh/authorized_keys && chmod 0600 ~/.ssh/authorized_keys + result=$? fi - rm -f '#{remote_path}' + exit $result EOH end end diff --git a/plugins/guests/netbsd/cap/change_host_name.rb b/plugins/guests/netbsd/cap/change_host_name.rb index 60a142399..226ee8d8b 100644 --- a/plugins/guests/netbsd/cap/change_host_name.rb +++ b/plugins/guests/netbsd/cap/change_host_name.rb @@ -5,9 +5,8 @@ module VagrantPlugins def self.change_host_name(machine, name) if !machine.communicate.test("hostname -s | grep '^#{name}$'") machine.communicate.sudo(< /tmp/rc.conf.vagrant_changehostname_#{name} -mv /tmp/rc.conf.vagrant_changehostname_#{name} /etc/rc.conf +sed -e 's/^hostname=.*$/hostname=#{name}/' /etc/rc.conf > /tmp/rc.conf.vagrant_changehostname_#{name} && +mv /tmp/rc.conf.vagrant_changehostname_#{name} /etc/rc.conf && hostname #{name} CMDS end diff --git a/test/unit/plugins/guests/bsd/cap/nfs_test.rb b/test/unit/plugins/guests/bsd/cap/nfs_test.rb index 66b1adde2..aec465b9d 100644 --- a/test/unit/plugins/guests/bsd/cap/nfs_test.rb +++ b/test/unit/plugins/guests/bsd/cap/nfs_test.rb @@ -31,10 +31,9 @@ describe "VagrantPlugins::GuestBSD::Cap::NFS" do } cap.mount_nfs_folder(machine, ip, folders) - expect(comm.received_commands[0]).to match(/set -e/) expect(comm.received_commands[0]).to match(/mkdir -p \/guest/) - expect(comm.received_commands[0]).to match(/mount -t nfs/) - expect(comm.received_commands[0]).to match(/1.2.3.4:\/host \/guest/) + expect(comm.received_commands[1]).to match(/mount -t nfs/) + expect(comm.received_commands[1]).to match(/1.2.3.4:\/host \/guest/) end it "mounts with options" do @@ -49,7 +48,7 @@ describe "VagrantPlugins::GuestBSD::Cap::NFS" do } cap.mount_nfs_folder(machine, ip, folders) - expect(comm.received_commands[0]).to match(/mount -t nfs -o 'nfsv2,mntudp,banana'/) + expect(comm.received_commands[1]).to match(/mount -t nfs -o 'nfsv2,mntudp,banana'/) end it "escapes host and guest paths" do @@ -61,8 +60,8 @@ describe "VagrantPlugins::GuestBSD::Cap::NFS" do } cap.mount_nfs_folder(machine, ip, folders) - expect(comm.received_commands[0]).to match(/host\\\'s/) - expect(comm.received_commands[0]).to match(/guest\\\ with\\\ spaces/) + expect(comm.received_commands[1]).to match(/host\\\'s/) + expect(comm.received_commands[1]).to match(/guest\\\ with\\\ spaces/) end end end diff --git a/test/unit/plugins/guests/linux/cap/mount_nfs_test.rb b/test/unit/plugins/guests/linux/cap/mount_nfs_test.rb index 5fcd55de4..7cec549f2 100644 --- a/test/unit/plugins/guests/linux/cap/mount_nfs_test.rb +++ b/test/unit/plugins/guests/linux/cap/mount_nfs_test.rb @@ -43,7 +43,7 @@ describe "VagrantPlugins::GuestLinux::Cap::MountNFS" do cap.mount_nfs_folder(machine, ip, folders) expect(comm.received_commands[0]).to match(/mkdir -p #{guestpath}/) - expect(comm.received_commands[0]).to match(/1.2.3.4:#{hostpath} #{guestpath}/) + expect(comm.received_commands[1]).to match(/1.2.3.4:#{hostpath} #{guestpath}/) end it "mounts with options" do @@ -58,7 +58,7 @@ describe "VagrantPlugins::GuestLinux::Cap::MountNFS" do } cap.mount_nfs_folder(machine, ip, folders) - expect(comm.received_commands[0]).to match(/mount -o vers=2,udp/) + expect(comm.received_commands[1]).to match(/mount -o vers=2,udp/) end it "emits an event" do @@ -71,7 +71,7 @@ describe "VagrantPlugins::GuestLinux::Cap::MountNFS" do } cap.mount_nfs_folder(machine, ip, folders) - expect(comm.received_commands[0]).to include( + expect(comm.received_commands[1]).to include( "/sbin/initctl emit --no-wait vagrant-mounted MOUNTPOINT=#{guestpath}") end @@ -84,8 +84,8 @@ describe "VagrantPlugins::GuestLinux::Cap::MountNFS" do } cap.mount_nfs_folder(machine, ip, folders) - expect(comm.received_commands[0]).to match(/host\\\'s/) - expect(comm.received_commands[0]).to match(/guest\\\ with\\\ spaces/) + expect(comm.received_commands[1]).to match(/host\\\'s/) + expect(comm.received_commands[1]).to match(/guest\\\ with\\\ spaces/) end end end