On Tue, Nov 27, 2012 at 07:51:42PM +0100, Heiko Voigt wrote: > On Mon, Nov 26, 2012 at 04:00:18PM -0500, W. Trevor King wrote: > > -b:: > > --branch:: > > - Branch of repository to add as submodule. > > + When used with the add command, gives the branch of repository to > > + add as submodule. > > ++ > > +When used with the update command, checks out a branch named > > +`submodule.<name>.branch` (as set by `--local-branch`) pointing at the > > +current HEAD SHA-1. This is useful for commands like `update > > +--rebase` that do not work on detached heads. > > Since you are reusing this option for update it further convinces me > that reusing it for add makes sense and simplifies the logic for users. > > I think an optional argument for --branch would be nice in the update > case: > > $ git submodule update --branch=master > > would then allow a user that has not configured anything (except the > branch tracking info in the submodule of course) to pull all submodules > master branches.
Sounds good to me. Remember that this is checking the branch and
pointing it at $sha1 (preparing for the pull), not pulling remote
branches. The pull happens in a later
$ git submodules foreach 'git pull'
> > diff --git a/git-submodule.sh b/git-submodule.sh
> > index c51b6ae..28eb4b1 100755
> > --- a/git-submodule.sh
> > +++ b/git-submodule.sh
> > @@ -627,7 +631,7 @@ Maybe you want to use 'update --init'?")"
> > die "$(eval_gettext "Unable to find current revision in
> > submodule path '\$sm_path'")"
> > fi
> >
> > - if test "$subsha1" != "$sha1" -o -n "$force"
> > + if test "$subsha1" != "$sha1" -o -n "$force" -o
> > "$update_module" = "branch"
>
> As said before I think separating your code from the current update
> logic will simplify the handling below.
This felt less invasive (it avoids duplicating the recursion logic),
but I don't mind breaking it into a separate function/block.
> > must_die_on_failure=
> > case "$update_module" in
> > rebase)
> > command="git rebase"
> > - die_msg="$(eval_gettext "Unable to rebase
> > '\$sha1' in submodule path '\$sm_path'")"
> > + die_msg="$(eval_gettext "Unable to rebase
> > '\$sha1' in submodule path '\$sm_path'")"
> > say_msg="$(eval_gettext "Submodule path
> > '\$sm_path': rebased into '\$sha1'")"
> > - must_die_on_failure=yes
> > + must_die_on_failure=yes
>
> Please always cleanup whitespace changes.
Oops, sloppy me. Will fix.
> > then
> > - die_with_status 2 "$die_msg"
> > - else
> > - err="${err};$die_msg"
> > - continue
> > + if (clear_local_git_env; cd "$sm_path" &&
> > + git branch -f "$branch" "$sha1" &&
> > + git checkout "$branch")
>
> You wrote in earlier emails that you wanted to protect the user from
> non-fastforward changes. So I would expect a
>
> $ git pull --ff-only
I'm not pulling here, I'm doing a regular `submodule update`, and
after that's done I checkout the branch pointing at the $sha1 to which
the branch was just updated. All the submodule-state-clobbering
caveats of a usual `submodule update` still apply to this new
`submodule update --branch`, and I'm fine with that.
> BTW, I am more and more convinced that an automatically manufactured
> commit on update with --branch should be the default.
Again, there's nothing to update. The pull happens in a separate
step.
Cheers,
Trevor
--
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
signature.asc
Description: OpenPGP digital signature
