Merge pull request #7921 from chrisroberts/fix/key-fixups

Remove `set -e` usage for better shell compatibility

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

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]}'
+              mv '#{remote_path}' '/etc/netctl/#{network[:device]}' &&
-              ip link set '#{network[:device]}' down
+              ip link set '#{network[:device]}' down &&
-              netctl restart '#{network[:device]}'
+              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

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

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

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

plugins/guests/bsd/cap/nfs.rb

@@ -11,10 +11,8 @@ module VagrantPlugins
         def self.mount_nfs_folder(machine, ip, folders)
           comm = machine.communicate
 
-          folders.each do |name, opts|
           # Mount each folder separately so we can retry.
-            commands = ["set -e"]
+          folders.each do |name, opts|
-
             # 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",
               )

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
+            chmod 0700 ~/.ssh &&
-            cat '#{remote_path}' >> ~/.ssh/authorized_keys
+              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
+              grep -v -x -f '#{remote_path}' ~/.ssh/authorized_keys > ~/.ssh/authorized_keys.tmp &&
-              mv ~/.ssh/authorized_keys.tmp ~/.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

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
-
-              # 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.
+            comm.sudo <<-EOH.gsub(/^ */, '')
+              # Set hostname
+              scutil --set ComputerName '#{name}' &&
+                scutil --set HostName '#{name}' &&
                 scutil --set LocalHostName '#{basename}'
+              result=$?
+              if [ $result -ne 0 ]; then
+                exit $result
+              fi
 
               hostname '#{name}'
 
@@ -27,7 +28,7 @@ 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
+                echo -e '127.0.0.1\\t#{name}\\t#{basename}' | cat - /etc/hosts > /tmp/tmp-hosts &&
                   mv /tmp/tmp-hosts /etc/hosts
               }
             EOH

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

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)
 

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

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

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,7 +18,7 @@ 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
+                echo -e '127.0.0.1\\t#{name}\\t#{basename}' | cat - /etc/hosts > /tmp/tmp-hosts &&
                   mv /tmp/tmp-hosts /etc/hosts
               }
             EOH

plugins/guests/linux/cap/nfs.rb

@@ -13,10 +13,8 @@ module VagrantPlugins
         def self.mount_nfs_folder(machine, ip, folders)
           comm = machine.communicate
 
-          folders.each do |name, opts|
           # Mount each folder separately so we can retry.
-            commands = ["set -e"]
+          folders.each do |name, opts|
-
             # 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.
+            machine.communicate.sudo("mkdir -p #{guest_path}")
-            commands << "mkdir -p #{guest_path}"
 
-            # Perform the mount operation.
+            # Perform the mount operation and emit mount event if applicable
-            commands << "mount -o #{mount_opts} #{ip}:#{host_path} #{guest_path}"
+            command = <<-EOH.gsub(/^ */, '')
-
+              mount -o #{mount_opts} #{ip}:#{host_path} #{guest_path}
-            # Emit a mount event
+              result=$?
-            commands << <<-EOH.gsub(/^ {14}/, '')
+              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

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}/, "")
+          comm.execute <<-EOH.gsub(/^ */, "")
-            set -e
-
             mkdir -p ~/.ssh
             chmod 0700 ~/.ssh
-            cat '#{remote_path}' >> ~/.ssh/authorized_keys
+            cat '#{remote_path}' >> ~/.ssh/authorized_keys && chmod 0600 ~/.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}/, "")
+          comm.execute <<-EOH.sub(/^ */, "")
-            set -e
-
             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
+              mv ~/.ssh/authorized_keys.tmp ~/.ssh/authorized_keys && chmod 0600 ~/.ssh/authorized_keys
-              chmod 0600 ~/.ssh/authorized_keys
+              result=$?
             fi
-
             rm -f '#{remote_path}'
+            exit $result
           EOH
         end
       end

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(<<CMDS, {shell: "sh"})
-set -e
+sed -e 's/^hostname=.*$/hostname=#{name}/' /etc/rc.conf > /tmp/rc.conf.vagrant_changehostname_#{name} &&
-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 &&
-mv /tmp/rc.conf.vagrant_changehostname_#{name} /etc/rc.conf
 hostname #{name}
 CMDS
           end

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[1]).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(/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[1]).to match(/host\\\'s/)
-      expect(comm.received_commands[0]).to match(/guest\\\ with\\\ spaces/)
+      expect(comm.received_commands[1]).to match(/guest\\\ with\\\ spaces/)
     end
   end
 end

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[1]).to match(/host\\\'s/)
-      expect(comm.received_commands[0]).to match(/guest\\\ with\\\ spaces/)
+      expect(comm.received_commands[1]).to match(/guest\\\ with\\\ spaces/)
     end
   end
 end