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: 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: 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: 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: 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: 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: 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
