Issue #1: Fix handling of default options

[webflo]

Hi,

i think i found the bug. The handling of default options is wrong. Because of the order the options are loaded rsync.rb overwrites all options that are already defined. Look at https://github.com/capistrano/capistrano/blob/master/lib/capistrano/setup.rb

Whats why :rsync_options is always en empty array and the rsync command in :rsync will fail.

Please note i am not a ruby developer and this is completely wrong.

[moll]

Thank for your troubleshooting this further! I'll take a look at it ASAP! Perhaps something had changed since Capistrano's prerelease that I didn't test properly.

Looking at Capistrano's setup.rb reminded me of https://github.com/capistrano/capistrano/issues/631 again. Jeez. They're using Rake in a totally backwards way. :unamused:

[webflo]

@moll The bug appear only if you deploy in a complete cold env. No current, release, shared folder etc.

[seantanly]

@poxrud I believe this pull request works because as per the README, require "capistrano/rsync" is added in our Capfile which loads the rsync.rb. Thus, it enhances the load:defaults task before it is invoked.

However require "capistrano/rsync" should be added after require 'capistrano/deploy' instead of right at the top.

[seantanly]

Just to add, I have success in setting up default settings in my capistrano tasks (lib/capistrano/tasks/*.cap) by simply enhancing the load:defaults task, and the settings can be overwritten in deploy.rb or config/deploy/<env>.rb

For example, inside my_task.cap

namespace :load do
  task :defaults do
    set :mytask_default, 'some_value'
  end
end

I believe it works on the same principles here.

[poxrud]

@seantan Yes you are correct it works if you put the line require "capistrano/rsync" inside the Capfile right below the require 'capistrano/deploy' line.

However the README mentions:

Require it at the top of your Capfile (or config/deploy.rb):

Which is incorrect. Putting it right at the top of Capfile or inside config/deploy.rb will not work. I still prefer my fix because it does not have these restrictions but yes this pull request works as well provided the README is made more clear.

[seantanly]

I guess it is a matter of preference.

My preference is to go with require 'capistrano/rsync' which follows the general convention of requiring external gem support below require 'capistrano/deploy'.

Take for example, require 'capistrano/bundler' which is generated by default in Capfile. Looking at the capistrano-bundler gem, it's setting up its defaults in a similar fashion.

https://github.com/capistrano/bundler/blob/master/lib/capistrano/tasks/bundler.cap

I agree with you the README needs to be fixed.

[poxrud]

@seantan agreed, the defaults should go into the defaults task. The README should also be updated accordingly. Hopefully the changes will be merged into master soon.

[STRML]

I've done a similar PR in #13 that uses the now-supported load:defaults task. I had found that it was impossible for me to include require 'capistrano/rsync' before require 'capistrano/deploy' as it would scream about deploy:check not being defined. With require 'capistrano/async' in deploy.rb or in the Capfile, it works great.

[seantanly]

@moll Don't mind me nagging... is there any chance we can get the fixes pulled into master soon? We're already getting 3 similar PRs that fixes the same show-stopper problem reported 4 months ago. Can the gem get some love? :)

[aganov]

@STRML @moll Can we use Kernel.system rsync.join(" ") instead of sending an array to system, because now, when I'm trying to send -e with string - for example %w[-e 'ssh\ -p\ 2222'] it's ending with an error rsync: Failed to exec ssh -p 2222. Sending a string works just fine...

PS: Also I think it will be better to use run_locally instead of Kernel.system

[poxrud]

@moll another reminder to please merge the fixes. I use capistrano-rsync to deploy my wordpress sites and would like to to recommend this workflow to my colleagues. However I cannot do so until this gem is fixed.

[moll]

@poxrud: Thank you for poking me! I will get to this eventually! It's just that Capistrano was not a good piece of software when I built this and I've since moved on — hence the delay.

How do you use Capistrano to deploy Wordpress sites, if I may ask? Is it the best tool for that? :)

@aganov: I'll have to see what run_locally does, but Kernel.system with a string will run it through the shell — there's no benefit in that, only risk that things will start behaving weirdly because of how special characters interact with the shell.

Seems to me, what you're after, is to pass two arguments to Rsync: -e and ssh -p 2222. The %w[] construct in Ruby creates an array from space separated words and does not care for backslashes. When you create the array "by hand" with something like ["-e", "ssh -p 2222"], I'm guessing, it should work for you. ;-)

[poxrud]

@moll I'm not sure if it's the best tool for wordpress deployment but it's a great tool.

I keep my wordpress sites in a local git repo, and then when I want to deploy it's just a simple

cap production deploy

I keep the common files/directories such as wp-config.php, wp-content, nginx.conf inside the shared directory, with symlinks inside the current directory.

Works great!