Re: Re: [RFC v2] submodule: Respect requested branch on all clones
Hi, On Thu, Jan 09, 2014 at 02:18:40PM -0800, W. Trevor King wrote: On Thu, Jan 09, 2014 at 10:40:52PM +0100, Jens Lehmann wrote: Am 09.01.2014 20:55, schrieb W. Trevor King: On Thu, Jan 09, 2014 at 08:23:07PM +0100, Jens Lehmann wrote: Am 09.01.2014 18:32, schrieb W. Trevor King: later updates, The same thing that currently happens, with the exception that checkout-style updates should use reset to update the currently-checked out branch (or detached-HEAD), instead of always detaching the HEAD. Won't the user loose any modifications to his local branch here? They just called for a checkout-style update, so yes. If they want to keep local modifications, chose an integration mode that preserves local changes. Hmm, as current submodule updates already makes it too easy to loose commits, this does not look right to me. I'd prefer to stop at that point and tell the user what he can do to solve the conflict. Users who are worried about loosing local updates should not be using a checkout-style updates. If they are using a checkout-style update, and they ask for an update, they're specifically requesting that we blow away their local work and checkout/reset to the new sha1. Solving update conflicts is the whole point of the non-checkout update modes. But checkout is different from reset --hard in that it does not blow away local uncommitted changes. Please see below. Maybe you meant for checkout I can easily overwrite the local changes with the upstream branch, which is what I understand checkout to do. But which I find really unfriendly and would not like to see in a new feature. We should protect the user from loosing any local changes, not simply throw them away. Recursive update makes sure it won't overwrite any local modification before it checks out anything and will abort before doing so (unless forced of course). If you want to get rid of checkout-mode updates, I'm fine with that. However, I don't think it supports use-cases like Heiko's (implied) “I don't care what's happening upstream, I never touch that submodule, just checkout what the superproject maintainer says should be checked out for this branch. Even if they have been rebasing or whatever” [3]. I never stated that in that post. Even with the current checkout-mode updates you'll never loose local modifications (without force) that are not committed. I think you have to distinguish between local modifications in the worktree and the ones that are committed. The recursive checkout Jens is working on is simply implementing the current checkout-mode to the places where the superproject checks out its files. That way submodules get checked out when the superproject is checked out. If the submodule does not match the sha1 registered in the superproject it either stays there (if the checkout would not need to update the submodule) or (if checkout would need to update) git will not do the checkout and bail out with you have local modifications to ... . I think the whole recursive checkout topic is already complicated enough as it is so we should currently not add anything from this discussion to it until it is cleaned up and merged. We also need recursive fetch for that which I am planning to work on as soon as we settle this discussion. I will write another post about how I think we should/can proceed. or be asked to resolve the conflict manually when checkout is configured and the branches diverged. I still think that checkout-mode updates should be destructive. See my paraphrased-version of Heiko's use case above. How are they going to resolve this manually? Merge or rebase? Why weren't they using that update mode in the first place? I strongly disagree. They should only be destructive in the sense that another commit get checked out but never loose local modifications. [1]: http://article.gmane.org/gmane.comp.version-control.git/240251 [2]: http://article.gmane.org/gmane.comp.version-control.git/240248 [3]: http://article.gmane.org/gmane.comp.version-control.git/240013 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: [RFC v2] submodule: Respect requested branch on all clones
On Tue, Jan 14, 2014 at 11:24:45AM +0100, Heiko Voigt wrote: On Thu, Jan 09, 2014 at 02:18:40PM -0800, W. Trevor King wrote: Users who are worried about loosing local updates should not be using a checkout-style updates. If they are using a checkout-style update, and they ask for an update, they're specifically requesting that we blow away their local work and checkout/reset to the new sha1. Solving update conflicts is the whole point of the non-checkout update modes. But checkout is different from reset --hard in that it does not blow away local uncommitted changes. Please see below. Ah, good point. We should definately die if there are local uncommitted changes. On Thu, Jan 09, 2014 at 10:40:52PM +0100, Jens Lehmann wrote: Am 09.01.2014 20:55, schrieb W. Trevor King: Maybe you meant for checkout I can easily overwrite the local changes with the upstream branch, which is what I understand checkout to do. But which I find really unfriendly and would not like to see in a new feature. We should protect the user from loosing any local changes, not simply throw them away. Recursive update makes sure it won't overwrite any local modification before it checks out anything and will abort before doing so (unless forced of course). If you want to get rid of checkout-mode updates, I'm fine with that. However, I don't think it supports use-cases like Heiko's (implied) “I don't care what's happening upstream, I never touch that submodule, just checkout what the superproject maintainer says should be checked out for this branch. Even if they have been rebasing or whatever” [3]. I never stated that in that post. Sorry for misunderstanding. I think I'm just unclear on your workflow? The recursive checkout Jens is working on is simply implementing the current checkout-mode to the places where the superproject checks out its files. That way submodules get checked out when the superproject is checked out. If the submodule does not match the sha1 registered in the superproject it either stays there (if the checkout would not need to update the submodule) or (if checkout would need to update) git will not do the checkout and bail out with you have local modifications to ... . Sounds good to me as far as it goes. I think it misses the “what should we do if the gitlinked hashes are different” case, because the checkout will always leave you with a detached HEAD. or be asked to resolve the conflict manually when checkout is configured and the branches diverged. I still think that checkout-mode updates should be destructive. See my paraphrased-version of Heiko's use case above. How are they going to resolve this manually? Merge or rebase? Why weren't they using that update mode in the first place? I strongly disagree. They should only be destructive in the sense that another commit get checked out but never loose local modifications. I think the key I'm missing is an example workflow where a developer wants to make local submodule changes, but also wants to use checkout-mode updates instead of merge/rebase updates. Can you sketch one out for me? 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
Re: Re: [RFC v2] submodule: Respect requested branch on all clones
On Tue, Jan 14, 2014 at 08:57:09AM -0800, W. Trevor King wrote: On Thu, Jan 09, 2014 at 10:40:52PM +0100, Jens Lehmann wrote: Am 09.01.2014 20:55, schrieb W. Trevor King: Maybe you meant for checkout I can easily overwrite the local changes with the upstream branch, which is what I understand checkout to do. But which I find really unfriendly and would not like to see in a new feature. We should protect the user from loosing any local changes, not simply throw them away. Recursive update makes sure it won't overwrite any local modification before it checks out anything and will abort before doing so (unless forced of course). If you want to get rid of checkout-mode updates, I'm fine with that. However, I don't think it supports use-cases like Heiko's (implied) “I don't care what's happening upstream, I never touch that submodule, just checkout what the superproject maintainer says should be checked out for this branch. Even if they have been rebasing or whatever” [3]. I never stated that in that post. Sorry for misunderstanding. I think I'm just unclear on your workflow? Yes probably. We mostly use submodules for shared code and for tracking external libraries. For the shared code we want to make sure that the changes that come from one project do not break anything in another project that also uses that code so the submodules are maintained and reviewed separately and ideally contain tests for the expected functionality. A typical workflow where a feature in a project needs some extension or change in a submodule goes like this: 1. The developer does his changes locally implementing everything needed. To commit he creates a local branch in the submodule and in the superproject (most of the times from the current HEAD that is checked out). 2. For convenience I usually commit the resulting commit sha1 of the submodule in the commit that needs the change. That way when I switch to a different branch and back I can simply say: git submodule update and get the correct code everywhere. 3. Once done with the whole feature I first do the review process (adding any missing tests to ensure my change does not break, ...) for the submodule to get the feature branch merged into a stable one. In our superproject only commits sha1 from a stable branch are allowed so the submodule change needs to be reviewed first. 4. Once the change is in a stable branch in the submodule I then update the commit sha1 link in the superproject that needs the change. That is usually done by rebasing the branch in the superproject and registering the new stable branch (typically master). 5. Then I proceed with the review process in the superproject as if it was a normal change without a submodule. Thats our main use case. The recursive checkout Jens is working on is simply implementing the current checkout-mode to the places where the superproject checks out its files. That way submodules get checked out when the superproject is checked out. If the submodule does not match the sha1 registered in the superproject it either stays there (if the checkout would not need to update the submodule) or (if checkout would need to update) git will not do the checkout and bail out with you have local modifications to ... . Sounds good to me as far as it goes. I think it misses the “what should we do if the gitlinked hashes are different” case, because the checkout will always leave you with a detached HEAD. or be asked to resolve the conflict manually when checkout is configured and the branches diverged. I still think that checkout-mode updates should be destructive. See my paraphrased-version of Heiko's use case above. How are they going to resolve this manually? Merge or rebase? Why weren't they using that update mode in the first place? I strongly disagree. They should only be destructive in the sense that another commit get checked out but never loose local modifications. I think the key I'm missing is an example workflow where a developer wants to make local submodule changes, but also wants to use checkout-mode updates instead of merge/rebase updates. Can you sketch one out for me? How about the use-case I sketched above? Is that what you are searching for? In that use-case we have to update to the new master after a submodule change was merged. That could be achieved by git submodule update --remote submodule with the wanted stable branch configured. But in practise something along the lines of (cd submodule git checkout origin/stable) is usually used and simple enough. We have a tool in our git gui configuration that does git submodule foreach 'git fetch git checkout origin/master' which can be used by the maintainer before a release to ensure that all submodules are up-to-date. But in practise it turn out that
Re: Re: [RFC v2] submodule: Respect requested branch on all clones
On Tue, Jan 14, 2014 at 09:58:30PM +0100, Heiko Voigt wrote:
A typical workflow where a feature in a project needs some extension or
change in a submodule goes like this:
1. The developer does his changes locally implementing everything
needed. To commit he creates a local branch in the submodule and in
the superproject (most of the times from the current HEAD that is
checked out).
2. For convenience I usually commit the resulting commit sha1 of the
submodule in the commit that needs the change. That way when I switch
to a different branch and back I can simply say: git submodule update
and get the correct code everywhere.
This checkout functionality is exactly what my
submodule.name.localBranch is designed to automate [1]. I think
that should be different from integrating local and external changes,
which is what 'git submodule update' is about. For example, after you
run 'git submodule update' here, you'll have your original commit
checked out, but you'll be on a detached HEAD instead of your original
branch. If you want to further develop the submodule feature branch,
you currently have to cd into the submodule and check the branch out
by hand.
How about the use-case I sketched above? Is that what you are searching
for? In that use-case we have to update to the new master after a
submodule change was merged. That could be achieved by
git submodule update --remote submodule
with the wanted stable branch configured. But in practise something
along the lines of
(cd submodule git checkout origin/stable)
is usually used and simple enough.
The “gitlinked commits must be in the subproject's master” rule
protects you from blowing stuff away here. You could use rebase- or
merge-style integration as well, assuming the maintainer didn't have
dirty local work in their submodule.
We have a tool in our git gui configuration that does
git submodule foreach 'git fetch git checkout origin/master'
I agree that with 'submodule update' seems superfluous. With proper
out-of-tree submodule configs specifying remote URLs and upstream
branches,
git submodule foreach 'git fetch git checkout @{upstream}'
(or merge/rebase/…) should cover this case more generically and with
less mental overhead.
I hope that draws a clear picture of how we use submodules.
It's certainly clearer, thanks :). I'm not sure where checkout-mode
is specifically important, though. In your step-2, it doesn't restore
your original branch. In your “update the superproject's master”
step, you aren't even using 'submodule update' :p.
Cheers,
Trevor
[1]: http://article.gmane.org/gmane.comp.version-control.git/240336
--
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: Re: [RFC v2] submodule: Respect requested branch on all clones
Hi, On Tue, Jan 14, 2014 at 11:24:45AM +0100, Heiko Voigt wrote: I will write another post about how I think we should/can proceed. and here is my suggestion how we should proceed. I think there have been many interesting ideas in this thread but IMO some of them tried to achieve a little bit to much and were not clear enough. I am a fan of: Keep it as simple as possible, but *no simpler*. I think some ideas where going in the make it to simple direction. Take my idea for feature branch support from here[1]. After thinking more thoroughly it still too many corner cases. E.g. it is way to easy to accidentally merge the feature branch configuration into the stable branch. But we want to support the user properly so we need to catch stuff like that. Submodules are separate projects. There is a boundary between superproject and submodule and IMO its there for a good reason. E.g. take the typical shared code use-case. If A and B are using C then both want to make sure a change from A does not break B's expectations and vice versa. Thats were you usually write unit tests in C for: Ensure that the expectations are met. The more users of the code the higher the quality and thus the boundary for bad code should be. I would like to step back a bit and get back to the original problem at hand: Francescos original use case of an attached head for direct commits on a stable branch in a submodule. How about we finish discussing the exact solution of that first. AFAIK that is already solved with the following: * Trevor's first patch[2] to create a branch on initial clone of a submodule * A possibly a configuration variable for --remote so it can be set as the default update method * Combined with submodule.name.update=merge/rebase That should be all (and IIRC Francesco agreed) needed for that use-case. Lets not implement more than currently is needed. We can revisit the ideas once some other real use-case manifests. Also we (Jens and I) would first like to proceed with the recursive checkout / fetch (for which the plan is clear) as the next complicated step. Once that is done and people gain some experience with it we can still extend further. What do you think? Cheers Heiko [1] http://article.gmane.org/gmane.comp.version-control.git/240178/ [2] http://article.gmane.org/gmane.comp.version-control.git/239921 -- 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: Re: Re: [RFC v2] submodule: Respect requested branch on all clones
On Tue, Jan 14, 2014 at 01:42:09PM -0800, W. Trevor King wrote:
On Tue, Jan 14, 2014 at 09:58:30PM +0100, Heiko Voigt wrote:
A typical workflow where a feature in a project needs some extension or
change in a submodule goes like this:
1. The developer does his changes locally implementing everything
needed. To commit he creates a local branch in the submodule and in
the superproject (most of the times from the current HEAD that is
checked out).
2. For convenience I usually commit the resulting commit sha1 of the
submodule in the commit that needs the change. That way when I switch
to a different branch and back I can simply say: git submodule update
and get the correct code everywhere.
This checkout functionality is exactly what my
submodule.name.localBranch is designed to automate [1]. I think
that should be different from integrating local and external changes,
which is what 'git submodule update' is about. For example, after you
run 'git submodule update' here, you'll have your original commit
checked out, but you'll be on a detached HEAD instead of your original
branch. If you want to further develop the submodule feature branch,
you currently have to cd into the submodule and check the branch out
by hand.
Yes and thats exactly what my idea was about but after further thinking
am afraid that this is the wrong place. I am not sure but afraid as I
wrote in the other post that it would be way to dangerous to accidentally
merge these changes in. We would need something to prevent this
configuration from ever entering a stable branch.
Another solution (and completely different approach) would be to have
something that is outside of the tree and actually attached to a
branchname. E.g. at the gitmerge last year I though it would be nice to
have a place for a description for a branch inside git. In a short
discussion we were envisioning a special ref like the notes trees but
allowing to attach and describe branches. That place could also be where
we could store such a configuration. Once the branchname ceases to exist
so would the configuration.
I know this is a completely different piece of work so I am not sure
whether we want to pursue it at the moment. But at the moment I think
this would actually be the correct solution.
How about the use-case I sketched above? Is that what you are searching
for? In that use-case we have to update to the new master after a
submodule change was merged. That could be achieved by
git submodule update --remote submodule
with the wanted stable branch configured. But in practise something
along the lines of
(cd submodule git checkout origin/stable)
is usually used and simple enough.
The “gitlinked commits must be in the subproject's master” rule
protects you from blowing stuff away here. You could use rebase- or
merge-style integration as well, assuming the maintainer didn't have
dirty local work in their submodule.
No we can't. Developers are not allowed to merge in some submodules.
The most central ones have maintainers and only they are allowed to
merge into the stable branch. So we need to track exact commits on the
stable branch, no local merge (except the fast-forward case of course)
allowed. Thats why the developer does an exact checkout here.
We have a tool in our git gui configuration that does
git submodule foreach 'git fetch git checkout origin/master'
I agree that with 'submodule update' seems superfluous. With proper
out-of-tree submodule configs specifying remote URLs and upstream
branches,
git submodule foreach 'git fetch git checkout @{upstream}'
(or merge/rebase/…) should cover this case more generically and with
less mental overhead.
I hope that draws a clear picture of how we use submodules.
It's certainly clearer, thanks :). I'm not sure where checkout-mode
is specifically important, though. In your step-2, it doesn't restore
your original branch. In your “update the superproject's master”
step, you aren't even using 'submodule update' :p.
Ah sorry I though that was clear. The others are using submodule update ;-)
I mean someone who gets stuff from a stable superproject branch by
either rebasing their development branch or updating their local copy of
a stable branch (e.g. master) by using
git checkout master
git pull --ff --ff-only
git submodule update --init --recursive
This also prevents the pure updaters from creating unnecessary merges.
Cheers Heiko
[1]: http://article.gmane.org/gmane.comp.version-control.git/240336
--
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: [RFC v2] submodule: Respect requested branch on all clones
On Tue, Jan 14, 2014 at 10:46:08PM +0100, Heiko Voigt wrote: I would like to step back a bit and get back to the original problem at hand: Francescos original use case of an attached head for direct commits on a stable branch in a submodule. How about we finish discussing the exact solution of that first. AFAIK that is already solved with the following: * Trevor's first patch[2] to create a branch on initial clone of a submodule v1 broke a bunch of tests. Are you ok with v2 [1]? v2 still needs a clearer commit message, a test, and a possible transition to triggering on non-checkout submodule.name.update instead of non-empty submodule.name.branch [2]. That should be all (and IIRC Francesco agreed) needed for that use-case. That was my understanding [3] ;). Lets not implement more than currently is needed. We can revisit the ideas once some other real use-case manifests. I have most of a real use case already. I have a repository with submodules in one branch (master) and a subtree version in another (assembled) [4]. The *tree* is the same in each case, so I have to 'git rm -rf .' to clear the submodules out of master before I can checkout assembled. $ git checkout assembled error: The following untracked working tree files would be overwritten by checkout: modular/README.md modular/shell/README.md … $ git rm -rf . $ git checkout assembled That leaves some extra stuff removed: $ git status On branch assembled Changes to be committed: (use git reset HEAD file... to unstage) deleted:.gitignore deleted:.mailmap deleted:CONTRIBUTING.md deleted:LICENSE.md deleted:instructor.md so I need to check that out by hand: $ git reset --hard HEAD Now I can work in the assembled branch. Going back to master is a bit less tedious: $ git checkout master $ git submodule update --recursive Luckily for me, I don't have a third superproject branch where the submodules are on a different, so the submodule's HEADs are preserved. As I understand it, the new recursive checkout functionality [5] would checkout my submodules with detached HEADs. The fact that they are only accidentally preserved now is not comforting ;). Also we (Jens and I) would first like to proceed with the recursive checkout / fetch (for which the plan is clear) as the next complicated step. Once that is done and people gain some experience with it we can still extend further. This is quite reasonable. Given the need for backwards compatibility, I just wanted to make sure my ideal UI was clear before we went forward. There's no need to break fingers twice ;), but if tight binding with localBranch is too big a chunk to bite off now, I'm happy to kick that can down the road. Cheers, Trevor [1]: http://article.gmane.org/gmane.comp.version-control.git/239967 [2]: http://article.gmane.org/gmane.comp.version-control.git/239973 [3]: http://article.gmane.org/gmane.comp.version-control.git/240139 [4]: (gitweb) http://git.tremily.us/?p=swc-boot-camp.git [5]: http://article.gmane.org/gmane.comp.version-control.git/239695 -- 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: [RFC v2] submodule: Respect requested branch on all clones
On Tue, Jan 14, 2014 at 11:19:07PM +0100, Heiko Voigt wrote:
On Tue, Jan 14, 2014 at 01:42:09PM -0800, W. Trevor King wrote:
The “gitlinked commits must be in the subproject's master” rule
protects you from blowing stuff away here. You could use rebase- or
merge-style integration as well, assuming the maintainer didn't have
dirty local work in their submodule.
No we can't. Developers are not allowed to merge in some submodules.
The most central ones have maintainers and only they are allowed to
merge into the stable branch. So we need to track exact commits on the
stable branch, no local merge (except the fast-forward case of course)
allowed. Thats why the developer does an exact checkout here.
Because both sides of the merge/rebase are required (by your rule) to
be in the subproject's master, you're guaranteed to have a
fast-forward merge/rebase.
We have a tool in our git gui configuration that does
git submodule foreach 'git fetch git checkout origin/master'
I agree that with 'submodule update' seems superfluous. With proper
out-of-tree submodule configs specifying remote URLs and upstream
branches,
git submodule foreach 'git fetch git checkout @{upstream}'
(or merge/rebase/…) should cover this case more generically and with
less mental overhead.
I hope that draws a clear picture of how we use submodules.
It's certainly clearer, thanks :). I'm not sure where checkout-mode
is specifically important, though. In your step-2, it doesn't restore
your original branch. In your “update the superproject's master”
step, you aren't even using 'submodule update' :p.
Ah sorry I though that was clear. The others are using submodule update ;-)
I mean someone who gets stuff from a stable superproject branch by
either rebasing their development branch or updating their local copy of
a stable branch (e.g. master) by using
git checkout master
git pull --ff --ff-only
git submodule update --init --recursive
This also prevents the pure updaters from creating unnecessary merges.
Right. Folks who don't do local development in their submodules can
get away with checkout-mode updates and their detached HEADs.
Assuming the upstream superproject only advances the gitlinked commits
using fast-forwards, they could equally well use any of the other
update modes, but there is no need for them to make that assumption.
*This* is the use case that I think the current recursive-checkout
facilitates [1]. I don't think it helps folks who are actually doing
submodule development on branches from within their superproject.
Cheers,
Trevor
[1]: http://article.gmane.org/gmane.comp.version-control.git/239695
--
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: [RFC v2] submodule: Respect requested branch on all clones
On Tue, Jan 14, 2014 at 02:22:31PM -0800, W. Trevor King wrote: On Tue, Jan 14, 2014 at 10:46:08PM +0100, Heiko Voigt wrote: I would like to step back a bit and get back to the original problem at hand: Francescos original use case of an attached head for direct commits on a stable branch in a submodule. How about we finish discussing the exact solution of that first. AFAIK that is already solved with the following: * Trevor's first patch[2] to create a branch on initial clone of a submodule v1 broke a bunch of tests. Are you ok with v2 [1]? v2 still needs a clearer commit message, a test, and a possible transition to triggering on non-checkout submodule.name.update instead of non-empty submodule.name.branch [2]. Will have a look. That should be all (and IIRC Francesco agreed) needed for that use-case. That was my understanding [3] ;). Thanks for the pointer. Lets not implement more than currently is needed. We can revisit the ideas once some other real use-case manifests. I have most of a real use case already. I have a repository with submodules in one branch (master) and a subtree version in another (assembled) [4]. The *tree* is the same in each case, so I have to 'git rm -rf .' to clear the submodules out of master before I can checkout assembled. $ git checkout assembled error: The following untracked working tree files would be overwritten by checkout: modular/README.md modular/shell/README.md … $ git rm -rf . $ git checkout assembled That leaves some extra stuff removed: $ git status On branch assembled Changes to be committed: (use git reset HEAD file... to unstage) deleted:.gitignore deleted:.mailmap deleted:CONTRIBUTING.md deleted:LICENSE.md deleted:instructor.md so I need to check that out by hand: $ git reset --hard HEAD Now I can work in the assembled branch. Going back to master is a bit less tedious: $ git checkout master $ git submodule update --recursive Luckily for me, I don't have a third superproject branch where the submodules are on a different, so the submodule's HEADs are preserved. As I understand it, the new recursive checkout functionality [5] would checkout my submodules with detached HEADs. The fact that they are only accidentally preserved now is not comforting ;). As it will be opt-in first (you will have to enable it with config options) you can keep your current workflow. Once done with the initial implementation we can discuss and iron out use-cases like yours. Also we (Jens and I) would first like to proceed with the recursive checkout / fetch (for which the plan is clear) as the next complicated step. Once that is done and people gain some experience with it we can still extend further. This is quite reasonable. Given the need for backwards compatibility, I just wanted to make sure my ideal UI was clear before we went forward. There's no need to break fingers twice ;), but if tight binding with localBranch is too big a chunk to bite off now, I'm happy to kick that can down the road. Yes, that would be good to clearly understand and find out what we actually want. Cheers Heiko [1]: http://article.gmane.org/gmane.comp.version-control.git/239967 [2]: http://article.gmane.org/gmane.comp.version-control.git/239973 [3]: http://article.gmane.org/gmane.comp.version-control.git/240139 [4]: (gitweb) http://git.tremily.us/?p=swc-boot-camp.git [5]: http://article.gmane.org/gmane.comp.version-control.git/239695 -- 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: Re: [RFC v2] submodule: Respect requested branch on all clones
2014/1/14 Heiko Voigt hvo...@hvoigt.net: On Tue, Jan 14, 2014 at 02:22:31PM -0800, W. Trevor King wrote: On Tue, Jan 14, 2014 at 10:46:08PM +0100, Heiko Voigt wrote: I would like to step back a bit and get back to the original problem at hand: Francescos original use case of an attached head for direct commits on a stable branch in a submodule. How about we finish discussing the exact solution of that first. AFAIK that is already solved with the following: * Trevor's first patch[2] to create a branch on initial clone of a submodule [...] That should be all (and IIRC Francesco agreed) needed for that use-case. That was my understanding [3] ;). I've been silent these days but yes: I confirm Trevor's second patch iteration[1] gives, IMO, a better meaning of the submodule.name.branch property and is perfectly fine for my use case, to the point of stopping pursuing for my non behavior changing patch[4] (I still think it deserved some more in depth reviews, though ;) ). Cheers, Francesco [1]: http://article.gmane.org/gmane.comp.version-control.git/239967 [2]: http://article.gmane.org/gmane.comp.version-control.git/239973 [3]: http://article.gmane.org/gmane.comp.version-control.git/240139 [4]: http://article.gmane.org/gmane.comp.version-control.git/239956 -- 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: [RFC v2] submodule: Respect requested branch on all clones
Am 09.01.2014 02:09, schrieb Francesco Pretto: 2014/1/9 W. Trevor King wk...@tremily.us: However, submodule.name.local-branch has nothing to do with remote repositories or tracking branches. My bad: this means the feature is still not entirely clear to me. [branch my-feature] remote = origin merge = refs/heads/my-feature [submodule submod] local-branch = my-feature and I don't think Git's config supports such nesting. Aesthetically, It doesn't look very nice. And I'm not sure we even need that. What's wrong with having the branch setting in the .gitmodules file of the my-feature branch? The only problem I can imagine is accidentally merging that into a branch where that isn't set, but that could be solved by a merge helper for the .gitmodules file. I can resuscitate that if folks want, but Heiko felt that my initial consolidation didn't go far enough [2]. If it turns out that we're ok with the current level of consolidation, would you be ok with non-checkout submodule.name.update as the trigger [3]? For me it was ok with what you did: - if just_cloned and config_branch then !git reset --hard -q fi - So yes: at the first clone 'checkout' keeps attached HEAD, while 'merge' and 'rebase' attach to the branch. It have the impression that attaching the head to the given branch for merge and rebase might be the sensible thing to do, but it would be great to hear from users of merge and rebase if that would break anything for them in their current use cases for these settings. If it's not the first clone, you should take no action (and your original patch was ok about this). I'm not sure this is the right thing to do, after all you configured git to follow that branch so I'd expect it to be updated later too, no? Otherwise you might end up with an old version of your branch while upstream is a zillion commits ahead. I think that adding a halfway step between the current status and full(ish) submodule.name.local-branch support is just going to confuse people Well, for now you got some success in confusing me with this local-branch :) At certain point you may ask maintainers what are the accepted features (because all these debates should be about getting, or not getting, endorsement about something) and finalize a patch so people can further review. First I'd like to see a real consensus about what exactly should happen when a branch is configured to be checked out (and if I missed such a summary in this thread, please point me to it ;-). And we should contrast that to the exact checkout and floating branch use cases. So what should happen on initial clone, later updates, updates where the local and the remote branch diverged, when superproject branches are merged (with and without conflicts), on a rebase in the superproject and so on. After that we can discuss about how to implement them (even though I believe we won't need a new submodule command at the end of this process, simply because if it isn't configurable we cannot teach git checkout and friends to do that automatically for us later). And from reading this discussion I believe we need another value for the ignore option which only ignores changes to the SHA-1 but not to work tree modifications of a submodule work tree relative to its HEAD (or make that two: another one which ignores untracked files too and only shows modification of tracked files). Otherwise users of the floating or attached model can easily miss submodule modifications. -- 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: [RFC v2] submodule: Respect requested branch on all clones
On Thu, Jan 09, 2014 at 09:31:13AM +0100, Jens Lehmann wrote: Am 09.01.2014 02:09, schrieb Francesco Pretto: 2014/1/9 W. Trevor King wk...@tremily.us: However, submodule.name.local-branch has nothing to do with remote repositories or tracking branches. My bad: this means the feature is still not entirely clear to me. [branch my-feature] remote = origin merge = refs/heads/my-feature [submodule submod] local-branch = my-feature and I don't think Git's config supports such nesting. Aesthetically, It doesn't look very nice. And I'm not sure we even need that. What's wrong with having the branch setting in the .gitmodules file of the my-feature branch? The only problem I can imagine is accidentally merging that into a branch where that isn't set, but that could be solved by a merge helper for the .gitmodules file. .gitmodules is fine so long as the config can live in the versioned tree. Many (all?) .gitmodules settings can be overridden in .git/config. However, the local-branch setting needs to be both per-submodule and per-superproject-branch, so .git/config doesn't work very well. I think it's better to use something like my .git/modules/submodule-name/config implementation [1] to set this override. This lack of per-superproject-branch overrides applies to all of the submodule.name.* settings, but you're unlikely to want an out-of-tree override for 'path' or a per-superproject-branch override for 'url', 'ignore', 'update', or 'chRecurseSubmodules'. Maybe folks would want per-superproject-branch overrides to 'branch', although I'd prefer we reuse branch.name.merge in the submodule's config for that [2]. On the other hand, maybe an in-tree .gitmodules is good enough, and folks who want a local override can just edit .gitmodules in their local branch? I've never felt the need to override .gitmodules myself (for any setting), so feedback from someone who has would be useful. I can resuscitate that if folks want, but Heiko felt that my initial consolidation didn't go far enough [2]. If it turns out that we're ok with the current level of consolidation, would you be ok with non-checkout submodule.name.update as the trigger [3]? For me it was ok with what you did: - if just_cloned and config_branch then !git reset --hard -q fi - So yes: at the first clone 'checkout' keeps attached HEAD, while 'merge' and 'rebase' attach to the branch. It have the impression that attaching the head to the given branch for merge and rebase might be the sensible thing to do, but it would be great to hear from users of merge and rebase if that would break anything for them in their current use cases for these settings. Which local branch would you attach to before merging? I think 'git submodule update' should always use the current submodule state (attached branch or detached HEAD) [3], and we should have a separate call that explicitly checked out the desired submodule branch [4]. If it's not the first clone, you should take no action (and your original patch was ok about this). I'm not sure this is the right thing to do, after all you configured git to follow that branch so I'd expect it to be updated later too, no? Otherwise you might end up with an old version of your branch while upstream is a zillion commits ahead. Non-clone updates should not change the submodule's *local* branch *name*. They should change the commit that that branch references, otherwise 'git submodule update' would be a no-op ;). First I'd like to see a real consensus about what exactly should happen when a branch is configured to be checked out (and if I missed such a summary in this thread, please point me to it ;-). I don't think we have a consensus yet. A stand-alone outline of my current position is in my v3 RFC [5], but I don't have any buy-in yet ;). And we should contrast that to the exact checkout and floating branch use cases. With my v3 series, there are no more detached HEADs. Folks using checkout updates get a local master branch. I do not change any of the exact checkout (superproject gitlinked sha1) vs. floating (subproject's remote submodule.name.branch via 'update --remote') logic, because that already works well. The problem is the local branch handling, not the update/integration logic. So what should happen on initial clone, For 'add', clone the command line URL and create a new branch 'master' pointing at the commit referenced by the remote's HEAD (or other branch with --branch). For 'update', do the same, except use a local-branch setting to determine the name for the local branch, falling back to 'master' if it is not set. later updates, The same thing that currently happens, with the exception that checkout-style updates should use reset to update the currently-checked
Re: [RFC v2] submodule: Respect requested branch on all clones
Am 09.01.2014 18:32, schrieb W. Trevor King: On Thu, Jan 09, 2014 at 09:31:13AM +0100, Jens Lehmann wrote: Am 09.01.2014 02:09, schrieb Francesco Pretto: 2014/1/9 W. Trevor King wk...@tremily.us: However, submodule.name.local-branch has nothing to do with remote repositories or tracking branches. My bad: this means the feature is still not entirely clear to me. [branch my-feature] remote = origin merge = refs/heads/my-feature [submodule submod] local-branch = my-feature and I don't think Git's config supports such nesting. Aesthetically, It doesn't look very nice. And I'm not sure we even need that. What's wrong with having the branch setting in the .gitmodules file of the my-feature branch? The only problem I can imagine is accidentally merging that into a branch where that isn't set, but that could be solved by a merge helper for the .gitmodules file. .gitmodules is fine so long as the config can live in the versioned tree. Many (all?) .gitmodules settings can be overridden in .git/config. With the exception of path, as that would make no sense at all. However, the local-branch setting needs to be both per-submodule and per-superproject-branch, so .git/config doesn't work very well. I think it's better to use something like my .git/modules/submodule-name/config implementation [1] to set this override. Yes, the local branch should be set in the submodule's .git/config to make operations done inside the submodule work seamlessly. This lack of per-superproject-branch overrides applies to all of the submodule.name.* settings, but you're unlikely to want an out-of-tree override for 'path' or a per-superproject-branch override for 'url', 'ignore', 'update', or 'chRecurseSubmodules'. Unlikely it is not ;-) We do have people who set update=none in the .git/config of the superproject for submodules they don't have access to (and which is not necessary for their work). And it isn't a per-superproject-branch override but a per-superproject-branch default which can be overridden in .git/config (except for 'update', but I intend to fix that). Maybe folks would want per-superproject-branch overrides to 'branch', although I'd prefer we reuse branch.name.merge in the submodule's config for that [2]. But that might still have to be synced with what the superproject wants. Maybe manually, maybe automatically on checkout. Dunno yet. On the other hand, maybe an in-tree .gitmodules is good enough, and folks who want a local override can just edit .gitmodules in their local branch? I've never felt the need to override .gitmodules myself (for any setting), so feedback from someone who has would be useful. That way these changes would propagate to others working on the same branch when pushing, which I believe is a feature. I can resuscitate that if folks want, but Heiko felt that my initial consolidation didn't go far enough [2]. If it turns out that we're ok with the current level of consolidation, would you be ok with non-checkout submodule.name.update as the trigger [3]? For me it was ok with what you did: - if just_cloned and config_branch then !git reset --hard -q fi - So yes: at the first clone 'checkout' keeps attached HEAD, while 'merge' and 'rebase' attach to the branch. It have the impression that attaching the head to the given branch for merge and rebase might be the sensible thing to do, but it would be great to hear from users of merge and rebase if that would break anything for them in their current use cases for these settings. Which local branch would you attach to before merging? I think 'git submodule update' should always use the current submodule state (attached branch or detached HEAD) [3], and we should have a separate call that explicitly checked out the desired submodule branch [4]. Like we currently do with git submodule update --remote (where you have to have an explicit command telling git when to advance the branch)? Having a separate call that does something *after* a git command is exactly the problem I'm trying to fix with recursive update, so I'm not terribly excited ;-) If it's not the first clone, you should take no action (and your original patch was ok about this). I'm not sure this is the right thing to do, after all you configured git to follow that branch so I'd expect it to be updated later too, no? Otherwise you might end up with an old version of your branch while upstream is a zillion commits ahead. Non-clone updates should not change the submodule's *local* branch *name*. They should change the commit that that branch references, otherwise 'git submodule update' would be a no-op ;). Okay, I seem to have misunderstood that. But what happens when the branch setting in .gitmodules changes, shouldn't that be updated? First I'd like to see a
Re: [RFC v2] submodule: Respect requested branch on all clones
On Thu, Jan 09, 2014 at 08:23:07PM +0100, Jens Lehmann wrote: Am 09.01.2014 18:32, schrieb W. Trevor King: However, the local-branch setting needs to be both per-submodule and per-superproject-branch, so .git/config doesn't work very well. I think it's better to use something like my .git/modules/submodule-name/config implementation [1] to set this override. Yes, the local branch should be set in the submodule's .git/config to make operations done inside the submodule work seamlessly. Once you're inside the submodule my local-branch setting shouldn't matter, because it just connects superproject branches with submodule branches. The submodule's config is just a convenient out-of-tree place to store per-submodule overrides. This lack of per-superproject-branch overrides applies to all of the submodule.name.* settings, but you're unlikely to want an out-of-tree override for 'path' or a per-superproject-branch override for 'url', 'ignore', 'update', or 'chRecurseSubmodules'. Unlikely it is not ;-) We do have people who set update=none in the .git/config of the superproject for submodules they don't have access to (and which is not necessary for their work). That is not a per-superproject-branch override. local-branch is the only per-submodule config I can think of where I can imagine a sane person actually wanting an out-of-tree per-superproject-branch override. And it isn't a per-superproject-branch override but a per-superproject-branch default which can be overridden in .git/config (except for 'update', but I intend to fix that). You're talking about .gitmodules vs. .git/config here, but for local-branch, I'm talking about a fallback chain like [1]: 1. superproject.superproject-branch.local-branch in the submodule's config (superproject/.git/modules/≤submodule-name/config). 2. submodule.submodule-name.local-branch in the superproject's config (.git/config). 3. submodule.submodule-name.local-branch in the superproject's .gitmodules file. 4. default to 'master' Only #1 is a new idea. On the other hand, maybe an in-tree .gitmodules is good enough, and folks who want a local override can just edit .gitmodules in their local branch? I've never felt the need to override .gitmodules myself (for any setting), so feedback from someone who has would be useful. That way these changes would propagate to others working on the same branch when pushing, which I believe is a feature. Sure. Unless they don't want to propagate them, at which point they use an out-of-tree override masking the .gitmodules value. The question is, would folks want local overrides for local-branch (like they do for submodule.name.update), or not? Since it's easy to do [1], I don't see the point of *not* supporting per-superproject-branch overrides. It have the impression that attaching the head to the given branch for merge and rebase might be the sensible thing to do, but it would be great to hear from users of merge and rebase if that would break anything for them in their current use cases for these settings. Which local branch would you attach to before merging? I think 'git submodule update' should always use the current submodule state (attached branch or detached HEAD) [3], and we should have a separate call that explicitly checked out the desired submodule branch [4]. Like we currently do with git submodule update --remote (where you have to have an explicit command telling git when to advance the branch)? Having a separate call that does something *after* a git command is exactly the problem I'm trying to fix with recursive update, so I'm not terribly excited ;-) I'm all for rolling my 'git submodule checkout' into 'git checkout --recurse-submodules' [2]. It was just faster to mock up in shell while we decide how it should work. If it's not the first clone, you should take no action (and your original patch was ok about this). I'm not sure this is the right thing to do, after all you configured git to follow that branch so I'd expect it to be updated later too, no? Otherwise you might end up with an old version of your branch while upstream is a zillion commits ahead. Non-clone updates should not change the submodule's *local* branch *name*. They should change the commit that that branch references, otherwise 'git submodule update' would be a no-op ;). Okay, I seem to have misunderstood that. But what happens when the branch setting in .gitmodules changes, shouldn't that be updated? Not by 'git submodule update'. If there are no out-of-tree overrides and the user calls 'git submodule checkout' with a new local-branch in .gitmodules, *that* should checkout a new submodule branch. First I'd like to see a real consensus about what exactly should happen when a branch is configured to be checked out (and if I missed such a summary in this thread, please point me to it ;-). I don't think we
Re: [RFC v2] submodule: Respect requested branch on all clones
Am 09.01.2014 20:55, schrieb W. Trevor King: On Thu, Jan 09, 2014 at 08:23:07PM +0100, Jens Lehmann wrote: Am 09.01.2014 18:32, schrieb W. Trevor King: However, the local-branch setting needs to be both per-submodule and per-superproject-branch, so .git/config doesn't work very well. I think it's better to use something like my .git/modules/submodule-name/config implementation [1] to set this override. Yes, the local branch should be set in the submodule's .git/config to make operations done inside the submodule work seamlessly. Once you're inside the submodule my local-branch setting shouldn't matter, because it just connects superproject branches with submodule branches. The submodule's config is just a convenient out-of-tree place to store per-submodule overrides. Now I get it, you want to be able to override a submodule branch for every superproject branch. I'm not sure I'd add that in the first iteration though, as it seems to add quite some complexity and I'm not convinced yet users really need it (but I won't object when we find real world use cases for that). This lack of per-superproject-branch overrides applies to all of the submodule.name.* settings, but you're unlikely to want an out-of-tree override for 'path' or a per-superproject-branch override for 'url', 'ignore', 'update', or 'chRecurseSubmodules'. Unlikely it is not ;-) We do have people who set update=none in the .git/config of the superproject for submodules they don't have access to (and which is not necessary for their work). That is not a per-superproject-branch override. local-branch is the only per-submodule config I can think of where I can imagine a sane person actually wanting an out-of-tree per-superproject-branch override. Oops, again I managed to miss the per-superproject*-branch* part. And it isn't a per-superproject-branch override but a per-superproject-branch default which can be overridden in .git/config (except for 'update', but I intend to fix that). You're talking about .gitmodules vs. .git/config here, but for local-branch, I'm talking about a fallback chain like [1]: 1. superproject.superproject-branch.local-branch in the submodule's config (superproject/.git/modules/≤submodule-name/config). 2. submodule.submodule-name.local-branch in the superproject's config (.git/config). 3. submodule.submodule-name.local-branch in the superproject's .gitmodules file. 4. default to 'master' Only #1 is a new idea. Thanks for the explanation, now I understand what you're aiming at. On the other hand, maybe an in-tree .gitmodules is good enough, and folks who want a local override can just edit .gitmodules in their local branch? I've never felt the need to override .gitmodules myself (for any setting), so feedback from someone who has would be useful. That way these changes would propagate to others working on the same branch when pushing, which I believe is a feature. Sure. Unless they don't want to propagate them, at which point they use an out-of-tree override masking the .gitmodules value. The question is, would folks want local overrides for local-branch (like they do for submodule.name.update), or not? Since it's easy to do [1], I don't see the point of *not* supporting per-superproject-branch overrides. Unless actual use cases are shown I'd vote for YAGNI here. A new config option means considerable maintenance burden, no matter how easy it is to implement in the first place. It have the impression that attaching the head to the given branch for merge and rebase might be the sensible thing to do, but it would be great to hear from users of merge and rebase if that would break anything for them in their current use cases for these settings. Which local branch would you attach to before merging? I think 'git submodule update' should always use the current submodule state (attached branch or detached HEAD) [3], and we should have a separate call that explicitly checked out the desired submodule branch [4]. Like we currently do with git submodule update --remote (where you have to have an explicit command telling git when to advance the branch)? Having a separate call that does something *after* a git command is exactly the problem I'm trying to fix with recursive update, so I'm not terribly excited ;-) I'm all for rolling my 'git submodule checkout' into 'git checkout --recurse-submodules' [2]. It was just faster to mock up in shell while we decide how it should work. Sure. As I said that's perfectly fine for testing this approach, but we should do that right in git checkout and friends and not add yet another submodule command. If it's not the first clone, you should take no action (and your original patch was ok about this). I'm not sure this is the right thing to do, after all you configured git to follow that branch so I'd expect it to be updated later too, no? Otherwise you might end up with an old
Re: [RFC v2] submodule: Respect requested branch on all clones
On Thu, Jan 09, 2014 at 10:40:52PM +0100, Jens Lehmann wrote: Am 09.01.2014 20:55, schrieb W. Trevor King: On Thu, Jan 09, 2014 at 08:23:07PM +0100, Jens Lehmann wrote: Am 09.01.2014 18:32, schrieb W. Trevor King: However, the local-branch setting needs to be both per-submodule and per-superproject-branch, so .git/config doesn't work very well. I think it's better to use something like my .git/modules/submodule-name/config implementation [1] to set this override. Yes, the local branch should be set in the submodule's .git/config to make operations done inside the submodule work seamlessly. Once you're inside the submodule my local-branch setting shouldn't matter, because it just connects superproject branches with submodule branches. The submodule's config is just a convenient out-of-tree place to store per-submodule overrides. Now I get it, you want to be able to override a submodule branch for every superproject branch. I'm not sure I'd add that in the first iteration though, as it seems to add quite some complexity and I'm not convinced yet users really need it (but I won't object when we find real world use cases for that). Not much complexity in the code, it's all in the first patch of my v3 series [1]. Adding a new override location doesn't seem that complicated to me, but I haven't been very successful at getting this idea across, so maybe it's weirder than I think ;). Clearer explanations welcome ;). And it isn't a per-superproject-branch override but a per-superproject-branch default which can be overridden in .git/config (except for 'update', but I intend to fix that). You're talking about .gitmodules vs. .git/config here, but for local-branch, I'm talking about a fallback chain like [1]: 1. superproject.superproject-branch.local-branch in the submodule's config (superproject/.git/modules/≤submodule-name/config). 2. submodule.submodule-name.local-branch in the superproject's config (.git/config). 3. submodule.submodule-name.local-branch in the superproject's .gitmodules file. 4. default to 'master' Only #1 is a new idea. Thanks for the explanation, now I understand what you're aiming at. For additional clarity, my whole v3 series is not super long [2]… ;) On the other hand, maybe an in-tree .gitmodules is good enough, and folks who want a local override can just edit .gitmodules in their local branch? I've never felt the need to override .gitmodules myself (for any setting), so feedback from someone who has would be useful. That way these changes would propagate to others working on the same branch when pushing, which I believe is a feature. Sure. Unless they don't want to propagate them, at which point they use an out-of-tree override masking the .gitmodules value. The question is, would folks want local overrides for local-branch (like they do for submodule.name.update), or not? Since it's easy to do [1], I don't see the point of *not* supporting per-superproject-branch overrides. Unless actual use cases are shown I'd vote for YAGNI here. A new config option means considerable maintenance burden, no matter how easy it is to implement in the first place. Automatically checking out the preferred submodule branch for a given superproject branch already requires a new config option. The per-superproject-branch out-of-tree override just renames it (from submodule.submodule-name.local-branch to superproject.superproject-branch.local-branch). So different names depending on superproject-level or submodule-level config, but still the same option. That doesn't sound like it's adding that much of a maintenance burden. On the other hand, I, personally, have no need for out-of-tree overrides for *any* submodule-related config, so I'm fine if we drop the submodule-level lookup location ;). I'm all for rolling my 'git submodule checkout' into 'git checkout --recurse-submodules' [2]. It was just faster to mock up in shell while we decide how it should work. Sure. As I said that's perfectly fine for testing this approach, but we should do that right in git checkout and friends and not add yet another submodule command. The current C code looked fairly focused on detached HEAD sha1 checkouts, which was so far away from what I think should happen that I didn't know where to start ;). If we like the logic layed out in my v3 series, I'll take another look at the C series and see if I can come up with something. If it's not the first clone, you should take no action (and your original patch was ok about this). I'm not sure this is the right thing to do, after all you configured git to follow that branch so I'd expect it to be updated later too, no? Otherwise you might end up with an old version of your branch while upstream is a zillion commits ahead. Non-clone updates should not change the submodule's *local* branch *name*. They should
Re: Re: [RFC v2] submodule: Respect requested branch on all clones
2014/1/8 W. Trevor King wk...@tremily.us: To elaborate the idea I sketched out here [2], say you want: Superproject branch Submodule branch Upstream branch === === master mastermaster super-featuremastermaster my-feature my-featuremaster other-featureother-feature other-feature That's only going to work with per-superproject-branch configs for both the local and remote branches. Using the same name for both local and remote branches does not work. After long thoughts, I think your idea about a local branch with a differently named remote branch looks interesting but I would be extremely cautious to add a ' submodule.name.local-branch' now. Do we have a similar mechanism on regular repository clones? We can clone and switch to a branch other than master by default, but can we also have a different remote by default? If we don't have it, we shouldn't add it first on submodules, as there's the chance the feature never get coupled on the regular clones. Also, I think you fear too much that this can't be added also later. I think you should pursue your initial proposal of --branch means attached to get it upstream first. It's alone, IMO, a great improvement on submodules. -- 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: Re: [RFC v2] submodule: Respect requested branch on all clones
2014/1/8 W. Trevor King wk...@tremily.us: I also prefer 'checkout' to 'head', because 'checkout' already exists in non-submodule Git for switching between local branches. Reasons I would loosely support 'git submodule checkout' 1) It's true that 'git submodule checkout' would also often run 'git checkout'. Reasons, as an user, I seriously would *not* like 'git submodule checkout' - 1) 'git submodule checkout' would also touch '.git/config' and '.gitmodules', and I don't like much the idea of a 'checkout' command touching config files. It looks dirty. 2) Having 'git checkout', 'git checkout --recurse-submodules' and finally 'git submodule checkout' is too much for me. Also, in my proposal, 'git submodule [tobedecided] --attach' would also merge orphaned commits by default, and 'checkout' is not about merge. Reasons I would fervently support 'git submodule head' 1) It tells immediately that this command is about HEAD of the submodule, and will take care of it. Newcomers would loveit if they don't like their HEAD state; 2) head is unspecific enough to admit it can also touch '.git/config' and '.gitmodules'. Said this, it seems Heiko[1] proposed a similar syntax and the only difference was about names, not behavior of the command to be added (if we eventually take this path, ofc). On Wed, Jan 08, 2014 at 01:17:49AM +0100, Francesco Pretto wrote: # Attach the submodule HEAD to branch. # Also set .git/config 'submodule.module.branch' to branch $ git submodule head -b branch --attach module I prefer submodule.name.local-branch for the submodule's local branch name. I think this was still part of your original misunderstanding about my git submodule head. This command would touch 'branch' property anyway because '-b branch' would still be the remote branch, even in the case you have a 'local-branch' property (maybe to be coupled here with a --local-branch local-branch switch). Greetings, Francesco [1] http://marc.info/?l=gitm=138913435216349w=2 -- 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: [RFC v2] submodule: Respect requested branch on all clones
On Thu, Jan 09, 2014 at 12:07:56AM +0100, Francesco Pretto wrote: After long thoughts, I think your idea about a local branch with a differently named remote branch looks interesting but I would be extremely cautious to add a ' submodule.name.local-branch' now. Do we have a similar mechanism on regular repository clones? The default upstream branch is currently configured with branch.name.merge. Earlier [1], I suggested we piggyback on this for the submodule's upstream branches, and only use submodule.name.branch for the initial setup. That would allow developers to configure upstreams on a per-submodule-branch basis. We should probably fall back to submodule.name.branch if the submodule does not have a branch.name.merge configured. However, submodule.name.local-branch has nothing to do with remote repositories or tracking branches. It just selects the preferred submodule branch for the superproject branch. This will only work for in-tree .gitmodules configs (since the contents are per-branch). I don't have a good idea for where local overrides would live. We'd want something like branch.superproject-branch.submodule.submodule-name.local-branch: [branch my-feature] remote = origin merge = refs/heads/my-feature [submodule submod] local-branch = my-feature and I don't think Git's config supports such nesting. We can clone and switch to a branch other than master by default, but can we also have a different remote by default? Sure, the existing submodule.name.url defines the remote repository, and the existing submodule.name.branch defines the remote branch. The existing code even sets up remote.origin.url and branch.name.merge (to the matching refs/heads/name) in the the submodule's config. Also, I think you fear too much that this can't be added also later. We can add submodule.name.local-branch support later, but I see no reason not to add it on top of Jens and Jonathan's current submodule checkout work. With increasingly robust submodule checkout support in the core, I expect the amount of update logic stored in git-submodule.sh will decrease significantly. I think you should pursue your initial proposal of --branch means attached to get it upstream first. It's alone, IMO, a great improvement on submodules. I can resuscitate that if folks want, but Heiko felt that my initial consolidation didn't go far enough [2]. If it turns out that we're ok with the current level of consolidation, would you be ok with non-checkout submodule.name.update as the trigger [3]? I think that adding a halfway step between the current status and full(ish) submodule.name.local-branch support is just going to confuse people ;). Cheers, Trevor [1]: http://article.gmane.org/gmane.comp.version-control.git/240164 [2]: http://article.gmane.org/gmane.comp.version-control.git/239968 [3]: http://article.gmane.org/gmane.comp.version-control.git/239973 -- 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: [RFC v2] submodule: Respect requested branch on all clones
On Thu, Jan 09, 2014 at 12:54:54AM +0100, Francesco Pretto wrote: 2) Having 'git checkout', 'git checkout --recurse-submodules' and finally 'git submodule checkout' is too much for me. Agreed. Since 'git checkout' already exists and 'git checkout --recurse-submodules' is close [1,2], I think that means we should drop this and start arguing about adjusting 'git checkout --recurse-submodules' to checkout branches as well ;). Also, in my proposal, 'git submodule [tobedecided] --attach' would also merge orphaned commits by default, and 'checkout' is not about merge. And that's good. Bailing with “you have orphaned commits, which you should integrate them with $some_integration_command before checking out a different branch” is better than having overlapping responsibilities between the checkout command and the integration command. Cheers, Trevor [1]: http://thread.gmane.org/gmane.comp.version-control.git/239695 [2]: http://article.gmane.org/gmane.comp.version-control.git/240117 -- 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: [RFC v2] submodule: Respect requested branch on all clones
2014/1/9 W. Trevor King wk...@tremily.us: However, submodule.name.local-branch has nothing to do with remote repositories or tracking branches. My bad: this means the feature is still not entirely clear to me. [branch my-feature] remote = origin merge = refs/heads/my-feature [submodule submod] local-branch = my-feature and I don't think Git's config supports such nesting. Aesthetically, It doesn't look very nice. I can resuscitate that if folks want, but Heiko felt that my initial consolidation didn't go far enough [2]. If it turns out that we're ok with the current level of consolidation, would you be ok with non-checkout submodule.name.update as the trigger [3]? For me it was ok with what you did: - if just_cloned and config_branch then !git reset --hard -q fi - So yes: at the first clone 'checkout' keeps attached HEAD, while 'merge' and 'rebase' attach to the branch. If it's not the first clone, you should take no action (and your original patch was ok about this). I think that adding a halfway step between the current status and full(ish) submodule.name.local-branch support is just going to confuse people Well, for now you got some success in confusing me with this local-branch :) At certain point you may ask maintainers what are the accepted features (because all these debates should be about getting, or not getting, endorsement about something) and finalize a patch so people can further review. -- 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: [RFC v2] submodule: Respect requested branch on all clones
On Thu, Jan 09, 2014 at 02:09:37AM +0100, Francesco Pretto wrote: 2014/1/9 W. Trevor King wk...@tremily.us: [branch my-feature] remote = origin merge = refs/heads/my-feature [submodule submod] local-branch = my-feature and I don't think Git's config supports such nesting. Aesthetically, It doesn't look very nice. The INI syntax does not lend itself to easy nesting, but I'm pretty sure some mapping from (superproject-branch, submodule-name) to submodule-local-branch-name is what we need for submodule checkouts. I'm just not sure where local overides to the per-branch .gitmodules should live. We could turn it around, and store: [superproject superproject-branch] local-branch = submodule-local-branch-name in .git/modules/submodule-name/config, with the UI: $ cd submodule $ git config superproject.superproject-branch.local-branch = submodule-branch Not beautiful, but maybe a bit more palatable, and already supported by Git's current config parser. That's enough that I can work up a patch that will hopefully clarify my position ;). 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
Re: [RFC v2] submodule: Respect requested branch on all clones
Francesco Pretto cez...@gmail.com writes: 2014/1/7 Francesco Pretto cez...@gmail.com: To not break the existing behavior what it's really needed here, IMO, is a submodule.name.attached property that says two things: - at the first clone on git submodule update stay attached to submodule.name.branch; - implies --remote, as it's the only thing that makes sense when the submodules are attached. Unless you decide to go with the proposed approach of Trevor, where submodule.name.branch set means attached (if it's not changed: this thread is quite hard to follow...). To this end, Junio could sync with more long-timers (Heiko?) submodule users/devs to understand if this breaks too much or not. It is not immediately obvious to me why anybody who specifies the submodule.*.branch variable to say I want _that_ branch not to want to be on that branch but in a detached state, so from that perspective, submodule.*.attach feels superfluous. But I'd mostly defer the design discussion to Heiko (and Jens), WTK and you. -- 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: [RFC v2] submodule: Respect requested branch on all clones
W. Trevor King wk...@tremily.us writes:
From: W. Trevor King wk...@tremily.us
The previous code only checked out the requested branch in cmd_add.
This commit moves the branch-checkout logic into module_clone, where
it can be shared by cmd_add and cmd_update. I also update the initial
checkout command to use 'rebase' to preserve branches setup during
module_clone.
I want to see the log message explain the motivation behind it
(i.e. instead of stopping after saying We used to do X, now we do
Y, but also explain why we consider that Y is better than X). Here
is my attempt.
submodule: respect requested branch on all clones
The previous code only checked out the requested branch in cmd_add
but not in cmd_update; this left the user on a detached HEAD after
an update initially cloned, and subsequent updates using rebase or
merge mode will kept the HEAD detached, unless the user moved to the
desired branch himself.
Move the branch-checkout logic into module_clone, where it can be
shared by cmd_add and cmd_update. Also update the initial checkout
command to use 'rebase' to preserve branches setup during
module_clone. This way, unless the user explicitly asks to work on
a detached HEAD, subsequent updates all happen on the specified
branch, which matches the end-user expectation much better.
Signed-off-by: W. Trevor King wk...@tremily.us
Signed-off-by: Junio C Hamano gits...@pobox.com
Please correct me if I misunderstood the intention.
Having writing all the above and then looking at the patch again, it
is not immediately obvious to me where you use rebase when doing
the initial checkout, though.
The current Documentation/git-submodule.txt has:
update::
Update the registered submodules, i.e. clone missing submodules
and checkout the commit specified in the index of the containing
repository. This will make the submodules HEAD be detached unless
`--rebase` or `--merge` is specified or the key
`submodule.$name.update` is set to `rebase`, `merge` or `none`.
Side note but doesn't Francesco's 'checkout' is a valid update mode
need to update this part of the documentation as well?
git-submodule.sh | 34 --
1 file changed, 20 insertions(+), 14 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index 2979197..167d4fa 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -253,6 +253,7 @@ module_clone()
url=$3
reference=$4
depth=$5
+ branch=$6
quiet=
if test -n $GIT_QUIET
then
@@ -306,7 +307,15 @@ module_clone()
echo gitdir: $rel/$a $sm_path/.git
rel=$(echo $a | sed -e 's|[^/][^/]*|..|g')
- (clear_local_git_env; cd $sm_path GIT_WORK_TREE=. git config
core.worktree $rel/$b)
+ (
+ clear_local_git_env
+ cd $sm_path
+ GIT_WORK_TREE=. git config core.worktree $rel/$b
+ case $branch in
+ '') git checkout -f -q ;;
+ ?*) git checkout -f -q -B $branch origin/$branch ;;
+ esac
+ ) || die $(eval_gettext Unable to setup cloned submodule
'\$sm_path')
}
isnumber()
@@ -469,16 +478,7 @@ Use -f if you really want to add it. 2
echo $(eval_gettext Reactivating local git
directory for submodule '\$sm_name'.)
fi
fi
- module_clone $sm_path $sm_name $realrepo $reference
$depth || exit
- (
- clear_local_git_env
- cd $sm_path
- # ash fails to wordsplit ${branch:+-b $branch...}
- case $branch in
- '') git checkout -f -q ;;
- ?*) git checkout -f -q -B $branch origin/$branch ;;
- esac
- ) || die $(eval_gettext Unable to checkout submodule
'\$sm_path')
+ module_clone $sm_path $sm_name $realrepo $reference
$depth $branch || exit
fi
git config submodule.$sm_name.url $realrepo
@@ -787,7 +787,8 @@ cmd_update()
fi
name=$(module_name $sm_path) || exit
url=$(git config submodule.$name.url)
- branch=$(get_submodule_config $name branch master)
+ config_branch=$(get_submodule_config $name branch)
+ branch=${config_branch:-master}
if ! test -z $update
then
update_module=$update
@@ -815,7 +816,7 @@ Maybe you want to use 'update --init'?)
if ! test -d $sm_path/.git -o -f $sm_path/.git
then
- module_clone $sm_path $name $url $reference
$depth || exit
+ module_clone $sm_path $name $url $reference
$depth $config_branch || exit
cloned_modules=$cloned_modules;$name
subsha1=
Re: [RFC v2] submodule: Respect requested branch on all clones
On Tue, Jan 07, 2014 at 10:15:25AM -0800, Junio C Hamano wrote: submodule: respect requested branch on all clones The previous code only checked out the requested branch in cmd_add but not in cmd_update; this left the user on a detached HEAD after an update initially cloned, and subsequent updates using rebase or merge mode will kept the HEAD detached, unless the user moved to the desired branch himself. Move the branch-checkout logic into module_clone, where it can be shared by cmd_add and cmd_update. Also update the initial checkout command to use 'rebase' to preserve branches setup during module_clone. This way, unless the user explicitly asks to work on a detached HEAD, subsequent updates all happen on the specified branch, which matches the end-user expectation much better. This looks reasonable to me, but there are still changes I'd like to make for a v3 (e.g. using submodule.name.update to trigger local branch checkout). However, I'm currently leaning towards a new 'git submodule checkout' command with explicit preferred local submodule branches (see [1]). Maybe this should all wait until Jens rolls out his update implementation [2]? Having writing all the above and then looking at the patch again, it is not immediately obvious to me where you use rebase when doing the initial checkout, though. It's used to shift the local branch reference from from the (arbitrary) cloned remote branch tip to the explicit submodule $sha1. Otherwise the default method for that operation is a HEAD-detaching 'checkout'. I tried to explain it here [3]. W. Trevor King wk...@tremily.us writes: The current Documentation/git-submodule.txt has: update:: Update the registered submodules, i.e. clone missing submodules and checkout the commit specified in the index of the containing repository. This will make the submodules HEAD be detached unless `--rebase` or `--merge` is specified or the key `submodule.$name.update` is set to `rebase`, `merge` or `none`. Side note but doesn't Francesco's 'checkout' is a valid update mode need to update this part of the documentation as well? That would be nice. I don't think his patch changes the docs, and I don't know if mentioning the --checkout option belongs in that patch as well, or in a separate fixup ;). Cheers, Trevor [1]: http://article.gmane.org/gmane.comp.version-control.git/240097 [2]: http://article.gmane.org/gmane.comp.version-control.git/240117 [3]: http://article.gmane.org/gmane.comp.version-control.git/239953 -- 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: [RFC v2] submodule: Respect requested branch on all clones
2014/1/7 Junio C Hamano gits...@pobox.com: Unless you decide to go with the proposed approach of Trevor, where submodule.name.branch set means attached (if it's not changed: this thread is quite hard to follow...). To this end, Junio could sync with more long-timers (Heiko?) submodule users/devs to understand if this breaks too much or not. It is not immediately obvious to me why anybody who specifies the submodule.*.branch variable to say I want _that_ branch not to want to be on that branch but in a detached state, so from that perspective, submodule.*.attach feels superfluous. Junio, for what it concerns me I fully support this patch as, IMO, it makes cleaner the role of the property submodule.name.branch. Because with my original proposal I decided to go non-breaking Heiko and Jens could also take position on this because this patch will represent a small behavior break. Also, and important feature should be added together with this patch: a way to go --remote by default on an attached HEAD. This can be done at least in two ways: - explicit, non breaking way: add a submodule.name.remote property. When set to true it implies --remote when doing git submodule update, both on attached and detached HEAD; - implicit, breaking way: assume --remote when doing git submodule update on an attached HEAD. I am quite sure this will break a couple of submodule tests (I already tried it), probably for marginal reasons. I think this is needed because it makes little sense to having an attached HEAD and git submodule update does nothing. Thank you, Francesco -- 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: [RFC v2] submodule: Respect requested branch on all clones
W. Trevor King wk...@tremily.us writes: On Tue, Jan 07, 2014 at 10:15:25AM -0800, Junio C Hamano wrote: submodule: respect requested branch on all clones The previous code only checked out the requested branch in cmd_add but not in cmd_update; this left the user on a detached HEAD after an update initially cloned, and subsequent updates using rebase or merge mode will kept the HEAD detached, unless the user moved to the desired branch himself. Move the branch-checkout logic into module_clone, where it can be shared by cmd_add and cmd_update. Also update the initial checkout command to use 'rebase' to preserve branches setup during module_clone. This way, unless the user explicitly asks to work on a detached HEAD, subsequent updates all happen on the specified branch, which matches the end-user expectation much better. This looks reasonable to me, but there are still changes I'd like to make for a v3 (e.g. using submodule.name.update to trigger local branch checkout). However, I'm currently leaning towards a new 'git submodule checkout' command with explicit preferred local submodule branches (see [1]). Maybe this should all wait until Jens rolls out his update implementation [2]? Sounds good. I'll backburner this one, then. Having writing all the above and then looking at the patch again, it is not immediately obvious to me where you use rebase when doing the initial checkout, though. It's used to shift the local branch reference from from the (arbitrary) cloned remote branch tip to the explicit submodule $sha1. The objective is not what I was questioning. In the patch I see git checkout -f -q -B $branch origin/$branch used in the module_clone (which I think makes sense), and then cmd_update uses git reset --hard -q to make sure that the working tree matches the commit $sha1 obtained from the superproject's gitlink (which may make $branch diverged from origin/$branch, but still I think it makes sense). But there is no 'rebase' I can see in the codepath, which was what I was puzzled about. -- 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: [RFC v2] submodule: Respect requested branch on all clones
On Tue, Jan 07, 2014 at 08:19:49PM +0100, Francesco Pretto wrote: 2014/1/7 Junio C Hamano gits...@pobox.com: It is not immediately obvious to me why anybody who specifies the submodule.*.branch variable to say I want _that_ branch not to want to be on that branch but in a detached state, so from that perspective, submodule.*.attach feels superfluous. Junio, for what it concerns me I fully support this patch as, IMO, it makes cleaner the role of the property submodule.name.branch. No, submodule.name.branch is the name of the remote-tracking branch for 'update --remote'. In this patch, I'm using it as a hint for the preferred local branch name [1], which I now think was a bad idea. After [2], I think that we should just define the preferred local branch name explicitly (submodule.name.local-branch?). Cheers, Trevor [1]: http://article.gmane.org/gmane.comp.version-control.git/239980 [2]: http://article.gmane.org/gmane.comp.version-control.git/240097 -- 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: [RFC v2] submodule: Respect requested branch on all clones
2014/1/7 W. Trevor King wk...@tremily.us: Junio, for what it concerns me I fully support this patch as, IMO, it makes cleaner the role of the property submodule.name.branch. No. Trevor, maybe it was not clear. But I wanted to say: I fully support *Trevor's* patch... :) -- 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: [RFC v2] submodule: Respect requested branch on all clones
On Tue, Jan 07, 2014 at 11:21:28AM -0800, Junio C Hamano wrote: W. Trevor King wk...@tremily.us writes: On Tue, Jan 07, 2014 at 10:15:25AM -0800, Junio C Hamano wrote: Having writing all the above and then looking at the patch again, it is not immediately obvious to me where you use rebase when doing the initial checkout, though. It's used to shift the local branch reference from from the (arbitrary) cloned remote branch tip to the explicit submodule $sha1. The objective is not what I was questioning. In the patch I see git checkout -f -q -B $branch origin/$branch used in the module_clone (which I think makes sense), and then cmd_update uses git reset --hard -q to make sure that the working tree matches the commit $sha1 obtained from the superproject's gitlink (which may make $branch diverged from origin/$branch, but still I think it makes sense). But there is no 'rebase' I can see in the codepath, which was what I was puzzled about. Ah, thanks. s/rebase/reset/ in the commit message ;). 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
Re: [RFC v2] submodule: Respect requested branch on all clones
2014/1/7 Francesco Pretto cez...@gmail.com: 2014/1/7 Junio C Hamano gits...@pobox.com: It is not immediately obvious to me why anybody who specifies the submodule.*.branch variable to say I want _that_ branch not to want to be on that branch but in a detached state, so from that perspective, submodule.*.attach feels superfluous. Junio, for what it concerns me I fully support *this* patch as, Where this patch is Trevor's one, don't get me wrong... :) -- 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: [RFC v2] submodule: Respect requested branch on all clones
On Tue, Jan 07, 2014 at 09:09:19PM +0100, Francesco Pretto wrote:
2014/1/7 W. Trevor King wk...@tremily.us:
Trevor, maybe it was not clear. But I wanted to say:
I fully support *Trevor's* patch... :)
Which I appreciate ;). I still though I should point out that my
patch *confuses* the role of submodule.name.branch :p.
You are welcome. Also, at your wish, can you please reply also in
public?
Here you go.
I'd be happy to hear ideas about superproject-branch-specific local
overrides to a hypothetical submodule.name.local-branch, in the
event that a developer doesn't like a default set in .gitmodules. If
I could think of a way to do that, we could avoid this heuristic
approach, and make the local submodule.name.local-branch
vs. remote-tracking submodule.name.branch distinction more obvious.
It would also be nice if submodule.name.branch was just an initial
setup-time and detached-HEAD default. If the submodule is on a branch
it would make more sense to use the checked-out branch's @{upstream}.
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
Re: [RFC v2] submodule: Respect requested branch on all clones
2014/1/7 W. Trevor King wk...@tremily.us: I'd be happy to hear ideas about superproject-branch-specific local overrides to a hypothetical submodule.name.local-branch, in the event that a developer doesn't like a default set in .gitmodules. If I could think of a way to do that, we could avoid this heuristic approach, and make the local submodule.name.local-branch vs. remote-tracking submodule.name.branch distinction more obvious. Uh, I think you got it wrong in the other thread: I didn't proposed such feature. I just wanted the attached submodule use case to be supported and of course --branch means attached is even easier to get this. -- 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: Re: [RFC v2] submodule: Respect requested branch on all clones
2014/1/7 Heiko Voigt hvo...@hvoigt.net: One thing is missing though (and I think thats where Francesco came from): What if the developer already has a detached HEAD in the submodule? How does he attach to a branch? For this we need something similar to Francescos attach/detach or Trevors submodule checkout with Junio's checkout HEAD~0 from here[1]. I am still undecided how we should call it. Because of my Idea for feature branch support - --- For the branch attaching feature I would also like something that can actually modify .git/config and for me more importantly .gitmodules. So e.g. if I want to work on a longer lived feature branch in a submodule which I need in a feature branch in the superproject I would do something like this: $ git submodule checkout --gitmodules --merge -b hv/my-cool-feature I said in another thread I said to Junio am not pursuing --attach|--detach anymore, but seeing that now everybody seem to be excited about attached HEAD here we are... Heiko, it's all day I think this syntax: it supports your above git submodule checkout and more. Take attention at the feature branch part! NOTE: the following seems to me compatible with Trevor's submodule.module.branch means attached patch. git submodule head The full syntax is the sum of the following ones: git submodule head [-b branch] [--attach] [--] [path...] git submodule head [-b branch] [--attach] --index [--] [path...] git submodule head --reset [--] [path...] git submodule head --reset --index [--] [path...] (NOTE: --index should be the same as Heiko's above --gitmodules, it means - touch .gitmodules) All the switches combinations follow, explained: # Attach the submodule HEAD to branch. # Also set .git/config 'submodule.module.branch' to branch $ git submodule head -b branch --attach module # Attach the submodule HEAD to 'submodule.module.branch'. # If it does not exists defaults to remote/master $ git submodule head --attach module # Unset .git/config 'submodule.module.branch' # Also attach or detach the HEAD according to what is in .gitmodules: # with Trevor's patch 'submodule.module.branch' set means attached, # unset means detached $ git submodule head --reset module NOTE: feature branch part! # Set .gitmodules 'submodule.module.branch' to branch $ git submodule head -b branch --attach --index module # Unset .gitmodules 'submodule.module.branch' $ git submodule head --reset --index module - Also note that a --detach switch is not needed with Trevor's patch. To resync to a dettached HEAD workflow, when 'submodule.module.branch' is unset in .gitmodule, --reset (without --index) should be enough. What do you think? Better? Thank you, Francesco -- 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: Re: [RFC v2] submodule: Respect requested branch on all clones
On Wed, Jan 08, 2014 at 01:17:49AM +0100, Francesco Pretto wrote:
# Attach the submodule HEAD to branch.
# Also set .git/config 'submodule.module.branch' to branch
$ git submodule head -b branch --attach module
I prefer submodule.name.local-branch for the submodule's local
branch name. I also prefer 'checkout' to 'head', because 'checkout'
already exists in non-submodule Git for switching between local
branches.
# Attach the submodule HEAD to 'submodule.module.branch'.
# If it does not exists defaults to remote/master
$ git submodule head --attach module
Defaulting to the configured local branch is fine, but I think it
should default to 'master' if no local branch is configured. This
should not have anything to do with remote-tracking branches (that's
what 'submodule update' already handles). I don't understand why
remote-tracking-branch integration keeps getting mixed up with
local-branch checkout.
# Unset .git/config 'submodule.module.branch'
# Also attach or detach the HEAD according to what is in .gitmodules:
# with Trevor's patch 'submodule.module.branch' set means attached,
# unset means detached
$ git submodule head --reset module
To me this reads “always detach HEAD” (because it unsets
submodule.name.branch, and submodule.name.branch unset means
detached). Note that I've moved away from “submodule.name.branch
set means attached” towards “we should set per-superproject-branch
submodule.name.local-branch explicitly” [1].
NOTE: feature branch part!
# Set .gitmodules 'submodule.module.branch' to branch
$ git submodule head -b branch --attach --index module
# Unset .gitmodules 'submodule.module.branch'
$ git submodule head --reset --index module
-
These are just manipulating .gitmodules. I think we also need
per-superproject-branch configs under the superproject's .git/ for
developer overrides.
What do you think? Better?
I don't think so. To elaborate the idea I sketched out here [2], say
you want:
Superproject branch Submodule branch Upstream branch
=== ===
master mastermaster
super-featuremastermaster
my-feature my-featuremaster
other-featureother-feature other-feature
That's only going to work with per-superproject-branch configs for
both the local and remote branches. Using the same name for both
local and remote branches does not work.
Let me motivate each of the combinations in the above table:
* master, master, master: The stable trunk.
* super-feature, master, master: A superproject feature that works
with the stock submodule.
* my-feature, my-feature, master: A superproject feature that needs an
improved submodule, but wants to integrate upstream master changes
during development.
* other-feature, other-feature, other-feature: A superproject feature
that needs an improved submodule, and wants to integrate
other-feature changes that are also being developed upstream.
The deal-breaker for recycling submodule.name.branch to also mean
the local branch name is the {my-feature, my-feature, master} case.
Do we want to force submodule developers to always match the upstream
name of the feature branch they'd like to integrate with? What if
there is no upstream my-feature branch (and the superproject developer
pushes submodule branches upstream via email)? Making the local
branch name independently configurable avoids these issues with a
minimal increase in complexity.
Cheers,
Trevor
[1]: http://article.gmane.org/gmane.comp.version-control.git/240177
[2]: http://article.gmane.org/gmane.comp.version-control.git/240180
--
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: [RFC v2] submodule: Respect requested branch on all clones
2014/1/8 W. Trevor King wk...@tremily.us: Note that I've moved away from “submodule.name.branch set means attached” towards “we should set per-superproject-branch submodule.name.local-branch explicitly” [1]. Honestly, I'm having an hard time to follow this thread. Also, you didn't update the patch. If you were endorsed by someone (Junio, Heiko, ...) for the submodule.name.local-branch feature please show me where. I somehow understand the point of the submodule.name.local-branch property, but I can't see the the workflow. Please, show me some hypothetical scripting example with as much complete as possible workflow (creation, developer update, mantainers creates feature branch, developer update, developer attach to another branch). Also, consider I proposed to support the attached HEAD path to reduce complexity and support a simpler use case for git submodules. I would be disappointed if the complexity is reduced in a way and augmented in another. On Wed, Jan 08, 2014 at 01:17:49AM +0100, Francesco Pretto wrote: # Attach the submodule HEAD to branch. # Also set .git/config 'submodule.module.branch' to branch $ git submodule head -b branch --attach module [...] I also prefer 'checkout' to 'head', because 'checkout' already exists in non-submodule Git for switching between local branches. I can agree with similarity to other git commands, but 'checkout' does not give me the idea of something that writes to .git/config or .gitmodules. # Attach the submodule HEAD to 'submodule.module.branch'. # If it does not exists defaults to remote/master $ git submodule head --attach module Defaulting to the configured local branch is fine, but I think it should default to 'master' if no local branch is configured. This should not have anything to do with remote-tracking branches (that's what 'submodule update' already handles). I don't understand why remote-tracking-branch integration keeps getting mixed up with local-branch checkout. Yep, it should default to master, my fault. # Unset .git/config 'submodule.module.branch' # Also attach or detach the HEAD according to what is in .gitmodules: # with Trevor's patch 'submodule.module.branch' set means attached, # unset means detached $ git submodule head --reset module To me this reads “always detach HEAD” (because it unsets submodule.name.branch, and submodule.name.branch unset means detached). I disagree: this would remove only the value in .git/config. If the value is till present in .gitmodules, as I wrote above, the behavior of what is in the index should be respected as for the other properties. Also it gives a nice meaning to a switch like --reset : return to how it was before. NOTE: feature branch part! # Set .gitmodules 'submodule.module.branch' to branch $ git submodule head -b branch --attach --index module # Unset .gitmodules 'submodule.module.branch' $ git submodule head --reset --index module - These are just manipulating .gitmodules. I think we also need per-superproject-branch configs under the superproject's .git/ for developer overrides. I disagree: in my idea the --index switch is a maintainer only command to modify the behavior of the developers and touch only indexed files (.gitmodules, or create a new submodule branch). It expressly don't touch .git/config. -- 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: Re: [RFC v2] submodule: Respect requested branch on all clones
On Sun, Jan 05, 2014 at 10:27:19PM +0100, Francesco Pretto wrote: 2014/1/5 W. Trevor King wk...@tremily.us: On Sun, Jan 05, 2014 at 04:53:12AM +0100, Francesco Pretto wrote: Also it could break some users that rely on the current behavior. The current code always has a detached HEAD after an initial-clone update, regardless of submodule.name.update, which doesn't match those docs either. I perfectly agree with you that the documentation is a bit contradictory with regard to update command and detached HEAD. That's why it's so hard to add a feature and keep the same spirit of those that coded submodules at first. Also, I think that submodules didn't get much feedback with regards to these pitfalls because many people try to setup them, they see that update detaches the HEAD and they think hmmm, maybe submodules are not what I was looking for. I am not so sure about that. Why should detached HEAD make you think like that? For us at $dayjob we have a pre-commit hook that denies you to commit on a detached HEAD and asks you to create a branch first. You then work on that branch and send it out for review. If the reviewer is happy he merges it into a stable branch (master most times) of the submodule. Only revisions that are on a stable branch in a submodule are allowed to be linked in a superprojects branch that should be merged. Before the submodule's branch gets merged we usually track the development branches sha1 of the submodule in the superproject. For cleanup in the submodule I currently use fixup! commits most times so the referenced sha1 is not lost. In the very end when everyone is happy with the submodule change I rebase, change the referenced sha1 in the superproject and send the final branch out for review another time. Adding a check to only checkout submodule.name.branch if submodule.name.update was 'rebase', 'merge', or 'none' would be easy, but I don't think that makes much sense. I can't see any reason for folks who specify submodule.name.branch to prefer a detached HEAD over a local branch matching the remote branch's name. I think the reason is that it still matches the original use case of submodules devs: - the maintainer decides the specific commit developers should have; Nope. We usually do not have a maintainer. We use a review based workflow. Everyone is allowed to review. If you develop you need to send you changes to a reviewer first who then merges when he is ok with it. - developers checkout that commit and don't pull (you can't do git pull in a detached HEAD); Exactly. We consider pull evil ;-) Seriously: To update we only do fast forward merges of local stable branches. Only reviewers or maintainers are allowed to merge and push into stable branches. Direct commits to stable branches are forbidden. To review we have a shortcut to update the stable branch in git gui for which the code can be found on my github[1]. - they optionally get the upstream commit from the specified submodule.name.branch with --remote. They are still in a detached HEAD and can't do git pull. Yes, why would you do a git pull in a submodule? Don't you want to do something like git checkout -t -b dev/my-topic origin/master to start your development? Maybe who coded submodules at first was thinking that the best way to contribute to a project is to checkout that repository, and not work in the submodule. As said, this works well when the submodule repository is a full project, and not a bunch of shared code. Why not work in the submodule? See explanation above. Cheers Heiko [1] https://github.com/hvoigt/git/commits/hv/gui-improvements -- 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: Re: Re: [RFC v2] submodule: Respect requested branch on all clones
On Sun, Jan 05, 2014 at 03:39:43PM -0800, W. Trevor King wrote: On Sun, Jan 05, 2014 at 11:57:33PM +0100, Heiko Voigt wrote: On Sun, Jan 05, 2014 at 01:24:58PM -0800, W. Trevor King wrote: If submodule.name.branch is set, it *always* creates a new local branch of that name pointing to the exact sha1. If submodule.name.branch is not set, we still create a detached-HEAD checkout of the exact sha1. Thanks for this clarification. Since the usual usage with --remote is with a remote-tracking branch, I confused this here. I am not sure whether blindly creating a local branch from the recorded sha1 is the right thing to do. In what situations would that be helpful? In any situation where your going to develop the submodule locally, you're going to want a branch to develop in. Starting local-submodule developers off on a branch seems useful, even if we can only use submodule.name.branch to guess at their preferred local branch name. Sometimes (often?) the guess will be right. However, A detached HEAD will never be right for local development, so being right sometimes is still an improvement ;). Starting developers at a local submodule branch makes sense. But lets think further. What happens after the initial update? Most times the submodule will already be initialized and cloned. Then developers will still get a detached HEAD even with your local branch feature. If there are no changes on it should we advance the local branch somehow on update? If it does not exist anymore should we recreate it? At $dayjob we usually use feature branches for our work. So if someone wants to work in a submodule you simply create a branch at the current sha1 which you then send out for review. I'm all for named feature branches for development, and in this case submodule.name.branch is likely to be the wrong choice. However, it's still safer to develop in that branch and then rename the branch to match your feature than it would be to develop your fix with a detached HEAD. If your developers have enough discipline to always checkout their feature branch before starting development, my patch won't affect them. However, I know a number of folks who go into fight-or-flight mode when they have a detached HEAD :p. I agree having an initial branch makes it less likely to loose committed changes. Thats good. Also starting on some local branch name and then renaming the branch sounds quite practical. Then we could recreate the default local branch on update (like described above). The reason why one would set a branch option here is to share the superproject branch with colleagues. He can make sure they can always fetch and checkout the submodule even though the branch there is still under cleanup and thus will be rebased often. The commit referenced by sha1 would not be available to a developer fetching after a rebase. Yeah, floating gitlinks are something else. I'd be happy to have that functionality (gitlinks pointing to references) should be built into gitlinks themselves, not added as an additional layer in the submodule script. This gitlinked sha1 rebased out of existence scenario is the first I've heard where I think gitlinked references would be useful. Yeah I have been thinking about this for quite a while now, but have not yet found the time to really think it through and come up with a good solution that does not put you in danger of unprecise revisions. The only solution I can think of is a similar approach as submodule.name.branch gives us now but possibly enabled by some option (i.e.: submodule.name.remote = true). This way you always get the current tip of development but still see the differences (which you can choose to commit) in git status. Thinking through this more, perhaps the logic should be: * If submodule.name.update (defaulting to checkout) is checkout, create a detached HEAD. * Otherwise, create a new branch submodule.name.branch (defaulting to master). Why not trigger the attached state with the submodule.name.branch configuration option? If there is a local branch available use that, if not the tracking branch (as it is currently). Then a developer can start working on the branch with: cd submodule; git checkout -t origin/branchname assuming that submodule update learns some more support for this. Isn't that already what 'git update --remote submodule' already does? Does it? As far as I understood (not using the branch option yet) it only does git checkout origin/branchname so there is no local branch created that tracks the remote branch (-t). What I was thinking is that when submodule.name.branch is set a git submodule update will: 1. if no local branch with that name exists: checkout the remote/branch 2. If a local branch with that name exists: checkout the local branch and possibly advance it according to its setting. Thinking further:
Re: [RFC v2] submodule: Respect requested branch on all clones
Heiko Voigt hvo...@hvoigt.net writes: On Sun, Jan 05, 2014 at 10:27:19PM +0100, Francesco Pretto wrote: 2014/1/5 W. Trevor King wk...@tremily.us: On Sun, Jan 05, 2014 at 04:53:12AM +0100, Francesco Pretto wrote: Also it could break some users that rely on the current behavior. The current code always has a detached HEAD after an initial-clone update, regardless of submodule.name.update, which doesn't match those docs either. I perfectly agree with you that the documentation is a bit contradictory with regard to update command and detached HEAD. That's why it's so hard to add a feature and keep the same spirit of those that coded submodules at first. Also, I think that submodules didn't get much feedback with regards to these pitfalls because many people try to setup them, they see that update detaches the HEAD and they think hmmm, maybe submodules are not what I was looking for. I am not so sure about that. Why should detached HEAD make you think like that? For us at $dayjob we have a pre-commit hook that denies you to commit on a detached HEAD and asks you to create a branch first. Perception is irrational ;-) We long-timers think detached is a perfect starting point for both users of submodule who merely want to use the specified commit and developers who want to work on the submodule to match the need of the superproject. The former do not have to do anything, and the latter will have to chdir to the submodule working tree and create a branch (or update the branch with rebase or pull on top of the specified commit) as the first thing before doing anything. Not everybody is a long-timer, but the saving grace is that nobody stays a newcomer. BUT. - developers checkout that commit and don't pull (you can't do git pull in a detached HEAD); Exactly. We consider pull evil ;-) Seriously: To update we only do fast forward merges of local stable branches. ... Yes, why would you do a git pull in a submodule? Don't you want to do something like git checkout -t -b dev/my-topic origin/master to start your development? As long as origin/master contains the commit specified by the superproject, that would work, but it may be a good thing to use a mode of submodule.*.update that does not have to drop the user into a detached state in the first place. I somehow thought that was what rebase (or merge) was about, that is, starting from the state where a branch is checked out in the submodule working tree, an update in the superproject would cause that branch checked out in the submodule brought up to date with respect to the commit specified in the superproject tree. If that is not how it is supposed to work, please correct me---and we may have to add another mode that does so (or even make rebase/merge do so as a bugfix). And wouldn't it make it unnecessary to have a new re-attach option if such a mode that never have to detach is used? -- 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: [RFC v2] submodule: Respect requested branch on all clones
On Mon, Jan 06, 2014 at 04:47:39PM +0100, Heiko Voigt wrote: On Sun, Jan 05, 2014 at 03:39:43PM -0800, W. Trevor King wrote: On Sun, Jan 05, 2014 at 11:57:33PM +0100, Heiko Voigt wrote: On Sun, Jan 05, 2014 at 01:24:58PM -0800, W. Trevor King wrote: Thinking through this more, perhaps the logic should be: * If submodule.name.update (defaulting to checkout) is checkout, create a detached HEAD. * Otherwise, create a new branch submodule.name.branch (defaulting to master). Why not trigger the attached state with the submodule.name.branch configuration option? If there is a local branch available use that, if not the tracking branch (as it is currently). Then a developer can start working on the branch with: cd submodule; git checkout -t origin/branchname assuming that submodule update learns some more support for this. Isn't that already what 'git update --remote submodule' already does? Does it? As far as I understood (not using the branch option yet) it only does git checkout origin/branchname so there is no local branch created that tracks the remote branch (-t). That's right. Anyone who wants to do local development in a submodule should probably not be using checkout updates, hence my proposal above to base local-branch creation on submodule.name.update. What I was thinking is that when submodule.name.branch is set a git submodule update will: 1. if no local branch with that name exists: checkout the remote/branch 2. If a local branch with that name exists: checkout the local branch and possibly advance it according to its setting. This sounds too complicated to me ;). Thinking further: Maybe submodule.name.update = pull could denote that a user wants to have a branch ready for work in a submodule. This sounds like my quoted realization above. We both even preface the idea with thinking ;). However, I think merge, rebase, !command, and all other non-checkout update schemes are already signals that the developer is interested in local developent (and therefore wants a branch), without the need to add an aditional 'pull' (and then how to distinguish between rebase/merge?). submodule update will then 1. if no local branch with that name exists: - automatically create the branch based on the referenced sha1 - set up that its tracking remote/branch With my patch this happens with the initial clone-update (as of v2, only when submodule.name.branch is set. In a hypothetical v3, only when submodule.name.update is not checkout). I'm not sure we want to do this if the user switches to non-checkout updates after the initial cloning update though. They may actually have work in that detached HEAD that we'd be clobbering. - issue a git pull in the submodule I think that updating using the gitlinked sha1 (a local update) and updating using the upstream origin/$branch (a --remote update) should always be two distinct events. Combining them in a single operation just complicates the situation. 2. if a local branch with that name exists: - issue a git pull in the submodule That's what we already have with submodule.name.update as 'merge'. The merged object is either the gitlinked sha1 (a local update) or a re-fetched upstream branch tip (a --remote update). We are on a local branch at this point, but not neccessarily pointing at the gitlinked sha1. The reset here ensures that the new local branch does indeed point at the gitlinked sha1. But isn't this a fresh clone? Why should the branch point at anything else? We don't pass $sha1 to module_clone(). Before my patch, we don't even pass $branch to module_clone(). That means that module_clone() will only checkout the gitlinked sha1 when the upstream HEAD (or $branch with my patch) happens to point to the gitlinked sha1. For example, if Alice adds Charie's repo as a submodule (gitlinking his current master d2dbd39), then Charlie pushes a new commit d0de817 to his master, and then Bob clones Alice's superproject. Post-clone, Charlie's submodule will have checked out Charlie's new d0de817, and we need update's additional: git reset --hard -q d2dbd39 to rewind to Alice's gitlinked sha1. Ah yeah, sorry I was confusing this with the checkout of remote/branch here again. Since I have done that twice already maybe we should be careful about not confusing users with this as well... After wrapping my head around the fact that you want to simply create a local branch on the referenced sha1 (and hopefully remembering it) I still would like to think a little more about it and let it settle a bit. The way I keep this straight is: 1. Submodules are links to Git commits (that's how they're stored in the index). 2. There are two places to look if you want to update the linked commit: a. The superproject's tree (a local updates). b.
Re: [RFC v2] submodule: Respect requested branch on all clones
On Mon, Jan 06, 2014 at 08:56:22AM -0800, Junio C Hamano wrote: Heiko Voigt hvo...@hvoigt.net writes: Yes, why would you do a git pull in a submodule? Don't you want to do something like git checkout -t -b dev/my-topic origin/master to start your development? As long as origin/master contains the commit specified by the superproject, that would work, but it may be a good thing to use a mode of submodule.*.update that does not have to drop the user into a detached state in the first place. I somehow thought that was what rebase (or merge) was about, that is, starting from the state where a branch is checked out in the submodule working tree, an update in the superproject would cause that branch checked out in the submodule brought up to date with respect to the commit specified in the superproject tree. That is my understanding as well. In fact, I don't think the detached-HEAD-vs-branch distinction is important here, you can still rebase your detached HEAD onto the superproject's referenced commit (or origin/$branch with --remote). This will also work for merge, and should work for well-crafted !commands. And wouldn't it make it unnecessary to have a new re-attach option if such a mode that never have to detach is used? I think so, but we currently don't have a never detached route for folks that are cloning submodules via update (instead of via 'submodule add'). Currently, new clone-updates will always leave you with a detached HEAD (unless you have branch-creation in your update !command). My patch aims to close this detached-HEAD gap, for folks we expect will be doing local development, by creating an initial branch at clone-update time. 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
Re: [RFC v2] submodule: Respect requested branch on all clones
W. Trevor King wk...@tremily.us writes: And wouldn't it make it unnecessary to have a new re-attach option if such a mode that never have to detach is used? I think so, but we currently don't have a never detached route for folks that are cloning submodules via update (instead of via 'submodule add'). Currently, new clone-updates will always leave you with a detached HEAD (unless you have branch-creation in your update !command). My patch aims to close this detached-HEAD gap, for folks we expect will be doing local development, by creating an initial branch at clone-update time. I am not a submodule expert so I may be missing some other gaps, but what your change does sounds sensible to me. -- 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: Re: [RFC v2] submodule: Respect requested branch on all clones
2014/1/6 Heiko Voigt hvo...@hvoigt.net: I agree. If we were to support this more easily we could add a configuration option so you can omit the --remote (i.e.: submodule.name.remote=true, as I also suggested in the other email). That way the developer checking out a branch in flight does not even need to know whether (and which) submodules sha1s are still in flight and temporarily set this configuration in the branches .gitmodules file. submodule.name.remote can be useful but can be added later to aid the current use case. To not break the existing behavior what it's really needed here, IMO, is a submodule.name.attached property that says two things: - at the first clone on git submodule update stay attached to submodule.name.branch; - implies --remote, as it's the only thing that makes sense when the submodules are attached. My patch at the current unreleased state does exactly this. Maybe that could actually be the attach operation Francesco is suggesting: git submodule attach [--pull] submodule path branchname will attach the specified submodule to a branch. That means it changes the .gitmodule file accordingly and stages it. With the --pull switch one can specify whether a local branch tracking the remote branch should be automatically created. Names and the command format are just a suggestion here. That way we can support the fork superproject needing submodule changes and send submodule changes upstream first. My patch didn't do this, as the maintainer can do these things quite easily[1] (maintainer is cooler with respect to other devs :) ), but I think it could be good to also have this feature. The feature I think that are still needed and you don't mention are: - an --attached switch for the add command when the maintainer create the submodule the first time (DONE in patch); - a easy way to attach|detach the submodule locally by developer. This should: * fix the head state (DONE in patch); * fix the local .git/config submodule.name.attached property accordingly (DONE in patch, unreleased). I do the latest in the update command but it seems bad to touch .git/config in the update command... Maybe we should have a git submodule head command that does all these things: --attach (for the maintainer), --attach|--detach (for the developer). [1] $ ( cd submodule git branch newbranch git push -u origin HEAD) $ git config -f .gitmodules submodule.newbranch.branch newbranch $ git config -f .gitmodules submodule.newbranch.attached true $ git add . git commit -m Forked superproject git push -- 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: Re: [RFC v2] submodule: Respect requested branch on all clones
2014/1/7 Francesco Pretto cez...@gmail.com: To not break the existing behavior what it's really needed here, IMO, is a submodule.name.attached property that says two things: - at the first clone on git submodule update stay attached to submodule.name.branch; - implies --remote, as it's the only thing that makes sense when the submodules are attached. Unless you decide to go with the proposed approach of Trevor, where submodule.name.branch set means attached (if it's not changed: this thread is quite hard to follow...). To this end, Junio could sync with more long-timers (Heiko?) submodule users/devs to understand if this breaks too much or not. -- 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: [RFC v2] submodule: Respect requested branch on all clones
2014/1/6 Junio C Hamano gits...@pobox.com: As long as origin/master contains the commit specified by the superproject, that would work, but it may be a good thing to use a mode of submodule.*.update that does not have to drop the user into a detached state in the first place. I somehow thought that was what rebase (or merge) was about, that is, starting from the state where a branch is checked out in the submodule working tree, an update in the superproject would cause that branch checked out in the submodule brought up to date with respect to the commit specified in the superproject tree. If that is not how it is supposed to work, please correct me---and we may have to add another mode that does so (or even make rebase/merge do so as a bugfix). I think the mode you are referring to is actually my submodule.name.attached property. As I said to Heiko, the submodule.name.attached property says two things: - at the first clone on git submodule update stay attached to submodule.name.branch; - implies --remote, as it's the only thing that makes sense when the submodules are attached. And wouldn't it make it unnecessary to have a new re-attach option if such a mode that never have to detach is used? I think the reattach|detach option are still needed (it is debatable if we should have something like git submodule head command or we can keep them in git submodule update) because the user may want to do so and doing it requires things should be really atomic: * fix the head state; * set the local .git/config submodule.name.attached property. The --attach switch also can add a great bonus: it can reintegrate orphaned commits when reattaching and having a update mode with merge or rebase. This is already in my patch. -- 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: [RFC v2] submodule: Respect requested branch on all clones
On Sun, Jan 05, 2014 at 08:48:50PM +0100, Heiko Voigt wrote: On Sun, Jan 05, 2014 at 08:17:00AM -0800, W. Trevor King wrote: It's not clear if this refers to the initial-clone update, future post-clone updates, or both. Ideally, the behavior should be the same, but in the initial-clone case we don't have an existing checked-out branch to work with. I do not think that its actually important to end up with a detached HEAD. The documentation just states it because in most cases there is no other option. But I do not think anything will break if a branch points to the exact sha1 we would checkout and we checkout the branch instead. There's no if the remote-tracking branch points to the exact sha1 logic in my patch. If submodule.name.branch is set, it *always* creates a new local branch of that name pointing to the exact sha1. If submodule.name.branch is not set, we still create a detached-HEAD checkout of the exact sha1. Thinking through this more, perhaps the logic should be: * If submodule.name.update (defaulting to checkout) is checkout, create a detached HEAD. * Otherwise, create a new branch submodule.name.branch (defaulting to master). The motivation is that if submodule.name.update is checkout, the user is unlikely to be developing locally in the submodule, as subsequent updates would clobber their local commits. Having a detached HEAD is a helpful don't develop here reminder ;). If submodule.name.update is set, the user is likely to be developing locally, and will probably want a local branch already checked out to facilitate that. - module_clone $sm_path $name $url $reference $depth || exit + module_clone $sm_path $name $url $reference $depth $config_branch || exit In the simple case (update=checkout, no branch specified) with the new checkout branch stuff in module_clone() this code here ends up calling checkout twice. First for master and then here later with the sha1. This feels a little bit double. There is no guarantee that the remote master and the exact sha1 point at the same commit. Ideally we'd just clone the exact sha1 in this case. I would prefer if we skip the checkout in module_clone() if its not necessary. When I tried to drop the '' case here: @@ -306,7 +307,15 @@ module_clone() echo gitdir: $rel/$a $sm_path/.git rel=$(echo $a | sed -e 's|[^/][^/]*|..|g') - (clear_local_git_env; cd $sm_path GIT_WORK_TREE=. git config core.worktree $rel/$b) + ( + clear_local_git_env + cd $sm_path + GIT_WORK_TREE=. git config core.worktree $rel/$b + case $branch in + '') git checkout -f -q ;; + ?*) git checkout -f -q -B $branch origin/$branch ;; + esac + ) || die $(eval_gettext Unable to setup cloned submodule '\$sm_path') } I got test-suite errors that I didn't get to the bottom of. However… How about we move the whole what to checkout-decision into one place instead of having it in update() and moving it from add() into module_clone() ? …this sounds like a good idea to me. However, it would be a more intrusive change, and there may be conflicts with Francesco's proposed attach/detach functionality. I'll wait until we have a clearer idea of where that is headed before I attempt a more complete consolidation. - update_module= ;; + if test -n $config_branch; then + update_module=!git reset --hard -q If we get here the checkout has already been done. Shouldn't this rather specify a noop. I.E. like update_module=!true We are on a local branch at this point, but not neccessarily pointing at the gitlinked sha1. The reset here ensures that the new local branch does indeed point at the gitlinked sha1. 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
Re: [RFC v2] submodule: Respect requested branch on all clones
2014/1/5 W. Trevor King wk...@tremily.us: On Sun, Jan 05, 2014 at 04:53:12AM +0100, Francesco Pretto wrote: Also it could break some users that rely on the current behavior. The current code always has a detached HEAD after an initial-clone update, regardless of submodule.name.update, which doesn't match those docs either. I perfectly agree with you that the documentation is a bit contradictory with regard to update command and detached HEAD. That's why it's so hard to add a feature and keep the same spirit of those that coded submodules at first. Also, I think that submodules didn't get much feedback with regards to these pitfalls because many people try to setup them, they see that update detaches the HEAD and they think hmmm, maybe submodules are not what I was looking for. Adding a check to only checkout submodule.name.branch if submodule.name.update was 'rebase', 'merge', or 'none' would be easy, but I don't think that makes much sense. I can't see any reason for folks who specify submodule.name.branch to prefer a detached HEAD over a local branch matching the remote branch's name. I think the reason is that it still matches the original use case of submodules devs: - the maintainer decides the specific commit developers should have; - developers checkout that commit and don't pull (you can't do git pull in a detached HEAD); - they optionally get the upstream commit from the specified submodule.name.branch with --remote. They are still in a detached HEAD and can't do git pull. Maybe who coded submodules at first was thinking that the best way to contribute to a project is to checkout that repository, and not work in the submodule. As said, this works well when the submodule repository is a full project, and not a bunch of shared code. If they prefer checkout updates, the first such update will return them to a detached HEAD. If they prefer merge/rebase updates, future updates will keep them on the same branch. All my commit does is setup that initial branch for folks who get the submodule via 'update', in the same way it's currently setup for folks who get the submodule via 'add'. My patch is in the same spirit but wants to obtain the same by being 100% additive and by not altering existing behavior in any way. Also it covers: - an autoremote behavior when the user wants an attached HEAD: with your patch --remote is still needed to really update the branch with git submodule update: - voluntary reattach/detach the HEAD with command line; - ff-only merge of changes in the case of checkout in an attached HEAD (doing git checkout branch is not enough); - reattach of the HEAD with orphaned commits. Unfortunately our patches are already a bit colliding. I'll wait for other comments from git maintainers and let see. Anyway, I'm happy because things are moving: after this debate git submodules will be better for sure. Cheers, Francesco -- 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: [RFC v2] submodule: Respect requested branch on all clones
On Sun, Jan 05, 2014 at 10:27:19PM +0100, Francesco Pretto wrote: 2014/1/5 W. Trevor King: Adding a check to only checkout submodule.name.branch if submodule.name.update was 'rebase', 'merge', or 'none' would be easy, but I don't think that makes much sense. I can't see any reason for folks who specify submodule.name.branch to prefer a detached HEAD over a local branch matching the remote branch's name. I think the reason is that it still matches the original use case of submodules devs: - the maintainer decides the specific commit developers should have; - developers checkout that commit and don't pull (you can't do git pull in a detached HEAD); - they optionally get the upstream commit from the specified submodule.name.branch with --remote. They are still in a detached HEAD and can't do git pull. Maybe who coded submodules at first was thinking that the best way to contribute to a project is to checkout that repository, and not work in the submodule. As said, this works well when the submodule repository is a full project, and not a bunch of shared code. You're saying that the detached HEAD is a feature because it breaks pull? And developers can't be trusted/trained to just not pull reflexively? I'm not buying that ;). Although I was sad to see jc/pull-training-wheel dropped and have that discussion stall out [1]. If they prefer checkout updates, the first such update will return them to a detached HEAD. If they prefer merge/rebase updates, future updates will keep them on the same branch. All my commit does is setup that initial branch for folks who get the submodule via 'update', in the same way it's currently setup for folks who get the submodule via 'add'. My patch is in the same spirit but wants to obtain the same by being 100% additive and by not altering existing behavior in any way. My v2 patch doesn't break the current test suite. I'd be surprised if a change in such peripheral existing behavior as the post clone-update branch actually break any user code, but I'd be happy to see links that prove me wrong. Also it covers: - an autoremote behavior when the user wants an attached HEAD: with your patch --remote is still needed to really update the branch with git submodule update: - voluntary reattach/detach the HEAD with command line; - ff-only merge of changes in the case of checkout in an attached HEAD (doing git checkout branch is not enough); - reattach of the HEAD with orphaned commits. Personally, I don't think autoremote updates are worth the additional UI complication (hence my alternative patch ;), but I'm open to discussion on this point. Can you make a case for why and explicit --remote update is burdensome? I'm also not entirely clear on the problems avoided or workflows enhanced via the last two entries. Could you sketch an example workflow that makes that more obvious? Unfortunately our patches are already a bit colliding. I'll wait for other comments from git maintainers and let see. Anyway, I'm happy because things are moving: after this debate git submodules will be better for sure. +1. I floated my patch as a proof-of-concept for my side of this debate, but I'm pretty happy with the current setup, so it's hard for me to imagine submodules getting worse as a result of this ;). Cheers, Trevor [1]: http://article.gmane.org/gmane.comp.version-control.git/236372 -- 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: [RFC v2] submodule: Respect requested branch on all clones
On Sun, Jan 05, 2014 at 01:24:58PM -0800, W. Trevor King wrote: On Sun, Jan 05, 2014 at 08:48:50PM +0100, Heiko Voigt wrote: On Sun, Jan 05, 2014 at 08:17:00AM -0800, W. Trevor King wrote: It's not clear if this refers to the initial-clone update, future post-clone updates, or both. Ideally, the behavior should be the same, but in the initial-clone case we don't have an existing checked-out branch to work with. I do not think that its actually important to end up with a detached HEAD. The documentation just states it because in most cases there is no other option. But I do not think anything will break if a branch points to the exact sha1 we would checkout and we checkout the branch instead. There's no if the remote-tracking branch points to the exact sha1 logic in my patch. I know I was more referring to the discussion whether detached HEAD for submodules is important or not. If submodule.name.branch is set, it *always* creates a new local branch of that name pointing to the exact sha1. If submodule.name.branch is not set, we still create a detached-HEAD checkout of the exact sha1. Thanks for this clarification. Since the usual usage with --remote is with a remote-tracking branch, I confused this here. I am not sure whether blindly creating a local branch from the recorded sha1 is the right thing to do. In what situations would that be helpful? At $dayjob we usually use feature branches for our work. So if someone wants to work in a submodule you simply create a branch at the current sha1 which you then send out for review. The reason why one would set a branch option here is to share the superproject branch with colleagues. He can make sure they can always fetch and checkout the submodule even though the branch there is still under cleanup and thus will be rebased often. The commit referenced by sha1 would not be available to a developer fetching after a rebase. Thinking through this more, perhaps the logic should be: * If submodule.name.update (defaulting to checkout) is checkout, create a detached HEAD. * Otherwise, create a new branch submodule.name.branch (defaulting to master). Why not trigger the attached state with the submodule.name.branch configuration option? If there is a local branch available use that, if not the tracking branch (as it is currently). Then a developer can start working on the branch with: cd submodule; git checkout -t origin/branchname assuming that submodule update learns some more support for this. The motivation is that if submodule.name.update is checkout, the user is unlikely to be developing locally in the submodule, as subsequent updates would clobber their local commits. Having a detached HEAD is a helpful don't develop here reminder ;). If submodule.name.update is set, the user is likely to be developing locally, and will probably want a local branch already checked out to facilitate that. - module_clone $sm_path $name $url $reference $depth || exit + module_clone $sm_path $name $url $reference $depth $config_branch || exit In the simple case (update=checkout, no branch specified) with the new checkout branch stuff in module_clone() this code here ends up calling checkout twice. First for master and then here later with the sha1. This feels a little bit double. There is no guarantee that the remote master and the exact sha1 point at the same commit. Ideally we'd just clone the exact sha1 in this case. I would prefer if we skip the checkout in module_clone() if its not necessary. When I tried to drop the '' case here: @@ -306,7 +307,15 @@ module_clone() echo gitdir: $rel/$a $sm_path/.git rel=$(echo $a | sed -e 's|[^/][^/]*|..|g') - (clear_local_git_env; cd $sm_path GIT_WORK_TREE=. git config core.worktree $rel/$b) + ( + clear_local_git_env + cd $sm_path + GIT_WORK_TREE=. git config core.worktree $rel/$b + case $branch in + '') git checkout -f -q ;; + ?*) git checkout -f -q -B $branch origin/$branch ;; + esac + ) || die $(eval_gettext Unable to setup cloned submodule '\$sm_path') } I got test-suite errors that I didn't get to the bottom of. However… How about we move the whole what to checkout-decision into one place instead of having it in update() and moving it from add() into module_clone() ? …this sounds like a good idea to me. However, it would be a more intrusive change, and there may be conflicts with Francesco's proposed attach/detach functionality. I'll wait until we have a clearer idea of where that is headed before I attempt a more complete consolidation. I agree, that would be good. - update_module= ;; + if test -n $config_branch; then + update_module=!git reset --hard -q
Re: Re: [RFC v2] submodule: Respect requested branch on all clones
On Sun, Jan 05, 2014 at 11:57:33PM +0100, Heiko Voigt wrote: On Sun, Jan 05, 2014 at 01:24:58PM -0800, W. Trevor King wrote: If submodule.name.branch is set, it *always* creates a new local branch of that name pointing to the exact sha1. If submodule.name.branch is not set, we still create a detached-HEAD checkout of the exact sha1. Thanks for this clarification. Since the usual usage with --remote is with a remote-tracking branch, I confused this here. I am not sure whether blindly creating a local branch from the recorded sha1 is the right thing to do. In what situations would that be helpful? In any situation where your going to develop the submodule locally, you're going to want a branch to develop in. Starting local-submodule developers off on a branch seems useful, even if we can only use submodule.name.branch to guess at their preferred local branch name. Sometimes (often?) the guess will be right. However, A detached HEAD will never be right for local development, so being right sometimes is still an improvement ;). At $dayjob we usually use feature branches for our work. So if someone wants to work in a submodule you simply create a branch at the current sha1 which you then send out for review. I'm all for named feature branches for development, and in this case submodule.name.branch is likely to be the wrong choice. However, it's still safer to develop in that branch and then rename the branch to match your feature than it would be to develop your fix with a detached HEAD. If your developers have enough discipline to always checkout their feature branch before starting development, my patch won't affect them. However, I know a number of folks who go into fight-or-flight mode when they have a detached HEAD :p. The reason why one would set a branch option here is to share the superproject branch with colleagues. He can make sure they can always fetch and checkout the submodule even though the branch there is still under cleanup and thus will be rebased often. The commit referenced by sha1 would not be available to a developer fetching after a rebase. Yeah, floating gitlinks are something else. I'd be happy to have that functionality (gitlinks pointing to references) should be built into gitlinks themselves, not added as an additional layer in the submodule script. This gitlinked sha1 rebased out of existence scenario is the first I've heard where I think gitlinked references would be useful. Thinking through this more, perhaps the logic should be: * If submodule.name.update (defaulting to checkout) is checkout, create a detached HEAD. * Otherwise, create a new branch submodule.name.branch (defaulting to master). Why not trigger the attached state with the submodule.name.branch configuration option? If there is a local branch available use that, if not the tracking branch (as it is currently). Then a developer can start working on the branch with: cd submodule; git checkout -t origin/branchname assuming that submodule update learns some more support for this. Isn't that already what 'git update --remote submodule' already does? - update_module= ;; + if test -n $config_branch; then + update_module=!git reset --hard -q If we get here the checkout has already been done. Shouldn't this rather specify a noop. I.E. like update_module=!true We are on a local branch at this point, but not neccessarily pointing at the gitlinked sha1. The reset here ensures that the new local branch does indeed point at the gitlinked sha1. But isn't this a fresh clone? Why should the branch point at anything else? We don't pass $sha1 to module_clone(). Before my patch, we don't even pass $branch to module_clone(). That means that module_clone() will only checkout the gitlinked sha1 when the upstream HEAD (or $branch with my patch) happens to point to the gitlinked sha1. For example, if Alice adds Charie's repo as a submodule (gitlinking his current master d2dbd39), then Charlie pushes a new commit d0de817 to his master, and then Bob clones Alice's superproject. Post-clone, Charlie's submodule will have checked out Charlie's new d0de817, and we need update's additional: git reset --hard -q d2dbd39 to rewind to Alice's gitlinked sha1. 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
Re: [RFC v2] submodule: Respect requested branch on all clones
On Sun, Jan 05, 2014 at 03:39:43PM -0800, W. Trevor King wrote: On Sun, Jan 05, 2014 at 11:57:33PM +0100, Heiko Voigt wrote: The reason why one would set a branch option here is to share the superproject branch with colleagues. He can make sure they can always fetch and checkout the submodule even though the branch there is still under cleanup and thus will be rebased often. The commit referenced by sha1 would not be available to a developer fetching after a rebase. Yeah, floating gitlinks are something else. I'd be happy to have that functionality (gitlinks pointing to references) be built into gitlinks themselves, not added as an additional layer in the submodule script. This gitlinked sha1 rebased out of existence scenario is the first I've heard where I think gitlinked references would be useful. On the other hand, if the subproject has such a rebase, a superproject dev can hop into an existing checkout, update around the rebase, add a superproject commit with the fixed sha1, and push that for other superproject devs. The only people who would need *automatic* rebase recovery would be superproject devs update-cloning the subproject. That's a small enough cross-section that I don't think it deserves the ambiguity of gitlink-to-reference. In that case, all you really need is a way to force a recovery gitlink (i.e. add a 'commit' object to the tree by hand). 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
Re: [RFC v2] submodule: Respect requested branch on all clones
On Sun, Jan 05, 2014 at 04:33:14PM -0800, W. Trevor King wrote: The only people who would need *automatic* rebase recovery would be superproject devs update-cloning the subproject. That's a small enough cross-section that I don't think it deserves the ambiguity of gitlink-to-reference. In that case, all you really need is a way to force a recovery gitlink (i.e. add a 'commit' object to the tree by hand). Actually, you recovering by hand is a lot easier. Setup a rebased-away gitlink target: mkdir subproject ( cd subproject git init echo 'Subproject' README git add README git commit -m 'Subproject v1' echo 'Changes' README git commit -am 'Subproject v2' ) mkdir superproject ( cd superproject git init git submodule add ../subproject git commit -m 'Superproject v1' ) ( cd subproject git reset --hard HEAD^ git reflog expire --expire=now --all git gc --aggressive --prune=now ) Then a recursive clone of the superproject dies: $ git clone --recursive superproject super2 Cloning into 'super2'... done. Submodule 'subproject' (/tmp/x/subproject) registered for path 'subproject' Cloning into 'subproject'... done. fatal: reference is not a tree: f589144d16282d1a80d17a9032c6f1d332e38dd0 Unable to checkout 'f589144d16282d1a80d17a9032c6f1d332e38dd0' in submodule path 'subproject' But you still have the submodule checkout (up until the $sha1 setup): $ cd super2 $ git diff diff --git a/subproject b/subproject index f589144..82d4553 16 --- a/subproject +++ b/subproject @@ -1 +1 @@ -Subproject commit f589144d16282d1a80d17a9032c6f1d332e38dd0 +Subproject commit 82d4553fe437ae014f22bbc87a082c6d19e5d9f9-dirty And you can automatically update to match the upstream remote: $ git submodule update --remote --force Submodule path 'subproject': checked out '82d4553fe437ae014f22bbc87a082c6d19e5d9f9' $ git diff diff --git a/subproject b/subproject index f589144..82d4553 16 --- a/subproject +++ b/subproject @@ -1 +1 @@ -Subproject commit f589144d16282d1a80d17a9032c6f1d332e38dd0 +Subproject commit 82d4553fe437ae014f22bbc87a082c6d19e5d9f9 When explicitly updating to the superproject or subproject's (--remote) new tip is so easy, I don't see a need for floating the gitlinks themselves. 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
