Merge pull request #5104 from mitchellh/sethvargo/ftp_push_fixes

Various fixes for FTP

plugins/pushes/ftp/adapter.rb

@@ -1,3 +1,5 @@
+require "pathname"
+
 module VagrantPlugins
   module FTPPush
     class Adapter
@@ -50,7 +52,7 @@ module VagrantPlugins
       end
 
       def default_port
-        20
+        21
       end
 
       def connect(&block)
@@ -68,15 +70,24 @@ module VagrantPlugins
 
       def upload(local, remote)
         parent   = File.dirname(remote)
+        fullpath = Pathname.new(File.expand_path(parent, pwd))
 
-        # Create the parent directory if it does not exist
-        if !@server.list("/").any? { |f| f.start_with?(parent) }
-          @server.mkdir(parent)
+        # Create the parent directories if they does not exist (naive mkdir -p)
+        fullpath.descend do |path|
+          if @server.list(path.to_s).empty?
+            @server.mkdir(path.to_s)
+          end
         end
 
         # Upload the file
         @server.putbinaryfile(local, remote)
       end
+
+      private
+
+      def pwd
+        @pwd ||= @server.pwd
+      end
     end
 
     #

plugins/pushes/ftp/push.rb

@@ -7,6 +7,12 @@ module VagrantPlugins
   module FTPPush
     class Push < Vagrant.plugin("2", :push)
       IGNORED_FILES = %w(. ..).freeze
+      DEFAULT_EXCLUDES = %w(.git .hg .svn .vagrant).freeze
+
+      def initialize(*)
+        super
+        @logger = Log4r::Logger.new("vagrant::pushes::ftp")
+      end
 
       def push
         # Grab files early so if there's an exception or issue, we don't have to
@@ -14,11 +20,16 @@ module VagrantPlugins
         files = Hash[*all_files.flat_map do |file|
           relative_path = relative_path_for(file, config.dir)
           destination = File.expand_path(File.join(config.destination, relative_path))
+          file = File.expand_path(file, env.root_path)
           [file, destination]
         end]
 
+        ftp = "#{config.username}@#{config.host}:#{config.destination}"
+        env.ui.info "Uploading #{env.root_path} to #{ftp}"
+
         connect do |ftp|
           files.each do |local, remote|
+            @logger.info "Uploading #{local} => #{remote}"
             ftp.upload(local, remote)
           end
         end
@@ -33,16 +44,6 @@ module VagrantPlugins
         ftp.connect(&block)
       end
 
-      # Parse the host into it's url and port parts.
-      # @return [Array]
-      def parse_host(host)
-        if host.include?(":")
-          host.split(":", 2)
-        else
-          [host, "22"]
-        end
-      end
-
       # The list of all files that should be pushed by this push. This method
       # only returns **files**, not folders or symlinks!
       # @return [Array<String>]
@@ -72,7 +73,10 @@ module VagrantPlugins
       # @param [Array<String>] excludes
       #   the exclude patterns or files
       def filter_excludes!(list, excludes)
-        excludes = Array(excludes).flat_map { |e| [e, "#{e}/*"] }
+        excludes = Array(excludes)
+        excludes = excludes + DEFAULT_EXCLUDES
+        excludes = excludes.flat_map { |e| [e, "#{e}/*"] }
+
         list.reject! do |file|
           basename = relative_path_for(file, config.dir)
 

test/unit/plugins/pushes/ftp/adapter_test.rb

@@ -54,8 +54,8 @@ describe VagrantPlugins::FTPPush::FTPAdapter do
   end
 
   describe "#default_port" do
-    it "is 20" do
-      expect(subject.default_port).to eq(20)
+    it "is 21" do
+      expect(subject.default_port).to eq(21)
     end
   end
 

test/unit/plugins/pushes/ftp/push_test.rb

@@ -20,6 +20,11 @@ describe VagrantPlugins::FTPPush::Push do
 
   subject { described_class.new(env, config) }
 
+  before do
+    allow(env).to receive(:root_path)
+      .and_return(File.expand_path("..", __FILE__))
+  end
+
   describe "#push" do
     before(:all) do
       @server = FakeFtp::Server.new(51234, 21213)
@@ -93,34 +98,6 @@ describe VagrantPlugins::FTPPush::Push do
     end
   end
 
-  describe "#parse_host" do
-    let(:result) { subject.parse_host(host) }
-
-    context "when no port is given" do
-      let(:host) { "127.0.0.1" }
-
-      it "returns the url and port 22" do
-        expect(result).to eq(["127.0.0.1", "22"])
-      end
-    end
-
-    context "when a port is given" do
-      let(:host) { "127.0.0.1:23456" }
-
-      it "returns the url and port 23456" do
-        expect(result).to eq(["127.0.0.1", "23456"])
-      end
-    end
-
-    context "when more than more port is given" do
-      let(:host) { "127.0.0.1:22:33:44" }
-
-      it "returns the url and everything after" do
-        expect(result).to eq(["127.0.0.1", "22:33:44"])
-      end
-    end
-  end
-
   describe "#all_files" do
     before(:all) do
       @dir = Dir.mktmpdir