-
-
Notifications
You must be signed in to change notification settings - Fork 2k
capistrano: use :release_path, not :current_release #1264
Conversation
This commit fixes the following generated command where a bogus `releases/ls` directory is assumed due to the use of :current_path. failed: "sh -c 'cd MY_DEPLOY_DIR/releases/ls && bundle install --gemfile MY_DEPLOY_DIR/releases/ls/Gemfile --path MY_DEPLOY_DIR/shared/bundle --deployment --quiet --without development test'" on MY_HOST_NAME I am using capistrano (2.6.0) and bundler (1.0.15) under ruby 1.9.2p180.
Thanks for the patch! Because Capistrano's docs say that :current_path is the correct variable to use, we are going to stick with that. Back when I was writing and testing bundler/capistrano, :release_path is not always the desired directory. If you need :current_path to work differently, please send a pull request to Capistrano. Alternatively, you could just overwrite the Bundler cap task yourself. You already wrote the code! :) |
Alright, I filed this as an issue on capistrano. |
@indirect, the problem here is not current_path, but current_release. Capistrano assigns current_release with the last file(directory) name in the releases folder. So if you have a folder created (by mistake in most cases) which happens to be the last one entry via On deployment, most other tasks (create links, compile assets, etc) use release_path while bundler uses current_release. The result is the new deployed folder doesn't have a proper .bundle folder, thus the app would fail to start. |
Last time I checked, Capistrano's maintainers said the correct variable to use was current_path. I am reluctant to go against their suggestion, since I want Bundler to keep working with future versions of Capistrano. |
Fair enough. I'll report back if I experience any problems without this patch, in which case I'll try ask the bundler devs to clarify which variable they recommend to use in this case. Cheers. |
+1 on the patch release_path is the most reliable variable because it points to exactly where the release is being put. the current_release you're using is causing problems for us because we have other folders in the release dir (I know, not very 'capistrano'-y, but it's the nature of the system we're dealing with). Since current_release appears to use magic (i.e. ls -x) to figure out which release is current, I don't know why it's even used at all in this case. release_path is specifically the path we want, I think. |
@skwp please let us know if Capistrano's maintainers start suggesting release_path over current_path. Until then, I've already explained why Bundler's built in recipe is the way it is. If you need something else, please copy the recipe and change the single line you need to change. The entirety of the capistrano integration is only a few lines of code anyway. |
As this plugin's maintainer you have absolute rights to merge or not merge a patch :) of course, it is easy for me to copy the recipe. I'm just wondering where you got the idea that they suggest using current_release instead of release_path (btw in your reply, I believe you confused 'current_path' with 'current_release' - they are not the same thing). I'd also like to know for my own education why release_path is a bad thing to use - it is after all exactly the path to the current deploy. |
The original patch author also confused it in his commit message. If you look at his patch, he changed 'current_release' to 'release_path' and in the comments he wrote 'current_path'. This may be adding more than the needed level of confusion to this already confusing topic :) |
Heh. My bad, I did in fact mean current_release. Like I said earlier in this ticket, if there is somewhere in the Cap docs (or it has otherwise been stated by someone on Cap-core) that recommends release_path over current_release, please let me know so that we can change it. Thanks. |
@indirect here's some stuff i found from a few mins of googling. it's not definitive, but it may be worth a look in this thread Jamis explains: release_path is recomputed every time Capistrano is invoked, and is only valid for a deploy that is IN PROGRESS in this thread, jamis explains that current_release changes throughout the capistrano deploy process, since it's filesystem based. before deploy it points to the latest current release, and after the new release is there. so I don't know why you would use it, since you have to know when you're calling it to ensure it's not the 'wrong' thing some more resources.. engine yard - uses release_path, does not mention current_release some stats that may be useful.. "capistrano current_release" google search 6,910 results If your capistrano bundler plugin is meant for use during deploys, I would think release_path is the most consistent way to access the deploy currently in progress rather than relying on filesystem |
Firstly, we of course know exactly when we are calling it, because we insert it into the deploy ourselves: This script is supposed to be the "easy version". If you use Cap in a default way, this just works. If you don't use Cap in a default way, all bets are off, and unfortunately I don't have time to provide support for situations like that. Since there is no clear indication that either one is "right", and the current version is known to work in both Cap and Vlad, I think we should leave things the way they are for right now. |
+1 for your research @skwp, I agree that release_path is the most consistent way to access the current deployment. I've run into this issue also (where current_release is being set to the wrong directory on initial deployments) and fixed it for our apps by overriding current_release to be release_path in deploy.rb:
|
@shevaun hah, what a clever hack, i didn't think of that :) |
I mostly agree with this pull request. I think that I have searched the Capistrano documentation and I don't see anywhere that they suggest using I say that with one caveat. If we want to be able to run the As far as Vlad is concerned, it has the same logic for each of these variables as Capistrano does (see here). To sum it all up, I think the |
I'm willing to try this change, but worried about breaking backwards compatibility with deploy scripts that expect |
fwiw, I'm using my own |
@indirect As I am "Capistrano's Maintainers" - I don't recall ever having had that conversation with you. I've only now been made aware of this farse because of a pull request at Capistrano. Please sort your house out, the Bundler integration with Capistrano is a constant source of support queries that I have to deal with, because it's poorly documented, and poorly implemented. |
@leehambley I apologize for not consulting with you directly. I wasn't aware that you are the only maintainer of capistrano, and frankly have no idea how I could have found that out, since it isn't documented anywhere that I was able to find when I looked. In another discussion of bundler/capistrano, a long time ago, someone with a sizable number of commits to capistrano suggested using You spent some time talking about how this "farse" means I should sort my house out, but you didn't mention what the correct variable to use is. Is it As for "poorly documented, and poorly implemented", please let me know (either in a public ticket or privately to my personal email) any way that we can possibly improve it. I would love to get some feedback containing actual suggestions from someone who knows capistrano well. Thanks. |
You could have approached our community via our mailing list and/or Github (my name is all over our commit history. I don't know exactly what the Capistrano Bundler integration is doing, and to be honest, a lot of the problems that come from new installations come from To offer something useful, in order to bundle gems for the current deployment, I would personally recommend using
I'm picking specifically on the small, un-link-to-able note about the cap-bundler integration page on the Bundler docs, I would recommend that this be a real page, or at least have an anchor so that I can refer people to that heading without issuing a general RTFM for the Bundler docs, which is the best I can do now. As for implementation, I'm referring to this issue. Anyway, @indirect I'm sorry for my over-reaction - we're all friends here on It's annoyed me for a long time that neither Carl nor Yehuda approached me to discuss ways of integrating Bundler more tightly with Capistrano, and this just piqued my pre-existing frustration. |
I think that it still makes sense to use |
I'm not sure why you would bundle (or use Cap for any ad-hoc commands) outside of a deployment, that's generally a bad practice but @dontangg is right. (Apart from using Cap for general administration being an anti-pattern) |
Thanks for the suggestions. We currently hook into I'm also making the change to @leehambley, I apologize for the lack of communication with you. I made the decision to accept Capistrano integration into Bundler back when Carl and Yehuda were still working on it, and I've since taken over as primary maintainer (along with Terence). It seemed like a good idea at the time to provide some easy integration between the two, and we tried to add copious documentation to the capistrano task itself. I completely understand the frustration of not being able to link to the right place in the Bundler docs, and I will fix that on the documentation site as soon as possible. If you have any other thoughts or ideas about integration between Bundler and Capistrano, I would truly love to hear them. As I said before, I don't use cap much myself, so input from someone who is knowledgable about it would be greatly appreciated. Thanks! |
Works great for me! Thanks! |
Just installed bundler from master and my original problem is gone, thanks @indirect :) |
Awesome, thanks everyone! These changes will be part of the Bundler 1.2 release, and will be available from Rubygems as soon as we push the 1.2.0.pre.1 gem. |
switch the bundler hook to before "deploy:finalize_update", so that finalize_update will not run if `bundle install` fails. (thanks @leehambley) change from `current_release` to `latest_release`, because both cap and vlad provide more robust directory-finding code behind `latest_release`. (thanks @dontangg) closes rubygems#1264
switch the bundler hook to before "deploy:finalize_update", so that finalize_update will not run if `bundle install` fails. (thanks @leehambley) change from `current_release` to `latest_release`, because both cap and vlad provide more robust directory-finding code behind `latest_release`. (thanks @dontangg) closes rubygems#1264
@indirect Good call on going with @leehambley Bundler's capistrano tasks are actually some of the most well documented 3rd party tasks in the Capistrano ecosystem, rivaling the documentation provided for the core deploy tasks. Also I cannot believe that I'm hearing you call ad-hoc commands or using Capistrano's task system an antipattern. This is contradicted right in the gem description ("Capistrano is a utility and framework for executing commands in parallel on multiple remote machines, via SSH.") and how many, including myself, use the project you now maintain. The tool is designed for this and this is how I use it. Preventing this type of use is like like having a rake task that only worked if triggered by the default task. In fact, I use the @sunaku Why would you have a @skwp Did you seriously call |
This commit fixes the following generated command where a bogus
releases/ls
directory is assumed due to the use of:current_release
.I am using capistrano (2.6.0) and bundler (1.0.15) under ruby 1.9.2p180.