Play nice with submodules

[nickmarden]

I ran into a problem where ironfan and knife-spork were not playing nice together. Specifically, the 'git submodule foreach git pull' command was failing thusly:

[nick@machine ironfan-homebase] (master)$ bundle exec knife spork bump mycookbook

Loaded config file /Users/nick/.chef/spork-config.yml...

Opening git repo /Users/nick/workspace/ironfan-homebase/

Pulling latest changes from git

Current branch master is up to date.

Pulling latest changes from git submodules (if any)

ERROR: Entering 'cookbooks/user'
Current branch master is up to date.
Entering 'notes'
Current branch master is up to date.
Entering 'vendor/ironfan-pantry'
You are not currently on a branch, so I cannot use any
'branch.<branchname>.merge' in your configuration file.
Please specify which branch you want to merge with on the command
line and try again (e.g. 'git pull <repository> <refspec>').
See git-pull(1) for details.

I think the attached patch fixes this, by using "git pull --recurse-submodules" to fetch submodule changes, and "git submodule update" to update the submodules that have been pulled.

Of course, I may have misunderstood something - feel free to let me know if that's the case.

[jonlives]

I need to have a little play around with this one to make sure it does what's expected - what we've always done to resolve the issue you saw above is do a one-time checkout of the master branch in the submodule directory so it plays nicely - I like the idea of not having to do this though! Will check it out today :)

[jonlives]

The main issue with this method of doing things is that using "git submodule update" takes you off the master branch (or whatever you've checked out) and into whatever branch was last committed to the "parent" repo - imho doing a one time checkout of the master branch into the submodule is the safest option.

I'm going to close off this issue as my feeling is that the current behavior produced more predictable results.

What I might look into by way of solving this more fully is allowing an option specifying of branch to check out for each submodule in the spork config. Watch this space!

[nickmarden]

Maybe I'm missing something, but I think that "git submodule update", by design, updates the submodule(s) to whichever version the repo maintainer chose to use. That's often not the master branch, but something else instead. Ignoring that information and pulling the master version of each submodule can lead to Very Bad Results when the parent repo maintainer has gone out of their way to specify which submodule hashes are desired.

See for example ironfan-homebase, which is careful tested and maintained with submodule pointers to (e.g.) vendor/ironfan-pantry and vendor/opscode. These choices of submodule hashes are not master but rather specific SHAs that represent versions that have been tested and given the thumbs-up.

[jonlives]

I can see your point with that one - if the submodule maintainer decides to switch to a different branch, currently you'd have to manually switch. Conversely, the issue we have is that some of our cookbooks are actively maintained submodules...using the current spork behaviour, if someone forgets to commit the submodule SHA to the repo, it shows as uncommitted changes in the main repo. Using git submodule update, you can end up with one cookbook version uploaded to the chef server, but the git repo still pointing to the old SHA, which imho is a worse situation to be in.

I think what I might do for the next version is make this a configurable option in the config file...you can choose whether to use the existing method by default, or optionally use submodule update instead. Sound good?

[nickmarden]

I think a spork configuration option would be a fine solution to the problem. Thanks for taking some time to think about it.

[jonlives]

It's on my to-do list for the next release, watch this space :)

[jonlives]

I've reopened this issue as it didn't quite make it into the latest release - support for configurable git behavior modes is top of the list for the next release though.