Fix unicorn pid auto-detection behavior

[k2nr]

• run auto-detection on remote
• convert relative path to absolute
• use unicorn -e instead of unicorn -c with Tempfile

This fixes #85

[sfsekaran]

Awesome. @aspiers @sosedoff Do either of you have a deployment on which to test this fix? I know we can't test everything out, but just a basic gut-check would be great.

[aspiers]

I'm looking at it - please don't merge yet.

[k2nr]

this works perfectly with my rails app.

[aspiers]

I'm not keen on this - why did you remove the error logging?

[k2nr]

Ah, it's just mistake. I will revert it.

[aspiers]

Thanks for the work on this but IMHO it needs more work before merging. One core problem is that the commit message does not explain the motivation for the changes it lists, which makes it hard for us to collaboratively arrive at a consensus for the right fix, leaving us in guesswork territory. I'd much prefer to know that we definitely fixed the issue in the right way, rather than rely on empirical "well it seems to work OK for me" data.

[k2nr]

Thanks for your feedback and sorry for my less explanation. Let me explain what this pull request tries to fix and why.

pid auto-detection in v0.2.0 doesn't work in some situation.

auto-detection runs on local environment

As you know the auto-detection runs on local machine in v0.2.0. This causes some problems:

If pid is specified likepid File.expand_path('tmp/pids/unicorn.pid').to_s this script run on local and the expanded path may be different from remote.

To fix the case above, auto-detection should run on server-side.

Relative path

If relative path is specified to pid, v0.2.0 causes error. this is what #85 reported.
The root cause is that duplicate_unicorn(and start, stop) doesn't run at correct path.

To fix this relative path problem, there're two approaches I think:

1. If relative pid path is extracted, expand to absolute path before run duplicate_unicorn
2. Run cd #{app_path} before execute duplicate_unicorn

I picked the first approach because I think it's more readable when reading log.

I rebase and add more detailed commit logs when the discussion reach conclusion.

[aspiers]

Thanks a lot for the explanation and replies! This is beginning to make more sense now, but we're not there yet. For example one of the issues (as I already commented) is failure to find the unicorn config file. And I'm worried about this previous comment but I can't remember the exact details.

[aspiers]

I picked the first approach because I think it's more readable when reading log.

Please could you give some examples of this?

[amw]

I ran into the same problem. My unicorn config uses Dir.home to set app_path and sets pid file path using concatenation. Like pid "#{app_path}/tmp/pids/unicorn.pid". So the official capistrano-unicorn release tries to deploy with these incorrect paths (this is from cap unicorn:show_vars):

# Absolute paths
app_path                  "/mnt/data/users/production/my-project/current"
unicorn_pid               "/Users/amw/my-project/current/tmp/pids/unicorn.pid"

I used k2nr version and it was promising, but in his commits he doesn't run unicorn -e through bundle exec and unicorn is not available as a global command on my deploy servers.

So I've rebased k2nr commits on top of current master and added a quick fix. You can review it here amw/capistrano-unicorn@1bd81f3.

Now I get a correct output from show_vars and deploy works again.

# Absolute paths
app_path                  "/mnt/data/users/production/my-project/current"
unicorn_pid               "/mnt/data/users/production/my-project/current/tmp/pids/unicorn.pid"

Note that I can't simply change my pid definition to use relative path like pid "tmp/pids/unicorn.pid", because capistrano-unicorn will not expand it properly. Deploy will fail trying to run commands like

if [ -e tmp/pids/unicorn.pid ] &&  kill -0 `cat tmp/pids/unicorn.pid` > /dev/null 2>&1;

when not in a proper working directory of the app. I believe checking pid path on the server is well worth the second request and tad slower deploy (additional hit to the server and additional bundle exec call). It makes the gem more bulletproof and saves adding some caveats to documentation.