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
Attachment:
signature.asc
Description: OpenPGP digital signature