Merge pull request #10529 from chrisroberts/e-rsync-chown

Add support for using the `--chown` flag with rsync when available.

plugins/synced_folders/rsync/command/rsync.rb

@@ -15,6 +15,7 @@ module VagrantPlugins
         end
 
         def execute
+          options = {}
           opts = OptionParser.new do |o|
             o.banner = "Usage: vagrant rsync [vm-name]"
             o.separator ""
@@ -23,6 +24,9 @@ module VagrantPlugins
             o.separator ""
             o.separator "Options:"
             o.separator ""
+            o.on("--[no-]rsync-chown", "Use rsync to modify ownership") do |chown|
+              options[:rsync_chown] = chown
+            end
           end
 
           # Parse the options and return if we don't have any target.
@@ -59,6 +63,9 @@ module VagrantPlugins
 
             # Sync them!
             folders.each do |id, folder_opts|
+              if options.has_key?(:rsync_chown)
+                folder_opts = folder_opts.merge(rsync_ownership: options[:rsync_chown])
+              end
               RsyncHelper.rsync_single(machine, ssh_info, folder_opts)
             end
           end

plugins/synced_folders/rsync/command/rsync_auto.rb

@@ -8,11 +8,6 @@ require "vagrant/util/platform"
 
 require_relative "../helper"
 
-# This is to avoid a bug in nio 1.0.0. Remove around nio 1.0.1
-if Vagrant::Util::Platform.windows?
-  ENV["NIO4R_PURE"] = "1"
-end
-
 require "listen"
 
 module VagrantPlugins
@@ -38,6 +33,10 @@ module VagrantPlugins
             o.on("--[no-]poll", "Force polling filesystem (slow)") do |poll|
               options[:poll] = poll
             end
+
+            o.on("--[no-]rsync-chown", "Use rsync to modify ownership") do |chown|
+              options[:rsync_chown] = chown
+            end
           end
 
           # Parse the options and return if we don't have any target.
@@ -88,6 +87,9 @@ module VagrantPlugins
                 machine.ui.info(I18n.t("vagrant.rsync_auto_remove_folder",
                                     folder: folder_opts[:hostpath]))
               else
+                if options.has_key?(:rsync_chown)
+                  folder_opts = folder_opts.merge(rsync_ownership: options[:rsync_chown])
+                end
                 sync_folders[id] = folder_opts
               end
             end

plugins/synced_folders/rsync/helper.rb

@@ -10,6 +10,9 @@ module VagrantPlugins
     # This is a helper that abstracts out the functionality of rsyncing
     # folders so that it can be called from anywhere.
     class RsyncHelper
+      # rsync version requirement to support chown argument
+      RSYNC_CHOWN_REQUIREMENT = Gem::Requirement.new(">= 3.1.0").freeze
+
       # This converts an rsync exclude pattern to a regular expression
       # we can send to Listen.
       def self.exclude_to_regexp(path, exclude)
@@ -24,7 +27,7 @@ module VagrantPlugins
         regexp = "^#{Regexp.escape(path)}"
         regexp += ".*" if !start_anchor
 
-        # This is REALLY ghetto, but its a start. We can improve and
+        # This is not an ideal solution, but it's a start. We can improve and
         # keep unit tests passing in the future.
         exclude = exclude.gsub("**", "|||GLOBAL|||")
         exclude = exclude.gsub("*", "|||PATH|||")
@@ -140,11 +143,18 @@ module VagrantPlugins
           args << "--no-perms" if args.include?("--archive") || args.include?("-a")
         end
 
-        # Disable rsync's owner/group preservation (implied by --archive) unless
+        if opts[:rsync_ownership] && rsync_chown_support?(machine)
-        # specifically requested, since we adjust owner/group to match shared
+          # Allow rsync to map ownership
-        # folder setting ourselves.
+          args << "--chown=#{opts[:owner]}:#{opts[:group]}"
-        args << "--no-owner" unless args.include?("--owner") || args.include?("-o")
+          # Notify rsync post capability not to chown
-        args << "--no-group" unless args.include?("--group") || args.include?("-g")
+          opts[:chown] = false
+        else
+          # Disable rsync's owner/group preservation (implied by --archive) unless
+          # specifically requested, since we adjust owner/group to match shared
+          # folder setting ourselves.
+          args << "--no-owner" unless args.include?("--owner") || args.include?("-o")
+          args << "--no-group" unless args.include?("--group") || args.include?("-g")
+        end
 
         # Tell local rsync how to invoke remote rsync with sudo
         rsync_path = opts[:rsync_path]
@@ -223,6 +233,55 @@ module VagrantPlugins
           end
         end
       end
+
+      # Check if rsync versions support using chown option
+      #
+      # @param [Vagrant::Machine] machine The remote machine
+      # @return [Boolean]
+      def self.rsync_chown_support?(machine)
+        if !RSYNC_CHOWN_REQUIREMENT.satisfied_by?(Gem::Version.new(local_rsync_version))
+          return false
+        end
+        mrv = machine_rsync_version(machine)
+        if mrv && !RSYNC_CHOWN_REQUIREMENT.satisfied_by?(Gem::Version.new(mrv))
+          return false
+        end
+        true
+      end
+
+      # @return [String, nil] version of remote rsync
+      def self.machine_rsync_version(machine)
+        if machine.guest.capability?(:rsync_command)
+          rsync_path = machine.guest.capability(:rsync_command)
+        else
+          rsync_path = "rsync"
+        end
+        output = ""
+        machine.communicate.execute(rsync_path + " --version") { |_, data| output << data }
+        vmatch = output.match(/version\s+(?<version>[\d.]+)\s/)
+        if vmatch
+          vmatch[:version]
+        end
+      end
+
+      # @return [String, nil] version of local rsync
+      def self.local_rsync_version
+        if !@_rsync_version
+          r = Vagrant::Util::Subprocess.execute("rsync", "--version")
+          vmatch = r.stdout.to_s.match(/version\s+(?<version>[\d.]+)\s/)
+          if vmatch
+            @_rsync_version = vmatch[:version]
+          end
+        end
+        @_rsync_version
+      end
+
+      # @private
+      # Reset the cached values for helper. This is not considered a public
+      # API and should only be used for testing.
+      def self.reset!
+        instance_variables.each(&method(:remove_instance_variable))
+      end
     end
   end
 end

test/unit/plugins/synced_folders/rsync/command/rsync_auto_test.rb

@@ -15,17 +15,17 @@ describe VagrantPlugins::SyncedFolderRSync::Command::RsyncAuto do
 
   let(:synced_folders_empty) { {} }
   let(:synced_folders_dupe) { {"1234":
-                               {type: "rsync",
+    {type: "rsync",
-                                exclude: false,
+      exclude: false,
-                                hostpath: "/Users/brian/code/vagrant-sandbox"},
+      hostpath: "/Users/brian/code/vagrant-sandbox"},
-                               "5678":
+    "5678":
-                               {type: "rsync",
+    {type: "rsync",
-                                exclude: false,
+      exclude: false,
-                                hostpath: "/Not/The/Same/Path"},
+      hostpath: "/Not/The/Same/Path"},
-                               "0912":
+    "0912":
-                               {type: "rsync",
+    {type: "rsync",
-                                exclude: false,
+      exclude: false,
-                                hostpath: "/Users/brian/code/relative-dir"}}}
+      hostpath: "/Users/brian/code/relative-dir"}}}
 
   let(:helper_class) { VagrantPlugins::SyncedFolderRSync::RsyncHelper }
 
@@ -65,23 +65,20 @@ describe VagrantPlugins::SyncedFolderRSync::Command::RsyncAuto do
     # For reference:
     # https://github.com/hashicorp/vagrant/blob/9c1b014536e61b332cfaa00774a87a240cce8ed9/lib/vagrant/action/builtin/synced_folders.rb#L45-L46
     let(:config_synced_folders)  { {"/vagrant":
-                                   {type: "rsync",
+      {type: "rsync",
-                                    exclude: false,
+        exclude: false,
-                                    hostpath: "/Users/brian/code/vagrant-sandbox"},
+        hostpath: "/Users/brian/code/vagrant-sandbox"},
-                                  "/vagrant/other-dir":
+      "/vagrant/other-dir":
-                                   {type: "rsync",
+      {type: "rsync",
-                                    exclude: false,
+        exclude: false,
-                                    hostpath: "/Users/brian/code/vagrant-sandbox/other-dir"},
+        hostpath: "/Users/brian/code/vagrant-sandbox/other-dir"},
-                                  "/vagrant/relative-dir":
+      "/vagrant/relative-dir":
-                                   {type: "rsync",
+      {type: "rsync",
-                                    exclude: false,
+        exclude: false,
-                                    hostpath: "/Users/brian/code/relative-dir"}}}
+        hostpath: "/Users/brian/code/relative-dir"}}}
 
     before do
-        allow(subject).to receive(:with_target_vms) { |&block| block.call machine }
+      allow(subject).to receive(:with_target_vms) { |&block| block.call machine }
-    end
-
-    it "does not sync folders outside of the cwd" do
       allow(machine.ui).to receive(:info)
       allow(machine.state).to receive(:id).and_return(:created)
       allow(machine.env).to receive(:cwd).
@@ -95,8 +92,9 @@ describe VagrantPlugins::SyncedFolderRSync::Command::RsyncAuto do
       allow(helper_class).to receive(:rsync_single).and_return(true)
       allow(Vagrant::Util::Busy).to receive(:busy).and_return(true)
       allow(Listen).to receive(:to).and_return(true)
+    end
 
-
+    it "does not sync folders outside of the cwd" do
       expect(machine.ui).to receive(:info).
         with("Not syncing /Not/The/Same/Path as it is not part of the current working directory.")
       expect(machine.ui).to receive(:info).
@@ -105,7 +103,17 @@ describe VagrantPlugins::SyncedFolderRSync::Command::RsyncAuto do
         with("Watching: /Users/brian/code/relative-dir")
       expect(helper_class).to receive(:rsync_single)
 
-      subject.execute()
+      subject.execute
+    end
+
+    context "with --rsync-chown option" do
+      let(:argv) { ["--rsync-chown"] }
+
+      it "should enable rsync_ownership on folder options" do
+        expect(helper_class).to receive(:rsync_single).
+          with(anything, anything, hash_including(rsync_ownership: true))
+        subject.execute
+      end
     end
   end
 

test/unit/plugins/synced_folders/rsync/command/rsync_test.rb

@@ -67,6 +67,20 @@ describe VagrantPlugins::SyncedFolderRSync::Command::Rsync do
 
         expect(subject.execute).to eql(0)
       end
+
+      context "with --rsync-chown option" do
+        let(:argv) { ["--rsync-chown"] }
+
+        it "should enable rsync_ownership on folder options" do
+          synced_folders[:rsync].each do |_, opts|
+            expect(helper_class).to receive(:rsync_single).
+              with(machine, ssh_info, hash_including(rsync_ownership: true)).
+              ordered
+          end
+
+          subject.execute
+        end
+      end
     end
   end
 end

test/unit/plugins/synced_folders/rsync/helper_test.rb

@@ -169,6 +169,60 @@ describe VagrantPlugins::SyncedFolderRSync::RsyncHelper do
       end
     end
 
+    context "with rsync_ownership option" do
+      let(:rsync_local_version) { "3.1.1" }
+      let(:rsync_remote_version) { "3.1.1" }
+      let(:rsync_result) { Vagrant::Util::Subprocess::Result.new(0, "", "") }
+
+      before do
+        expect(Vagrant::Util::Subprocess).to receive(:execute).
+          with("rsync", "--version").and_return(Vagrant::Util::Subprocess::Result.new(0, " version #{rsync_local_version} ", ""))
+        allow(machine.communicate).to receive(:execute).with(/--version/).and_yield(:stdout, " version #{rsync_remote_version} ")
+        allow(Vagrant::Util::Subprocess).to receive(:execute).with("rsync", any_args).and_return(rsync_result)
+        opts[:rsync_ownership] = true
+      end
+
+      after { subject.reset! }
+
+      it "should use the rsync --chown flag" do
+        expect(Vagrant::Util::Subprocess).to receive(:execute) { |*args|
+          expect(args.detect{|a| a.include?("--chown")}).to be_truthy
+          rsync_result
+        }
+        subject.rsync_single(machine, ssh_info, opts)
+      end
+
+      it "should set the chown option to false" do
+        expect(opts.has_key?(:chown)).to eq(false)
+        subject.rsync_single(machine, ssh_info, opts)
+        expect(opts[:chown]).to eq(false)
+      end
+
+      context "when local rsync version does not support --chown" do
+        let(:rsync_local_version) { "2.0" }
+
+        it "should not use the --chown flag" do
+          expect(Vagrant::Util::Subprocess).to receive(:execute) { |*args|
+            expect(args.detect{|a| a.include?("--chown")}).to be_falsey
+            rsync_result
+          }
+          subject.rsync_single(machine, ssh_info, opts)
+        end
+      end
+
+      context "when remote rsync version does not support --chown" do
+        let(:rsync_remote_version) { "2.0" }
+
+        it "should not use the --chown flag" do
+          expect(Vagrant::Util::Subprocess).to receive(:execute) { |*args|
+            expect(args.detect{|a| a.include?("--chown")}).to be_falsey
+            rsync_result
+          }
+          subject.rsync_single(machine, ssh_info, opts)
+        end
+      end
+    end
+
     context "excluding files" do
       it "excludes files if given as a string" do
         opts[:exclude] = "foo"
@@ -345,4 +399,141 @@ describe VagrantPlugins::SyncedFolderRSync::RsyncHelper do
       subject.rsync_single(machine, ssh_info, opts)
     end
   end
+
+  describe ".rsync_chown_support?" do
+    let(:local_version) { "3.1.1" }
+    let(:remote_version) { "3.1.1" }
+
+    before do
+      allow(subject).to receive(:local_rsync_version).and_return(local_version)
+      allow(subject).to receive(:machine_rsync_version).and_return(remote_version)
+    end
+
+    it "should return when local and remote versions support chown" do
+      expect(subject.rsync_chown_support?(machine)).to be_truthy
+    end
+
+    context "when local version does not support chown" do
+      let(:local_version) { "2.0" }
+
+      it "should return false" do
+        expect(subject.rsync_chown_support?(machine)).to be_falsey
+      end
+    end
+
+    context "when remote version does not support chown" do
+      let(:remote_version) { "2.0" }
+
+      it "should return false" do
+        expect(subject.rsync_chown_support?(machine)).to be_falsey
+      end
+    end
+
+    context "when both local and remote versions do not support chown" do
+      let(:local_version) { "2.0" }
+      let(:remote_version) { "2.0" }
+
+      it "should return false" do
+        expect(subject.rsync_chown_support?(machine)).to be_falsey
+      end
+    end
+  end
+
+  describe ".machine_rsync_version" do
+    let(:version_output) {
+      <<-EOV
+      rsync  version 3.1.3  protocol version 31
+      Copyright (C) 1996-2018 by Andrew Tridgell, Wayne Davison, and others.
+      Web site: http://rsync.samba.org/
+      Capabilities:
+      64-bit files, 64-bit inums, 64-bit timestamps, 64-bit long ints,
+      socketpairs, hardlinks, symlinks, IPv6, batchfiles, inplace,
+      append, ACLs, xattrs, iconv, symtimes, prealloc
+
+      rsync comes with ABSOLUTELY NO WARRANTY.  This is free software, and you
+      are welcome to redistribute it under certain conditions.  See the GNU
+      General Public Licence for details.
+      EOV
+    }
+
+    before do
+      allow(machine.communicate).to receive(:execute).with(/--version/).
+        and_yield(:stdout, version_output)
+      allow(guest).to receive(:capability?).and_return(false)
+    end
+
+    it "should extract the version string" do
+      expect(subject.machine_rsync_version(machine)).to eq("3.1.3")
+    end
+
+    context "when version output is an unknown format" do
+      let(:version_output) { "unknown" }
+
+      it "should return nil value" do
+        expect(subject.machine_rsync_version(machine)).to be_nil
+      end
+    end
+
+    context "with guest rsync_command capability" do
+      let(:rsync_path) { "custom_rsync" }
+
+      before do
+        allow(guest).to receive(:capability?).with(:rsync_command).
+          and_return(true)
+        allow(guest).to receive(:capability).with(:rsync_command).
+          and_return(rsync_path)
+      end
+
+      it "should use custom rsync_path" do
+        expect(machine.communicate).to receive(:execute).
+          with("#{rsync_path} --version").and_yield(:stdout, version_output)
+        subject.machine_rsync_version(machine)
+      end
+    end
+  end
+
+  describe ".local_rsync_version" do
+    let(:version_output) {
+      <<-EOV
+      rsync  version 3.1.3  protocol version 31
+      Copyright (C) 1996-2018 by Andrew Tridgell, Wayne Davison, and others.
+      Web site: http://rsync.samba.org/
+      Capabilities:
+      64-bit files, 64-bit inums, 64-bit timestamps, 64-bit long ints,
+      socketpairs, hardlinks, symlinks, IPv6, batchfiles, inplace,
+      append, ACLs, xattrs, iconv, symtimes, prealloc
+
+      rsync comes with ABSOLUTELY NO WARRANTY.  This is free software, and you
+      are welcome to redistribute it under certain conditions.  See the GNU
+      General Public Licence for details.
+      EOV
+    }
+    let(:result) { Vagrant::Util::Subprocess::Result.new(0, version_output, "") }
+
+    before do
+      allow(Vagrant::Util::Subprocess).to receive(:execute).with("rsync", "--version").
+        and_return(result)
+    end
+
+    after { subject.reset! }
+
+    it "should extract the version string" do
+      expect(subject.local_rsync_version).to eq("3.1.3")
+    end
+
+    it "should cache the version lookup" do
+      expect(Vagrant::Util::Subprocess).to receive(:execute).with("rsync", "--version").
+        and_return(result).once
+      expect(subject.local_rsync_version).to eq("3.1.3")
+      expect(subject.local_rsync_version).to eq("3.1.3")
+    end
+
+    context "when version output is an unknown format" do
+      let(:version_output) { "unknown" }
+
+      it "should return nil value" do
+        expect(subject.local_rsync_version).to be_nil
+      end
+    end
+  end
 end

website/source/docs/cli/rsync-auto.html.md

@@ -22,6 +22,10 @@ for filesystem changes, and does not simply poll the directory.
 
 ## Options
 
+* `--[no-]rsync-chown` - Use rsync to modify ownership of transferred files. Enabling
+    this option can result in faster completion due to a secondary process not being
+    required to update ownership. By default this is disabled.
+
 * `--[no-]poll` - Force Vagrant to watch for changes using filesystem
     polling instead of filesystem events. This is required for some filesystems
     that do not support events. Warning: enabling this will make `rsync-auto`

website/source/docs/cli/rsync.html.md

@@ -16,3 +16,9 @@ This command forces a re-sync of any
 Note that if you change any settings within the rsync synced folders such
 as exclude paths, you will need to `vagrant reload` before this command will
 pick up those changes.
+
+## Options
+
+* `--[no-]rsync-chown` - Use rsync to modify ownership of transferred files. Enabling
+    this option can result in faster completion due to a secondary process not being
+    required to update ownership. By default this is disabled.

website/source/docs/synced-folders/rsync.html.md

@@ -69,6 +69,10 @@ The rsync synced folder type accepts the following options:
   pattern. By default, the ".vagrant/" directory is excluded. We recommend
   excluding revision control directories such as ".git/" as well.
 
+* `rsync__rsync_ownership` (boolean) - If true, and rsync executables in use
+  are >= 3.1.0, then rsync will be used to set the owner and group instead
+  of a separate call to modify ownership. By default, this is false.
+
 * `rsync__rsync_path` (string) - The path on the remote host where rsync
   is and how it is executed. This is platform specific but defaults to
   "sudo rsync" for many guests.