Re: Re: [PATCH v4 0/4] git-submodule add: Add --local-branch option
On Tue, Nov 27, 2012 at 6:28 PM, Heiko Voigt wrote: > > Hi, > > On Tue, Nov 27, 2012 at 02:01:05PM -0500, W. Trevor King wrote: > > On Tue, Nov 27, 2012 at 07:31:25PM +0100, Heiko Voigt wrote: > > The v4 series leaves the remote branch amigious, but it helps you > > point the local branch at the right hash so that future calls to > > > > $ git submodule foreach 'git pull' > > > > can use the branch's .git/modules//config settings. > > But IMO thats the functionality which should be implemented in submodule > update and not left to the user. > > > > I would think more of some convention like: > > > > > > $ git checkout -t origin/$branch > > > > > > when first initialising the submodule with e.g. > > > > > > $ git submodule update --init --branch > > > > > > Then later calls of > > > > > > $ git submodule update --branch > > > > > > would have a branch configured to pull from. I imagine that results in > > > a similar behavior gerrit is doing on the server side? > > > > That sounds like it's doing pretty much the same thing. Can you think > > of a test that would distinguish it from my current v4 implementation? > > Well the main difference is that gerrit is automatically updating the > superproject AFAIK. I would like it if we could implement the same > workflow support in the submodule script. It seems to me that this is > already proven to be useful workflow. It is proven in Gerrit, but Gerrit implements a central-server workflow. That is, only Gerrit ever floats the submodules, and he pushes the result for everyone else to share. I fear the consequences of everyone pulling submodules and then later trying to merge superprojects with someone else's breadcrumbs. Do you have some idea how this would be handled? Phil ps. Apologies for my lateness on this topic. I'm trying to catch up now. pps. Re-sent since Gmail has hidden the "plain text" option in a different place, now. On Tue, Nov 27, 2012 at 9:42 PM, W. Trevor King wrote: > On Wed, Nov 28, 2012 at 12:28:58AM +0100, Heiko Voigt wrote: >> On Tue, Nov 27, 2012 at 02:01:05PM -0500, W. Trevor King wrote: >> > On Tue, Nov 27, 2012 at 07:31:25PM +0100, Heiko Voigt wrote: >> > The v4 series leaves the remote branch amigious, but it helps you >> > point the local branch at the right hash so that future calls to >> > >> > $ git submodule foreach 'git pull' >> > >> > can use the branch's .git/modules//config settings. >> >> But IMO thats the functionality which should be implemented in submodule >> update and not left to the user. > > Then you might need submodule..local-branch, > submodule..remote-repository, and submodule..remote-branch > to configure > > $ git checkout submodule..local-branch > $ git pull submodule..remote-repository submodule..remote-branch > > and this would ignore the $sha1 stored in the gitlink (which all of > the other update commands use). This ignoring-the-$sha1 bit made me > think that a built-in pull wasn't a good fit for 'submodule update'. > Maybe if it went into a new 'submodule pull'? Then users have a clear > distinction: > > * 'update' to push superproject $sha1 changes into the submodules > * 'pull' to push upstream-branch changes into the submodules > >> > > I would think more of some convention like: >> > > >> > > $ git checkout -t origin/$branch >> > > >> > > when first initialising the submodule with e.g. >> > > >> > > $ git submodule update --init --branch >> > > >> > > Then later calls of >> > > >> > > $ git submodule update --branch >> > > >> > > would have a branch configured to pull from. I imagine that results in >> > > a similar behavior gerrit is doing on the server side? >> > >> > That sounds like it's doing pretty much the same thing. Can you think >> > of a test that would distinguish it from my current v4 implementation? >> >> Well the main difference is that gerrit is automatically updating the >> superproject AFAIK. I would like it if we could implement the same >> workflow support in the submodule script. It seems to me that this is >> already proven to be useful workflow. > > Ah, sorry, I meant the configuring which remote branch you were > pulling from happens at submodule initialization (via .git/modules/…) > for both your workflow and my v4. > > You're right that having a builtin pull is different from my v4. > >> https://github.com/hvoigt/git/commits/hv/floating_submodules_draft > > I looked over this before, but maybe not thoroughly enough ;). > >> > > How about reusing the -b|--branch option for add? Since we only change >> > > the behavior when submodule.$name.update is set to branch it seems >> > > reasonable to me. Opinions? >> > >> > That was the approach I used in v1, but people were concerned that we >> > would be stomping on previously unclaimed config space. Since noone >> > has pointed out other uses besides Gerrit's very similar case, I'm not >> > sure if that is still an issue. >> >> Could you point me to that mail? I cannot seem to find it in my arc
Re: [PATCH v4 0/4] git-submodule add: Add --local-branch option (summary)
On Wed, Nov 28, 2012 at 08:09:03AM -0500, W. Trevor King wrote: > * A new 'submodule pull' for tracking the submodule's remote, which is > pulling --ff-only origin/$branch into a whatever state the submodule > is currently in. If any changes were made to submodule $shas, > optionally commit them with a shortlog-generated commit message. I thought of a better idea on the train. How about adding `--remote` to `submodule update` that overrides the gitlinked SHA-1 with the SHA-1 for origin/$branch? All of the other checkout/merge/rebase functionality is untouched. The only thing that changes is where we look for the update. I'm working up the patch now, and will submit v5 shortly. -- 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 0/4] git-submodule add: Add --local-branch option (summary)
On Tue, Nov 27, 2012 at 09:42:05PM -0500, W. Trevor King wrote: > On Wed, Nov 28, 2012 at 12:28:58AM +0100, Heiko Voigt wrote: > > https://github.com/hvoigt/git/commits/hv/floating_submodules_draft > > I looked over this before, but maybe not thoroughly enough ;). Heiko pointed out that I likely hadn't looked at this branch at all, which is true. I'd confused it with his earlier: On Fri, Nov 09, 2012 at 05:29:27PM +0100, Heiko Voigt wrote: > https://github.com/hvoigt/git/commits/hv/floating_submodules but the new floating_submodules_draft branch adds Heiko's function reorganization and floating submodule pull (with 'update --branch') on top of my v4 commits (instead of my branch-checkout with 'update --branch'). > This looks fine, but my current --branch implementation (which doesn't > pull) is only a thin branch-checkout layer on top of the standard > `update` functionality. I'm still unsure if built-in pulls are worth > the configuration trouble. I'll sleep on it. Maybe I'll feel better > about them tomorrow ;). I do feel better about them today. To get a better feel for how people see this going forward, here is a summary of proposals to date: v1. Record submodule..branch with 'submodule add --branch …', leave interpretation up to the user. Feedback (paraphrased): Phil Hord: that clobbers a configure option used by Gerritt and possibly others, rename to --record-branch. Maybe we should export submodule..* as $submodule_* in 'foreach'? Nahor: what about corner cases (e.g. upstream renames branches)? Jens Lehmann: if you record it, people will expect Git to use it. What about automatic pulls & commits? v2. Add --record instead of using --branch to address Phil's v1 comment. Feedback (paraphrased): Jens Lehmann: this is still a tiny optimization over using 'git config'. People still use this option in different ways. Shawn Pearce: the Gerrit use is basically the same as Ævar's, but Gerrit has special handling for '.'. Jeff King: if we set up submodule..branch, we should either use it in Git, or make it very clear that we do not use it. If we use Phil's $submodule_* export, we should clear the variables for recursive submodules. v3. Renamed Added $submodule_* export to v2. Give an example of Ævar's pull workflow when explaining that Git does not use the value internally. Feedback (paraphrased): Heiko Voigt: what about automatic pulls & commits? You should allow .git/config overrides of the .gitmodules settings. if we set up submodule..branch, we should use it in Git. How does the submodule know which remote branch to track? Junio's proposed 'diff' changes may hide $sha1 information. Junio C Hamano: if we set up submodule..branch, we should use it in Git. Use something more specific than --record. Update 'git diff [$path]' and friends in the superproject compares the HEAD of the checkout of the submodule at $path with the tip of the branch named by submodule.$name.branch in .gitmodules of the superproject, instead of the commit that is recorded in the index of the superproject. Sascha Cunz: you can use 'git shortlog $OLD_SHA1..$NEW_SHA1' for the automatic commit message. Trevor King: actually, Ævar's update command only specifies the local branch name. The remote is recorded for that branch in .git/modules//config. v4. Rename --record to --local-branch. Remove $submodule_* export. Add .git/config overrides for submodule..branch. Add 'submodule update --branch' to check out $sha1 as submodule..branch. Feedback (paraphrased): Heiko Voigt: who cares about the local branch name? You should be pulling origin/$branch, but still using .git/modules//config to record the tracked branch. You should also use 'submodule add --branch[=branch]' instead of '--local-branch'. You should pull the 'update --branch' functionality out into a sub-function like 'handle_tracked_branch_update'. Jens Lehmann: .git/config overrides are nice, but they should be optional. Trevor King: floating branches are following the submodule's remote, while 'update --rebase/--merge' are following the superproject's recorded $sha1. Bundling remote-following and superproject-following in the same command may be confusing. Here's what I think we need in v5: * Jens' optional .git/config overrides. * Return --local-branch handling to a side effect of --branch (as in v1). * A new 'submodule pull' for tracking the submodule's remote, which is pulling --ff-only origin/$branch into a whatever state the submodule is currently in. If any changes were made to submodule $shas, optionally commit them with a shortlog-generated commit message. * Remove my c
Re: Re: [PATCH v4 0/4] git-submodule add: Add --local-branch option
On Wed, Nov 28, 2012 at 12:28:58AM +0100, Heiko Voigt wrote:
> On Tue, Nov 27, 2012 at 02:01:05PM -0500, W. Trevor King wrote:
> > On Tue, Nov 27, 2012 at 07:31:25PM +0100, Heiko Voigt wrote:
> > The v4 series leaves the remote branch amigious, but it helps you
> > point the local branch at the right hash so that future calls to
> >
> > $ git submodule foreach 'git pull'
> >
> > can use the branch's .git/modules//config settings.
>
> But IMO thats the functionality which should be implemented in submodule
> update and not left to the user.
Then you might need submodule..local-branch,
submodule..remote-repository, and submodule..remote-branch
to configure
$ git checkout submodule..local-branch
$ git pull submodule..remote-repository submodule..remote-branch
and this would ignore the $sha1 stored in the gitlink (which all of
the other update commands use). This ignoring-the-$sha1 bit made me
think that a built-in pull wasn't a good fit for 'submodule update'.
Maybe if it went into a new 'submodule pull'? Then users have a clear
distinction:
* 'update' to push superproject $sha1 changes into the submodules
* 'pull' to push upstream-branch changes into the submodules
> > > I would think more of some convention like:
> > >
> > > $ git checkout -t origin/$branch
> > >
> > > when first initialising the submodule with e.g.
> > >
> > > $ git submodule update --init --branch
> > >
> > > Then later calls of
> > >
> > > $ git submodule update --branch
> > >
> > > would have a branch configured to pull from. I imagine that results in
> > > a similar behavior gerrit is doing on the server side?
> >
> > That sounds like it's doing pretty much the same thing. Can you think
> > of a test that would distinguish it from my current v4 implementation?
>
> Well the main difference is that gerrit is automatically updating the
> superproject AFAIK. I would like it if we could implement the same
> workflow support in the submodule script. It seems to me that this is
> already proven to be useful workflow.
Ah, sorry, I meant the configuring which remote branch you were
pulling from happens at submodule initialization (via .git/modules/…)
for both your workflow and my v4.
You're right that having a builtin pull is different from my v4.
> https://github.com/hvoigt/git/commits/hv/floating_submodules_draft
I looked over this before, but maybe not thoroughly enough ;).
> > > How about reusing the -b|--branch option for add? Since we only change
> > > the behavior when submodule.$name.update is set to branch it seems
> > > reasonable to me. Opinions?
> >
> > That was the approach I used in v1, but people were concerned that we
> > would be stomping on previously unclaimed config space. Since noone
> > has pointed out other uses besides Gerrit's very similar case, I'm not
> > sure if that is still an issue.
>
> Could you point me to that mail? I cannot seem to find it in my archive.
Hmm. It seems like Phil's initial response was (accidentally?) off
list. The relevant portion was:
On Mon, Oct 22, 2012 at 06:03:53PM -0400, Phil Hord wrote:
> Some projects now use the 'branch' config value to record the tracking
> branch for the submodule. Some ascribe different meaning to the
> configuration if the value is given vs. undefined. For example, see
> the Gerrit submodule-subscription mechanism. This change will cause
> those workflows to behave differently than they do now.
>
> I do like the idea, but I wish it had a different name for the
> recording. Maybe --record-branch=${BRANCH} as an extra switch so the
> action is explicitly requested.
As I said, I'm happy to go back to --branch if opinions have changed.
On Wed, Nov 28, 2012 at 12:28:58AM +0100, Heiko Voigt wrote:
> On Tue, Nov 27, 2012 at 02:01:05PM -0500, W. Trevor King wrote:
> > On Tue, Nov 27, 2012 at 07:31:25PM +0100, Heiko Voigt wrote:
> > > > Because you need to recurse through submodules for `update --branch`
> > > > even if "$subsha1" == "$sha1", I had to amend the conditional
> > > > controlling that block. This broke one of the existing tests, which I
> > > > "fixed" in patch 4. I think a proper fix would involve rewriting
> > > >
> > > > (clear_local_git_env; cd "$sm_path" &&
> > > >( (rev=$(git rev-list -n 1 $sha1 --not --all 2>/dev/null) &&
> > > > test -z "$rev") || git-fetch)) ||
> > > > die "$(eval_gettext "Unable to fetch in submodule path '\$sm_path'")"
> > > >
> > > > but I'm not familiar enough with rev-list to want to dig into that
> > > > yet. If feedback for the earlier three patches is positive, I'll work
> > > > up a clean fix and resubmit.
> > >
> > > You probably need to separate your handling here. The comparison of the
> > > currently checked out sha1 and the recorded sha1 is an optimization
> > > which skips unnecessary fetching in case the submodules commits are
> > > already correct. This code snippet checks whether the to be checked out
> > > sha1 is already local and also skips the fetc
Re: Re: [PATCH v4 0/4] git-submodule add: Add --local-branch option
Hi,
On Tue, Nov 27, 2012 at 02:01:05PM -0500, W. Trevor King wrote:
> On Tue, Nov 27, 2012 at 07:31:25PM +0100, Heiko Voigt wrote:
> The v4 series leaves the remote branch amigious, but it helps you
> point the local branch at the right hash so that future calls to
>
> $ git submodule foreach 'git pull'
>
> can use the branch's .git/modules//config settings.
But IMO thats the functionality which should be implemented in submodule
update and not left to the user.
> > I would think more of some convention like:
> >
> > $ git checkout -t origin/$branch
> >
> > when first initialising the submodule with e.g.
> >
> > $ git submodule update --init --branch
> >
> > Then later calls of
> >
> > $ git submodule update --branch
> >
> > would have a branch configured to pull from. I imagine that results in
> > a similar behavior gerrit is doing on the server side?
>
> That sounds like it's doing pretty much the same thing. Can you think
> of a test that would distinguish it from my current v4 implementation?
Well the main difference is that gerrit is automatically updating the
superproject AFAIK. I would like it if we could implement the same
workflow support in the submodule script. It seems to me that this is
already proven to be useful workflow.
I do not have a test but a small draft diff (completely untested, quick and
dirty) to illustrate the approach I am talking about.
You can find the whole change at
https://github.com/hvoigt/git/commits/hv/floating_submodules_draft
and the interesting patch for easy commenting below[1].
> > How about reusing the -b|--branch option for add? Since we only change
> > the behavior when submodule.$name.update is set to branch it seems
> > reasonable to me. Opinions?
>
> That was the approach I used in v1, but people were concerned that we
> would be stomping on previously unclaimed config space. Since noone
> has pointed out other uses besides Gerrit's very similar case, I'm not
> sure if that is still an issue.
Could you point me to that mail? I cannot seem to find it in my archive.
> > > Because you need to recurse through submodules for `update --branch`
> > > even if "$subsha1" == "$sha1", I had to amend the conditional
> > > controlling that block. This broke one of the existing tests, which I
> > > "fixed" in patch 4. I think a proper fix would involve rewriting
> > >
> > > (clear_local_git_env; cd "$sm_path" &&
> > >( (rev=$(git rev-list -n 1 $sha1 --not --all 2>/dev/null) &&
> > > test -z "$rev") || git-fetch)) ||
> > > die "$(eval_gettext "Unable to fetch in submodule path '\$sm_path'")"
> > >
> > > but I'm not familiar enough with rev-list to want to dig into that
> > > yet. If feedback for the earlier three patches is positive, I'll work
> > > up a clean fix and resubmit.
> >
> > You probably need to separate your handling here. The comparison of the
> > currently checked out sha1 and the recorded sha1 is an optimization
> > which skips unnecessary fetching in case the submodules commits are
> > already correct. This code snippet checks whether the to be checked out
> > sha1 is already local and also skips the fetch if it is. We should not
> > break that.
>
> Agreed. However, determining if the target $sha1 is local should have
> nothing to do with the current checked out $subsha1.
See my draft or the diff below for an illustration of the splitup.
Cheers Heiko
[1]
diff --git a/git-submodule.sh b/git-submodule.sh
index 9ad4370..3fa1465 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -183,6 +183,7 @@ module_clone()
sm_path=$1
url=$2
reference="$3"
+ branch="$4"
quiet=
if test -n "$GIT_QUIET"
then
@@ -209,6 +210,8 @@ module_clone()
clear_local_git_env
git clone $quiet -n ${reference:+"$reference"} \
--separate-git-dir "$gitdir" "$url" "$sm_path"
+ test -n "$branch" && (cd $sm_path &&
+ git checkout -t origin/$branch)
) ||
die "$(eval_gettext "Clone of '\$url' into submodule path
'\$sm_path' failed")"
fi
@@ -361,7 +364,7 @@ Use -f if you really want to add it." >&2
else
- module_clone "$sm_path" "$realrepo" "$reference" || exit
+ module_clone "$sm_path" "$realrepo" "$reference"
"$local_branch" || exit
(
clear_local_git_env
cd "$sm_path" &&
@@ -577,6 +580,12 @@ handle_on_demand_update () {
fi
}
+handle_tracking_branch_update () {
+ (clear_local_git_env; cd "$sm_path" &&
+ git-checkout $branch && git-pull --ff-only) ||
+ die "$(eval_gettext "Unable to pull branch '\$branch' in submodule path
'\$sm_path'")"
+}
+
#
# Update each submodule path to correct revision, using clone and checkout as
needed
#
@@ -648,6 +657,7 @@ cmd_update()
clo
Re: [PATCH v4 0/4] git-submodule add: Add --local-branch option
On Tue, Nov 27, 2012 at 07:31:25PM +0100, Heiko Voigt wrote: > On Mon, Nov 26, 2012 at 04:00:15PM -0500, W. Trevor King wrote: > > From: "W. Trevor King" > > > > On Fri, Nov 23, 2012 at 12:54:02PM -0500, W. Trevor King wrote: > > > We could add > > > > > > $ git submodule update --branch > > > > > > to checkout the gitlinked SHA1 as submodule..branch in each of > > > the submodules, leaving the submodules on the .gitmodules-configured > > > branch. Effectively (for each submodule): > > > > > > $ git branch -f $branch $sha1 > > > $ git checkout $branch > > > > I haven't gotten any feedback on this as an idea, but perhaps someone > > will comment on it as a patch series ;). > > I am not sure I understand you correctly. You are suggesting that the > branch option as an alias for the registered SHA1 in the superproject? > > I though the goal of your series was that you want to track submodules > branch which come from the remote side? That's what I'd initially thought, but when I went to implement `update --pull`, I realized that $ git submodule foreach 'git checkout $(git config --file $toplevel/.gitmodules submodule.$name.branch) && …' is using submodule..branch as the local branch name. The remote branch name was actually setup in .git/modules//config during the initial "clone -b …". The v4 series leaves the remote branch amigious, but it helps you point the local branch at the right hash so that future calls to $ git submodule foreach 'git pull' can use the branch's .git/modules//config settings. > I would think more of some convention like: > > $ git checkout -t origin/$branch > > when first initialising the submodule with e.g. > > $ git submodule update --init --branch > > Then later calls of > > $ git submodule update --branch > > would have a branch configured to pull from. I imagine that results in > a similar behavior gerrit is doing on the server side? That sounds like it's doing pretty much the same thing. Can you think of a test that would distinguish it from my current v4 implementation? > > Changes since v3: > > > > * --record=??? is now --local-branch=??? > > * Dropped patches 2 ($submodule_ export) and 3 (motivating documentation) > > * Added local git-config overrides of .gitmodules' submodule..branch > > * Added `submodule update --branch` > > I would prefer if we could squash all these commits together into one > since it seems to me one logical step, using the new variable for update > belongs together with its configuration on initialization. > > How about reusing the -b|--branch option for add? Since we only change > the behavior when submodule.$name.update is set to branch it seems > reasonable to me. Opinions? That was the approach I used in v1, but people were concerned that we would be stomping on previously unclaimed config space. Since noone has pointed out other uses besides Gerrit's very similar case, I'm not sure if that is still an issue. > > Because you need to recurse through submodules for `update --branch` > > even if "$subsha1" == "$sha1", I had to amend the conditional > > controlling that block. This broke one of the existing tests, which I > > "fixed" in patch 4. I think a proper fix would involve rewriting > > > > (clear_local_git_env; cd "$sm_path" && > >( (rev=$(git rev-list -n 1 $sha1 --not --all 2>/dev/null) && > > test -z "$rev") || git-fetch)) || > > die "$(eval_gettext "Unable to fetch in submodule path '\$sm_path'")" > > > > but I'm not familiar enough with rev-list to want to dig into that > > yet. If feedback for the earlier three patches is positive, I'll work > > up a clean fix and resubmit. > > You probably need to separate your handling here. The comparison of the > currently checked out sha1 and the recorded sha1 is an optimization > which skips unnecessary fetching in case the submodules commits are > already correct. This code snippet checks whether the to be checked out > sha1 is already local and also skips the fetch if it is. We should not > break that. Agreed. However, determining if the target $sha1 is local should have nothing to do with the current checked out $subsha1. Thanks for the feedback! 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
Re: Re: [PATCH v4 0/4] git-submodule add: Add --local-branch option
Hi, I just realized that I gave you an confusing suggestion. On Tue, Nov 27, 2012 at 07:31:25PM +0100, Heiko Voigt wrote: > if test "$subsha1" != "$sha1" > then > handle_on_demand_fetch_update ... > else > handle_tracked_branch_update ... > fi That obviously does not work. Here I meant of course something like: if test "$update_module" = "branch" then handle_tracked_branch_update ... else handle_on_demand_fetch_update ... fi Cheers Heiko -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 0/4] git-submodule add: Add --local-branch option
On Tue, Nov 27, 2012 at 07:31:25PM +0100, Heiko Voigt wrote: > I would prefer if we could squash all these commits together into > one since it seems to me one logical step, using the new variable > for update belongs together with its configuration on > initialization. Works for me. I could also try to rework the patch boundaries if a monolithic patch is not acceptable. I agree that the current documentation assignments are fairly arbitrary. If I don't hear from anyone in favor of keeping them separate, v5 will be monolithic. -- 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
Re: [PATCH v4 0/4] git-submodule add: Add --local-branch option
Hi, On Mon, Nov 26, 2012 at 04:00:15PM -0500, W. Trevor King wrote: > From: "W. Trevor King" > > On Fri, Nov 23, 2012 at 12:54:02PM -0500, W. Trevor King wrote: > > We could add > > > > $ git submodule update --branch > > > > to checkout the gitlinked SHA1 as submodule..branch in each of > > the submodules, leaving the submodules on the .gitmodules-configured > > branch. Effectively (for each submodule): > > > > $ git branch -f $branch $sha1 > > $ git checkout $branch > > I haven't gotten any feedback on this as an idea, but perhaps someone > will comment on it as a patch series ;). I am not sure I understand you correctly. You are suggesting that the branch option as an alias for the registered SHA1 in the superproject? I though the goal of your series was that you want to track submodules branch which come from the remote side? Doing the above does not assist you much in that does it? I would think more of some convention like: $ git checkout -t origin/$branch when first initialising the submodule with e.g. $ git submodule update --init --branch Then later calls of $ git submodule update --branch would have a branch configured to pull from. I imagine that results in a similar behavior gerrit is doing on the server side? > Changes since v3: > > * --record=??? is now --local-branch=??? > * Dropped patches 2 ($submodule_ export) and 3 (motivating documentation) > * Added local git-config overrides of .gitmodules' submodule..branch > * Added `submodule update --branch` I would prefer if we could squash all these commits together into one since it seems to me one logical step, using the new variable for update belongs together with its configuration on initialization. How about reusing the -b|--branch option for add? Since we only change the behavior when submodule.$name.update is set to branch it seems reasonable to me. Opinions? > Because you need to recurse through submodules for `update --branch` > even if "$subsha1" == "$sha1", I had to amend the conditional > controlling that block. This broke one of the existing tests, which I > "fixed" in patch 4. I think a proper fix would involve rewriting > > (clear_local_git_env; cd "$sm_path" && >( (rev=$(git rev-list -n 1 $sha1 --not --all 2>/dev/null) && > test -z "$rev") || git-fetch)) || > die "$(eval_gettext "Unable to fetch in submodule path '\$sm_path'")" > > but I'm not familiar enough with rev-list to want to dig into that > yet. If feedback for the earlier three patches is positive, I'll work > up a clean fix and resubmit. You probably need to separate your handling here. The comparison of the currently checked out sha1 and the recorded sha1 is an optimization which skips unnecessary fetching in case the submodules commits are already correct. This code snippet checks whether the to be checked out sha1 is already local and also skips the fetch if it is. We should not break that. Maybe we need an else block here and possibly extract the current code inside the if statement into a function. E.g. that the final code looks something like this: if test "$subsha1" != "$sha1" then handle_on_demand_fetch_update ... else handle_tracked_branch_update ... fi Not sure about the function names though. If we decide to go that route: The extraction into a function should go in an extra preparation patch which does not change any functionality. I will reply to the patches for further comments. Cheers Heiko -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 0/4] git-submodule add: Add --local-branch option
From: "W. Trevor King" On Fri, Nov 23, 2012 at 12:54:02PM -0500, W. Trevor King wrote: > We could add > > $ git submodule update --branch > > to checkout the gitlinked SHA1 as submodule..branch in each of > the submodules, leaving the submodules on the .gitmodules-configured > branch. Effectively (for each submodule): > > $ git branch -f $branch $sha1 > $ git checkout $branch I haven't gotten any feedback on this as an idea, but perhaps someone will comment on it as a patch series ;). Changes since v3: * --record=… is now --local-branch=… * Dropped patches 2 ($submodule_ export) and 3 (motivating documentation) * Added local git-config overrides of .gitmodules' submodule..branch * Added `submodule update --branch` Because you need to recurse through submodules for `update --branch` even if "$subsha1" == "$sha1", I had to amend the conditional controlling that block. This broke one of the existing tests, which I "fixed" in patch 4. I think a proper fix would involve rewriting (clear_local_git_env; cd "$sm_path" && ( (rev=$(git rev-list -n 1 $sha1 --not --all 2>/dev/null) && test -z "$rev") || git-fetch)) || die "$(eval_gettext "Unable to fetch in submodule path '\$sm_path'")" but I'm not familiar enough with rev-list to want to dig into that yet. If feedback for the earlier three patches is positive, I'll work up a clean fix and resubmit. W. Trevor King (4): git-submodule add: Add --local-branch option git-submodule init: Record submodule..branch in repository config. git-submodule update: Add --branch option Hack fix for 'submodule update does not fetch already present commits' Documentation/config.txt| 9 ++--- Documentation/git-submodule.txt | 32 - Documentation/gitmodules.txt| 5 +++ git-submodule.sh| 76 + t/t7400-submodule-basic.sh | 43 +++ t/t7406-submodule-update.sh | 50 ++- 6 files changed, 187 insertions(+), 28 deletions(-) -- 1.8.0.3.g95edff1.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
