-
Notifications
You must be signed in to change notification settings - Fork 97
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix Berkshelf integration #74
Conversation
sethvargo
commented
Jun 10, 2013
- Fixes knife spork promote with Berksfile #73
- Fixes #73
I lied - I think this might fix #73, but I don't have a reliable test case. Can someone point their gem 'knife-spork', github: 'sethvargo/knife-spork', branch: 'promote_berksfie' |
This is still broken, but works after patching runner.rb:104 (pass cookbook_name) and runner.rb:128 (cookbook_name as param): cookbook = load_from_chef(cookbook_name) || load_from_berkshelf(cookbook_name) || load_from_librarian(cookbook_name) and def load_from_librarian(name) |
@ziuchkovski yea... that would definitely be a bug. Updated |
Confirmed, your 4e15d59 commit works as expected. Thanks! |
@ziuchkovski you think you could provide me some feedback. I must be doing something wrong but I can't get it to work cc @sethvargo |
@sethvargo what's the status on this one? Looks like @scalp42 is still having issues per #73 |
@jonlives I'm not sure. For some reason, the berksfile arg is just coming in as a boolean instead of the stringed value. I have no idea why. |
Huh. will keep this one open then! |
yea. I haven't had time to dig into it yet |
I am not quite sure if this is the correct place to post this, since I encounter errors in both the master branch as in your promote_berksfile branch. In your branch, when I run:
I get:
Any idea what's going wrong here? As you can see I do not use the
I get:
I hope this helps you with finding a fix for this. I have no time left, but will see if I can put some more effort in finding this bug this weekend. |
I found out that the bugs I encountered were caused by the git plugin. Disabling this plugin solved the problem for me. It would be nice to fix the git plugin before merging this. |
I needed to promote a cookbook that was not part of the immediate Berksfile, but was in the Berksfile.lock as it was a dependency of a cookbook.
I added a pull request to this branch on sethvargo/knife-spork#1 which resolves the cookbooks from the Berksfile.lock instead of the Berksfile |
@sethvargo should we keep track somewhere that this can be solved differently in master?
Allow loading cookbooks from the Berksfile.lock file
@jonlives could you take a peek at this please? |
Awesome, will try and get this out for y'all early next week :) |
@RSO, @sethvargo could one of you please write a quick comment with what this has changed in the way Berkshelf is used? I don't use berkshelf locally to test - I assume this has now resolved the original issues? |
This PR added support for Berkshelf in knife-spork promote. What this means is that when a cookbook is not found locally, knife-spork will check if the file can be found in the Berksfile.lock file. I think it might be best to add some text to the README, since there are some gotcha's, for instance: the git option in knife-spork config isn't supported. I could see if I can find some time this week to update this PR, or maybe create another one for the git issue. |
Thanks @RSO :) If you have a mo to look at the git issue, that would be awesome :) |
@sethvargo, @RSO it looks like the issues with the git plugin lie in the Berkshelf::CachedCookbook object not having a root_dir method. There are a couple of ways we could tackle this.
Do either of you have any thoughts on this? I'm not a Berkshelf user so I'm hesitant to decree the fix :p |
@jonlives there should be methods on CachedCookbook to do what you need. What exactly are you trying to do? |
This part of the plugin starts with the root_dir of the cookbook object and figures out if it's a subdir of a git repo. If it is, it traverses the FS tree to find the repo root, and runs a git pull there. Same for if it's a submodule. For CachedCookbook, I basically need a method for determining where the cookbook lives :) |
Does .path or .cookbook_path not work? |
I haven't got berkshelf set up locally yet to test with - gives me a place to start looking though :) |
I tried changing the
And:
|
@RSO that looks like"path" will work then, but it's returning you a Pathname object, which doesn't have an include? method. I think what would work here is testing if the cookbook is a Berkshelf::CachedCookbook, and if it is then using cookbook.path.to_s. Converting it back to a string will allow us to use the include? method as normal. You able to have a look at this? If not, I can try and get Berkshelf set up locally to do some messing around. |
I'm on it |
Awesome thanks :) |
Commit submitted in sethvargo/knife-spork#2 |
Thanks @RSO - @sethvargo, if you could merge this into your branch and update this PR please, I'll get this tested at my end and into the new version. |
Use cookbook.path.to_s when the cookbook is a CachedCookbook
@jonlives done |
Lovely stuff, thanks! |
@sethvargo does this fix the original #73 referenced in this ticket? It fixes the git plugin issues, but do we still have the issue with the Berksfile coming in as a boolean? |
Ah damn, forgot about that one, I tried and #73 still bugs. Will look in to that |
@RSO thanks, would be awesome if we can FIX ALL THE THINGS :p |
Fixed in sethvargo/knife-spork#3 |
This commit fixes #73
@RSO merged |
You two are awesome! Thanks - will get this merged into the new release branch. Watch this space for 1.3.0 :p |
knife-spork-1.3.0 released, containing this feature :) |