Re: [PATCH v4 1/6] submodule: Make 'checkout' update_module explicit
2014/1/16 W. Trevor King wk...@tremily.us: Avoiding useless clones is probably more important than avoiding duplicate Invalid update mode messages. No, it's not duplicate code. I'll explain, please follow me: @@ -803,17 +803,10 @@ cmd_update() update_module=$update else update_module=$(git config submodule.$name.update) - case $update_module in - '') - ;; # Unset update mode - checkout | rebase | merge | none) - ;; # Known update modes - !*) - ;; # Custom update command - *) - die $(eval_gettext Invalid update mode '$update_module' for submodule '$name') - ;; - esac This is a *validation*. It's done before going more through the code and die early. *) + die $(eval_gettext Invalid update mode '$update_module' for submodule '$name') This should be an *assert* - it means if you reach this case statement you (programmer) have messed the code something in the code before. In fact in my original patch I wrote something like invalid update_module at this flow. Please keep both as Junio said. Thanks, 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
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 v3 3/4] submodule: Teach 'add' about a configurable local-branch
I've matured this opinion about local-branch some days ago, but I
couldn't join the discussion because I was extremely busy. Hope it's
is still current (and correct).
2014/1/9 W. Trevor King wk...@tremily.us
@@ -339,7 +339,19 @@ 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)
+ superproject_branch=$(get_current_branch)
+ default_local_branch=$(get_submodule_config $sm_name local-branch)
+ (
+ clear_local_git_env
+ cd $sm_path
+ GIT_WORK_TREE=. git config core.worktree $rel/$b
+ local_branch=$(get_local_branch ${superproject_branch}
${default_local_branch})
+ # ash fails to wordsplit ${branch:+-b $branch...}
+ case $branch in
+ '') git checkout -f -q -B $local_branch ;;
+ ?*) git checkout -f -q -B $local_branch origin/$branch ;;
+ esac
+ ) || die $(eval_gettext Unable to checkout submodule '\$sm_path')
}
also
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.
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 local-branch feature means to my brain the following: I,
maintainer, decide for you, developer, what name should be the branch
you are checking out. While, in general, it makes sense for a
developer to switch to a differently named feature branch that can
pull the original remote branch if he's actively developing (on any
repository, not only a submodule), this leads me to the following
questions: would it be good to introduce such enforcement? Do we allow
something similar on regular repositories? In short I believe this
workflow may reflect a personal attitude. In that case I'm unsure if
git should ease it so specifically.
--
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: [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: [PATCH] git-submodule.sh: Support 'checkout' as a valid update command
2014/1/7 Junio C Hamano gits...@pobox.com: It is not about preference but what we want to convey to the readers. When you start the sentence with Oh, it already works correctly, the readers need to see this sentence finished: It already works, it is handled correctly, but we change the code nevertheless because ...?. Here is my attempt to fill that because ... part: Subject: git-submodule.sh: 'checkout' is a valid update mode 'checkout' is documented as one of the valid values for 'submodule.name.update' variable, and in a repository with the variable set to 'checkout', git submodule update command do update using the 'checkout' mode. However, it has been an accident that the implementation works this way; any unknown value would trigger the same codepath and update using the 'checkout' mode. Tighten the codepath and explicitly list 'checkout' as one of the known update modes, and error out when an unknown update mode is used. Also, teach the codepath that initializes the configuration variable from in-tree .gitmodules that 'checkout' is one of the valid values---the code since ac1fbbda (submodule: do not copy unknown update mode from .gitmodules, 2013-12-02) used to treat the value 'checkout' as unknown and mapped it to 'none', which made little sense. I wouldn't be able to explain the change better than your description. Also, I was under the improper assumption that the change was obvious. Thank you very much for the amended patch description. 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
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: [PATCH 2/2] Introduce git submodule attached update
2014/1/7 Junio C Hamano gits...@pobox.com: Francesco Pretto cez...@gmail.com writes: My bottom line: - For what I understand, detached HEAD it's a way to say hey, you have to stay on this commit. Also don't even think you can push to the upstream branch. This sometimes can't be spurious, as in the use case I wrote above: access control on the remote repositories should be enough. I think maintainers should have the option to make developers to clone a repository starting with an attached HEAD on the branch suggested in submodule.$name.branch; - git submodule update is missing a property to do automatically --remote. I think in the use case I wrote it's really handy to have a git submodule update to act like this. The short version I read in the message is that your workflow, in which partipants want to work on a branch, gets frustrating with the current system only because the default update/initial cloning detaches HEAD and will stay in that state until the user gets out of the detached state manually. Once that initial detachment is fixed, there is no more major issue, as update will stay on that branch. Am I reading you correctly? Yep, you got it correctly. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Introduce git submodule attached update
2014/1/7 Junio C Hamano gits...@pobox.com: Francesco Pretto cez...@gmail.com writes: The developer does it voluntarily, at his responsibility, because he may decide to partecipate more actively to the development of the submodule and still want to use a simple git submodule update to updates his submodules, overriding its configuration as it can be done for other properties like, for example, branch. It is still unclear to me why we need attached/detached mode for that. The developer may want to do an exploratory development, whose result is unknown to deserve to be committed on the specified branch at the beginning, and choose to build on a detached HEAD, which is a perfectly normal thing to do. But the standard way to do so, whether the developer is working in the top-level superproject or in a submodule, would be to just do: cd $there git checkout HEAD^0 or use whatever commit the state to be detached is at instead of HEAD in the above example, no? Because of the overlapping change with the the other patch proposed by Trevor, and to not generate confusion, I will stop for now pursuing for an attach|detach command/switch specific for submodules, waiting for Trevors's patch possible acceptance. After that I will see it still makes sense 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/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
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
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
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: [PATCH 1/2] git-submodule.sh: Support 'checkout' as a valid update command
Ok, applying the suggested modifications and resending shortly.
Thank you,
Francesco
2014/1/6 Junio C Hamano gits...@pobox.com:
Junio C Hamano gits...@pobox.com writes:
W. Trevor King wk...@tremily.us writes:
On Sun, Jan 05, 2014 at 03:50:48AM +0100, Francesco Pretto wrote:
+ case $update_module in
+ '')
+ ;; # Unset update mode
+ checkout | rebase | merge | none)
+ ;; # Known update modes
+ !*)
+ ;; # Custom update command
+ *)
+ update_module=
+ echo 2 warning: invalid update mode for
submodule '$name'
+ ;;
+ esac
I'd prefer `die …` to `echo 2 …`. It's hard to know if mapping
the user's preferred (unknown) update mechanism to 'checkout' is
serious or not.
This commit also makes me think that --rebase, --merge, and --checkout
should be replaced with a single --update={rebase|merge|checkout|!…}
option, but that's probably food for another commit (and a long
finger-breaking deprecation period).
All of the above points sound sensible to me.
I'll tentatively queue this on 'pu' (with the suggested die
update), with some rewording of the log message. The patch needs to
be signed-off, though.
Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-submodule.sh: Support 'checkout' as a valid update command
According to Documentation/gitmodules.txt, 'checkout' is a valid 'submodule.name.update' command. Also git-submodule.sh refers to it and processes it correctly. Reflecting commit 'ac1fbb' to support this syntax and also validate property values during 'update' command, issuing an error if the value found is unknown. Signed-off-by: Francesco Pretto cez...@gmail.com --- git-submodule.sh | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/git-submodule.sh b/git-submodule.sh index 2677f2e..4a30087 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -622,7 +622,7 @@ cmd_init() test -z $(git config submodule.$name.update) then case $upd in - rebase | merge | none) + checkout | rebase | merge | none) ;; # known modes of updating *) echo 2 warning: unknown update mode '$upd' suggested for submodule '$name' @@ -805,6 +805,17 @@ cmd_update() update_module=$update else update_module=$(git config submodule.$name.update) + case $update_module in + '') + ;; # Unset update mode + checkout | rebase | merge | none) + ;; # Known update modes + !*) + ;; # Custom update command + *) + die $(eval_gettext Invalid update mode '$update_module' for submodule '$name') + ;; + esac fi displaypath=$(relative_path $prefix$sm_path) -- 1.8.5.2.229.g4448466.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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: What's cooking in git.git (Jan 2014, #01; Mon, 6)
2014/1/6 Junio C Hamano gits...@pobox.com: - git-submodule.sh: support 'checkout' as a valid update mode Need to pick up a rerolled one. I resent it, can you see it? 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
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: What's cooking in git.git (Jan 2014, #01; Mon, 6)
2014/1/7 Junio C Hamano gits...@pobox.com: The thing is, it takes a non trivial amount of time to run through a single day's integration cycle, and any reroll that comes later in a day once the cycle started may be too late for that day. Otherwise I have to discard the the result of earlier merges and tests and start over from scratch. Got it, thank 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: [PATCH] git-submodule.sh: Support 'checkout' as a valid update command
2014/1/7 Junio C Hamano gits...@pobox.com: Francesco Pretto cez...@gmail.com writes: According to Documentation/gitmodules.txt, 'checkout' is a valid 'submodule.name.update' command. As you can see in the surrounding text, we call the value of submodule.*.update a mode, not a command. Ok. Also git-submodule.sh refers to it and processes it correctly. This present tense puzzles me. If it already refers to checkout and handles it correctly is there anything that needs to be done? Or did you mean it should refer to and process it but it doesn't, so make it so? Like you said, it already refers to checkout and handles it correctly. I think the use of the simple present tense here is correct: it's a fact. Feel free to advice another wording if you prefer. Reflecting commit 'ac1fbb' to support this syntax and also validate property values during 'update' command, issuing an error if the value found is unknown. Sorry, but -ECANNOTPARSE. Not sure what's wrong here, can you explain why it's failing? I'm using git-format-patch/git-send-email with default settings. Also, if you can edit and keep the sign-off (I'm not familiar with the mailing-list maintainer workflow, sorry), feel free to do it. Thanks -- 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: [PATCH] git-submodule.sh: Support 'checkout' as a valid update command
2014/1/7 Junio C Hamano gits...@pobox.com: Sorry, but -ECANNOTPARSE. A bird told me what -ECANNOTPARSE means. Tell me if this comment sounds better: According to Documentation/gitmodules.txt, 'checkout' is a valid 'submodule.name.update' mode. Also git-submodule.sh already refers to it and handles it correctly. Fix cmd_init() to also accept 'checkout' as valid update mode and add a similar validation in cmd_update(), issuing an error if the value read is unknown. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Introduce git submodule attached update
(Hmmpth, forgot signoff...) To whom it may interest, added some CC. 2014/1/5 Francesco Pretto cez...@gmail.com: At the current state, the following use-case is not supported very well in git: - a maintainer adds a submodule, checking out a specific branch of the repository. He doesn't track the upstream submodule revision sha1; - a developer checkout the repository branch decided by the maintainer. Subsequent merge or rebase update operations don't detach the HEAD. To ease the above use-case this patch: - introduces a submodule.module.attached property that, when set to true, ensures that the update operation will result in the HEAD attached to a branch; - introduces --attach|--dettach switches to the submodule update command: they attach/detach the HEAD, overriding submodule.module.attached property value; - introduces --attached-update switch to the add operation. It: * sets submodule.module.attached to true; * sets submodule.module.ignore to all. Using the '--attach' switch or operating in a repository with 'submodule.name.attached' set to 'true' during update will: - checkout a branch with an attached HEAD if the repository was just cloned; - perform a fast-forward only merge of changes if it's a 'checkout' update operation; - reattach the HEAD prior performing a 'merge', 'rebase' or '!command' update operation if the HEAD was found detached. Orphaned commits will also be merged back in the branch. '--attach' or 'submodule.name.attached' set to true also implies '--remote'. Using the '--detach' switch or operating in a repository with 'submodule.name.attached' set to 'false' during update will: - checkout a detached HEAD if the repository was just cloned; - detach the HEAD prior performing a 'merge', 'rebase' or '!command' update operation if the HEAD was found attached. 'submodule.name.attached' works similarly to 'submodule.name.update' property: git copies the values found in .gitmodules in .git/config when performing an init command. update looks for values in .git/config only. '--attach' and '--detach' switches override an opposite behaviour of 'submodule.name.attached' properties. The patch is strongly additive and doesn't break any submodule specific test. It also adds some tests specific to the added feature. --- Documentation/git-submodule.txt| 48 +-- Documentation/gitmodules.txt | 10 +- git-submodule.sh | 154 +++-- t/t7410-submodule-attached-head.sh | 268 + 4 files changed, 457 insertions(+), 23 deletions(-) create mode 100755 t/t7410-submodule-attached-head.sh diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt index bfef8a0..b97eefb 100644 --- a/Documentation/git-submodule.txt +++ b/Documentation/git-submodule.txt @@ -10,13 +10,14 @@ SYNOPSIS [verse] 'git submodule' [--quiet] add [-b branch] [-f|--force] [--name name] - [--reference repository] [--depth depth] [--] repository [path] + [--reference repository] [--attached-update] [--depth depth] + [--] repository [path] 'git submodule' [--quiet] status [--cached] [--recursive] [--] [path...] 'git submodule' [--quiet] init [--] [path...] 'git submodule' [--quiet] deinit [-f|--force] [--] path... 'git submodule' [--quiet] update [--init] [--remote] [-N|--no-fetch] - [-f|--force] [--rebase] [--reference repository] [--depth depth] - [--merge] [--recursive] [--] [path...] + [-f|--force] [--rebase] [--reference repository] [--attach | --detach] + [--depth depth] [--merge] [--recursive] [--] [path...] 'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) n] [commit] [--] [path...] 'git submodule' [--quiet] foreach [--recursive] command @@ -107,6 +108,9 @@ is the superproject and submodule repositories will be kept together in the same relative location, and only the superproject's URL needs to be provided: git-submodule will correctly locate the submodule using the relative URL in .gitmodules. ++ +If `--attached-update` is specified, the property `submodule.name.attached` +will be set to `true` and `submodule.name.ignore` will be set to `all`. status:: Show the status of the submodules. This will print the SHA-1 of the @@ -156,12 +160,15 @@ it contains local modifications. 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`. `none` can be overridden by specifying - `--checkout`. Setting the key `submodule.$name.update` to `!command` - will cause
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: [PATCH 2/2] Introduce git submodule attached update
2014/1/5 Heiko Voigt hvo...@hvoigt.net: Could you please extend the description of your use-case so we can understand your goal better? Just in case you missed the first patch iteration[1]. The following questions directly pop into my mind: - What means the maintainer does not track the submodules sha1? Does that mean the superproject always refers to submodule commits using branches? It means he doesn't need to control other developers commit to be checked out so he sets submodule.name.ignore to all. In this way he and the developers can work actively in their submodule copy. - What happens if you want to go back to an earlier revision? Lets say a tagged release? How is ensured that you get the correct revision in the submodules? submodule.name.branch is one setting that is not copied in .git/config by git submodule init. git submodule update will use the setting in .gitmodules if not overridden voluntarily by the developer in .git/config. The maintainer can change that setting in .gitmodules and commit the change. Modifies will be propagated by the next git pull git submodule update of the developer in the superproject. - In which situations does the developer or maintainer switch between your attached/detached mode? The developer/maintainer does so optionally and voluntarily and it effects only its private working tree. - What is the repository branch which is given to the developer by the maintainer used for? Who creates this branch and who merges into it? The branch of course must exist prior submodule adding. In this use-case it does not really matter who creates it and who merges into it. Everyone with the right to merge into it has to work in the submodule seamlessly, as it was working on separate clone of the same repository used as the submodule. - What are these subsequent merge or rebase update operations? Do you mean everyone has submodule.name.update configured to merge or rebase? subsequent merge or rebase update operations are just the ones after the initial clone/checkout, nothing particular. Greetings, Francesco [1] http://marc.info/?l=gitm=138836829531511w=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: [PATCH 2/2] Introduce git submodule attached update
2014/1/5 Heiko Voigt hvo...@hvoigt.net: Could you please extend the description of your use-case so we can understand your goal better? Maybe I found better words to explain you my goal: the current git submodule use-case threats the submodule as a project independent dependency. My use case threats the submodule as part of the superproject repository. It could be easier to say that in this way submodules would behave very similarly to svn:externals, something that is actually missing in git. My goal is obtain this without altering git behavior for the existing use case. - In which situations does the developer or maintainer switch between your attached/detached mode? As I told you in the other answer this is voluntary done by the developer, as he prefers. I came to the conclusion that the --attach|--detach switches for the update command are not that useful and can be removed. It's still possible to obtain the switch between detached/attached very easily in this way: # Attach submodule $ git config submodule.name.attached true $ git submodule update # Detach submodule $ git config submodule.name.attached false $ git submodule update # Unset property in both .gitmodules and .git/config means - do nothing $ git config --unset submodule.name.attached $ git submodule update Also my submodule.name.attached property at the moment behaves like submodule.name.update: it is copied in .git/config by git submodule init. This is probably a mistake: the overridden value should be stored in .git/config only at the developer will, so the maintainer has still a chance to modify it in .gitmodules and propagate the behavior. I would send an updated patch but at this point I prefer to wait for a full review. 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
[PATCH 2/2] Introduce git submodule attached update
;;
+ checkout)
+ if test -n $attach_module
+ then
+ command=git checkout $subforce -q
+ suffix=$branch
+ die_msg=$(eval_gettext Unable to
checkout banch '\$branch' in submodule path '\$displaypath')
+ say_msg=$(eval_gettext Submodule path
'\$displaypath': checked out branch '\$branch')
+ if test -z $just_cloned -a test
$subsha1 != $sha1
+ then
+ # Perform a fast-forward only
merge of the origin
+ command_post=git merge
$subforce --ff-only
+ suffix_post=origin/$branch
+ fi
+ else
+ command=git checkout $subforce -q
+ suffix=$sha1
+ die_msg=$(eval_gettext Unable to
checkout '\$sha1' in submodule path '\$displaypath')
+ say_msg=$(eval_gettext Submodule path
'\$displaypath': checked out '\$sha1')
+ fi
+ ;;
!*)
command=${update_module#!}
+ suffix=$sha1
die_msg=$(eval_gettext Execution of
'\$command \$sha1' failed in submodule path '\$prefix\$sm_path')
say_msg=$(eval_gettext Submodule path
'\$prefix\$sm_path': '\$command \$sha1')
must_die_on_failure=yes
+ custom_update=yes
;;
*)
- command=git checkout $subforce -q
- die_msg=$(eval_gettext Unable to checkout
'\$sha1' in submodule path '\$displaypath')
- say_msg=$(eval_gettext Submodule path
'\$displaypath': checked out '\$sha1')
+ # Valid user configurable update modes are
already filtered above
+ die $(eval_gettext Unexpected update mode in
the current flow)
;;
esac
- if (clear_local_git_env; cd $sm_path $command
$sha1)
+ if (clear_local_git_env; cd $sm_path
$command_attach $suffix_attach
+ $command_pre $suffix_pre $command
$suffix $command_post $suffix_pro)
then
say $say_msg
elif test -n $must_die_on_failure
diff --git a/t/t7410-submodule-attached-head.sh
b/t/t7410-submodule-attached-head.sh
new file mode 100755
index 000..04b3018
--- /dev/null
+++ b/t/t7410-submodule-attached-head.sh
@@ -0,0 +1,268 @@
+#!/bin/sh
+#
+# Copyright (c) 2014 Francesco Pretto
+#
+
+test_description='Support for submodules with attached head
+
+This test verifies the sanity of the add and update git submodule commands with
+or without the --attached-update, --attach, --detach switches or the
+submoudule.module.attach property set
+'
+
+TEST_NO_CREATE_REPO=true
+. ./test-lib.sh
+
+submodurl1=$(pwd -P)/repo1
+submodurl2=$(pwd -P)/repo2
+repourl=$(pwd -P)/repo
+
+test_expect_success 'setup - create repository repo1 to be used as
submodule' '
+ mkdir repo1
+ (
+ cd repo1
+ git init
+ git config receive.denyCurrentBranch ignore
+ echo a a
+ git add a
+ git commit -m repo1 commit 1
+ )
+'
+
+test_expect_success 'setup - reate repository repo2 to be used as submodule'
'
+ mkdir repo2
+ (
+ cd repo2
+ git init
+ git config receive.denyCurrentBranch ignore
+ echo a a
+ git add a
+ git commit -m repo2 commit 1
+ )
+'
+
+test_expect_success 'setup - create repository repo to be added with
sumodules' '
+ mkdir repo
+ (
+ cd repo
+ git init
+ git config receive.denyCurrentBranch ignore
+ echo a a
+ git add a
+ git commit -m repo commit 1
+ )
+'
+
+test_expect_success 'setup - clone repository repo in repoclone' '
+ git clone $repourl repoclone
+'
+
+test_expect_success 'setup - add mod1 as regular submodule of repo' '
+ (
+ cd repo
+ git submodule add $submodurl1 submod1
+ )
+'
+
+test_expect_success 'setup - add mod2 as update attached HEAD submodule of
repo
[PATCH 1/2] git-submodule.sh: Support 'checkout' as a valid update command
According to Documentation/gitmodules.txt, 'checkout' is a valid 'submodule.name.update' command. Also git-submodule.sh refers to it and processes it correctly. Reflect commit 'ac1fbb' to support this syntax and also validates property values during 'update' command, issuing a warning if the value found is unknwon. --- git-submodule.sh | 14 +- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/git-submodule.sh b/git-submodule.sh index 2677f2e..1d041a7 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -622,7 +622,7 @@ cmd_init() test -z $(git config submodule.$name.update) then case $upd in - rebase | merge | none) + checkout | rebase | merge | none) ;; # known modes of updating *) echo 2 warning: unknown update mode '$upd' suggested for submodule '$name' @@ -805,6 +805,18 @@ cmd_update() update_module=$update else update_module=$(git config submodule.$name.update) + case $update_module in + '') + ;; # Unset update mode + checkout | rebase | merge | none) + ;; # Known update modes + !*) + ;; # Custom update command + *) + update_module= + echo 2 warning: invalid update mode for submodule '$name' + ;; + esac fi displaypath=$(relative_path $prefix$sm_path) -- 1.8.5.2.230.g032cd47.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] submodule: Respect reqested branch on all clones
Thanks for adding your contribute. My comments below: 2014/1/3 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. [...] @@ -306,7 +307,14 @@ 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 + if test -n $branch; then + git checkout -f -q -B $branch origin/$branch echo checked out $branch + fi + ) || die $(eval_gettext Unable to setup cloned submodule '\$sm_path') } If I understand it correctly, looking at your intervention in module_clone and cmd_update, when submodule.module.branch is set during update the resulting first clone will always be a branch checkout (cause $branch is filled with branch property). I believe this will break a lot of tests, as the the documentation says that in this configuration the HEAD should be detached. Also it could break some users that rely on the current behavior. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] Introduce git submodule add|update --attach
2014/1/3 Francesco Pretto cez...@gmail.com: Concluding, my point is that at the current state submodules in git seem to be flawed because of the inconsistent HEAD state between add and update users. With my patch applied the attached HEAD behavior would be fully supported. At some point git submodule add (without the --attached switch) could be also modified to produce a detached HEAD by default, removing any remaining inconsistency. Junio, please let me amend this affirmation as it's inaccurate. According to the current *upstream* features this should the supported use case for submodules: - there's a maintainer add user that adds the submodule. He will need to track the upstream submodule revision sha1, so he will clone the repository with an *attached* HEAD; - there's a developer update use that just clone the submodule repository and track the sha1 decided by the maintainer add user. He won't need to track upstream submodule revision sha1 so cloning the repository with a *detached* HEAD. Subsequent merge or rebase update operations will keep the HEAD detached. We should *not* modify/break this default workflow in any way. The current documentation seems to be misleading when saying that This [the update command] will make the submodules HEAD be detached **unless** --rebase or --merge is specified, as it seems to imply that the update operation will result in a repository with the HEAD attached. The repository will be indeed updated merging changes, but the HEAD will stay detached. Now, the use case I would like git to support is this: - there's a maintainer add user that adds the submodule. He won't track the upstream submodule revision sha1, so submodule.module.ignore will be set to all. He will still clone the repository with an *attached* HEAD; - there's a developer update user. He will clone the submodule repository with an *attached* HEAD. Subsequent merge or rebase update operations will keep the HEAD attached. To fully support this workflow in my patch I want to: - introduce a submodule.module.attach property so update users will clone the submodule repository with an attached HEAD; - introduce --attach|--dettach switches to update commmant to eventually override submodule.module.attach property value in the command line; - introduce --attach (but better rename it to --keep-attached or something else) switch to the add operation. This --keep-attached will: * set submodule.module.attach to true; * set submodule.module.ignore to all. Being the workflow complementary, and the behavior fully optional, I think the proposed feature should be solid. Give me some time to produce an improved patch and let see if I can convince you. Regards, 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: [PATCH/RFC] Introduce git submodule add|update --attach
Thanks for the comments, my replies below. Before, a couple of general questions: - I'm also writing some tests, should I commit them together with the feature patch? - to determine the attached/detached state I did this: head_detached= if test $(rev-parse --abbrev-ref HEAD) = HEAD then head_detached=true fi Is this correct? 2013/12/31 Phil Hord phil.h...@gmail.com On Sun, Dec 29, 2013 at 8:49 PM, Francesco Pretto cez...@gmail.com wrote: [...] When updating, using the '--attach' switch or operating in a repository with 'submodule.name.attach' set to 'true' will: - checkout a branch with an attached HEAD if the repository was just cloned; - perform a fast-forward only merge of changes if it's a 'checkout' update, keeping the HEAD attached; - reattach the HEAD prior performing a 'merge', 'rebase' or '!command' update operation if the HEAD was found detached. I need to understand this reattach the HEAD case better. Can you give some examples of the expected behavior when merge, rebase or !command is encountered? Thanks for pointing this out, actually my patch was a bit lacking at this. Reattaching the HEAD prior to merge, rebase or !command would have caused just a *silent* git checkout branch, possibly leaving orphaned commits forgotten. My plans for the patch are now to implement this safer and, IMO, intuitive behavior: let set say we have a submodule mod with the HEAD detached at orphaned commit orphaned-sha1. Let say origin/branch is at commit origin-sha1. Let say I set submodule.mod.attach to true or I run git submodule update with the --attach switch. The expected behavior for submodule mod would be (pseudocode): git checkout branch if merge and head_detached then git merge orphaned-sha1 case: merge: git merge origin-sha1 rebase: git rebase origin-sha1 !command: command origin-sha1 if rebase and head_detached then git merge orphaned-sha1 So, in both merge|rebase cases we merge back orphaned commits with a git merge, but the effect will be a merge or a rebase depending of the ordering of the main update operation. We can't assume a merge or rebase operation in the case of !command so we let the responsibility of merging back orphaned commits to the user. Sounds good? + +--attach:: + This option is only valid for the add and update commands. Cause the Grammar: 'Causes the result' Ok. + result of an add or update operation to be an attached HEAD. In the + update command , if `submodule.name.branch` is not set, it will typo: space before comma. Ok. Also, the pronoun it here is unclear to me. Does this convey the correct meaning? In the update operation, the branch named by 'submodule.name.branch' is checked out as the new HEAD of the submodule repository. If 'submodule.name.branch' is not set, the 'master' branch is checked out as the new HEAD of the submodule. Sounds good to me. + default to `master`. Note: for the update command `--attach` also + implies `--remote`. + +--detach:: + This option is only valid for the update command. Cause the result Grammar: 'Causes the result' Ok. + of the update operation to be forcedly a detached HEAD. Forcedly is a bit strong, maybe, slightly misplaced, and not a word, besides. How's this, instead: Forces the result of the update operation to be a detached HEAD in the submodule. Sounds good to me. submodule.name.update:: Defines what to do when the submodule is updated by the superproject. - If 'checkout' (the default), the new commit specified in the - superproject will be checked out in the submodule on a detached HEAD. + If 'checkout' (the default), the new commit (or the branch, when using + the '--attach' switch or the 'submodule.name.attach' property is set + to 'true' during an update operation) specified in the superproject will + be checked out in the submodule. IMHO, this wording is overcomplicated by this change. How about: If 'checkout' (the default), the new commit specified in the superproject (or branch, with '--attach') will be checked out in the submodule. Sounds good to me. If 'rebase', the current branch of the submodule will be rebased onto the commit specified in the superproject. If 'merge', the commit specified in the superproject will be merged into the current branch Does the 'merge', 'rebase' and '!command' description need to be updated, too? Here and above it seems to still suggest the old behavior is kept when --attach is used. You are right, I'll improve those. Also I think the documentation was a bit innacurate because, with the current git features state, it's possible merge changes keeping the HEAD detached, and that's what happen. @@ -54,6 +56,11 @@ submodule.name.branch
Re: [PATCH/RFC] Introduce git submodule add|update --attach
2014/1/2 Junio C Hamano gits...@pobox.com: Francesco Pretto cez...@gmail.com writes: by default git submodule performs its add or update operations on a detached HEAD. This works well when using an existing full-fledged/indipendent project as the submodule, as there's less frequent need to update it or commit back changes. When the submodule is actually a large portion of shareable code between different projects, and the superproject needs to track very closely the evolution of the submodule (or the other way around), I feel more confortable to reattach the HEAD of the submodule with an existing branch. I may be missing some fundamental assumption in your mind when you did this change, but in a workflow where somebody wants submodule checkout to be on branches (as opposed to detached), wouldn't it make more sense not to detach in the first place, rather than introducing yet another option to re-attach? The documentation of submodule update seems to say that its merge and rebase modes do not detach in the first place (and it alludes to --checkout but it is unclear what it does purely from the documentation, as git submodule --help does not even list it as one of the options). Thanks for commenting: because of more checking I just spotted some unuseful code in my patch in cmd_add() function. In short: it seems to me git submodule update doesn't allow to work with attached an HEAD (unless it is *manually* attached, as I regularly do). My feeling is the documentation of merge, rebase update commands is inaccurate and doesn't really reflect what happen when adding a submodule with add or cloning it for the first time with update. You can look at the following conditionals in the current code in git-submodule.sh: ## cmd_add() case $branch in '') git checkout -f -q ;; ?*) git checkout -f -q -B $branch origin/$branch ;; esac ## cmd_update() # Is this something we just cloned? case ;$cloned_modules; in *;$name;*) # then there is no local change to integrate update_module= ;; esac This means that the add command will always checkout an *attached* HEAD but at the first clone of a different user the checkout will resolve in git checkout sha1, always producing a *detached* HEAD and resulting in inconsistent HEAD state between who added the submodule with add and who cloned it with update. Subsequent merge or rebase operations won't change this fact, the HEAD will remain detached. The following test case confirms this, unless I did something wrong: - parentdir=$(pwd -P) submodurl1=$(pwd -P)/repo1 submodurl2=$(pwd -P)/repo2 repourl=$(pwd -P)/repo # Create repo to be added with submodules cd $parentdir mkdir repo cd repo git init git config receive.denyCurrentBranch ignore echo a a git add a git commit -m repo commit 1 # Create repo repo1 to be used as submodule cd $parentdir mkdir repo1 cd repo1 git init git config receive.denyCurrentBranch ignore echo a a git add a git commit -m repo1 commit 1 # Create repo repo2 to be used as submodule cd $parentdir mkdir repo2 cd repo2 git init git config receive.denyCurrentBranch ignore echo a a git add a git commit -m repo2 commit 1 # Clone repo to test git submodule update cd $parentdir git clone $repourl repoclone # ## Adding submodule with update rebase, not specifying a branch # cd $parentdir/repo git submodule add $submodurl1 submod1 git config -f .gitmodules submodule.submod1.ignore all git config -f .gitmodules submodule.submod1.update rebase git commit -m Added submodule ## repo/submod1 has an attached HEAD ## cd $parentdir/repoclone git pull git submodule init git submodule update ## repoclone/submod1 has a detached HEAD -- note the inconsistency ## # ## Adding submodule with update rebase, specifying a branch # cd $parentdir/repo git submodule add --branch master $submodurl2 submod2 git config -f .gitmodules submodule.submod2.ignore all git config -f .gitmodules submodule.submod2.update rebase git add . git commit -m Added submodule ## repo/submod2 has a attached HEAD ## cd $parentdir/repoclone git pull git submodule init git submodule update --remote ## repoclone/submod2 has a detached HEAD -- note the inconsistency ## # ## Adding something to submod2 and test update rebase on repoclone # cd $parentdir/repo1 echo b b git add b git commit -m repo1 commit 2 cd $parentdir/repoclone git submodule update --remote ## repoclone/submod1 has still a detached HEAD ## - And if there is a good reason why detaching to update and then (perhaps after verifying the result or cleaning it up? I dunno what the expected use case is, so I am purely guessing) attaching the result to a specific branch in separate steps, does it make sense to give --attach option to update in the first place? That makes the whole thing into a single step, not giving the user
[PATCH/RFC] Introduce git submodule add|update --attach
submodule add --branch master-project1 --attach repository path $ git commit -m Added submodule $ git push # Developer $ git pull $ git submodule init $ git submodule update Is there interest in supporting this workflow and seeing this patch applied? Thanks, Francesco [1] http://marc.info/?l=gitm=138610752125816w=2 [2] Documentation/gitmodules.txt: If 'checkout' (the default)... Signed-off-by: Francesco Pretto cez...@gmail.com --- Documentation/git-submodule.txt | 37 ++--- Documentation/gitmodules.txt| 11 ++- git-submodule.sh| 172 +--- 3 files changed, 197 insertions(+), 23 deletions(-) diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt index bfef8a0..452376d 100644 --- a/Documentation/git-submodule.txt +++ b/Documentation/git-submodule.txt @@ -10,13 +10,14 @@ SYNOPSIS [verse] 'git submodule' [--quiet] add [-b branch] [-f|--force] [--name name] - [--reference repository] [--depth depth] [--] repository [path] + [--reference repository] [--attach] [--depth depth] + [--] repository [path] 'git submodule' [--quiet] status [--cached] [--recursive] [--] [path...] 'git submodule' [--quiet] init [--] [path...] 'git submodule' [--quiet] deinit [-f|--force] [--] path... 'git submodule' [--quiet] update [--init] [--remote] [-N|--no-fetch] - [-f|--force] [--rebase] [--reference repository] [--depth depth] - [--merge] [--recursive] [--] [path...] + [-f|--force] [--rebase] [--reference repository] [--attach | --detach] + [--depth depth] [--merge] [--recursive] [--] [path...] 'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) n] [commit] [--] [path...] 'git submodule' [--quiet] foreach [--recursive] command @@ -107,6 +108,10 @@ is the superproject and submodule repositories will be kept together in the same relative location, and only the superproject's URL needs to be provided: git-submodule will correctly locate the submodule using the relative URL in .gitmodules. ++ +If `--attach` is specified, the submodule will be registered to be +checked out with an an attached HEAD. Also `submodule.name.attach` will +be set to `true` and `submodule.name.ignore` will be set to `all`. status:: Show the status of the submodules. This will print the SHA-1 of the @@ -156,12 +161,15 @@ it contains local modifications. 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`. `none` can be overridden by specifying - `--checkout`. Setting the key `submodule.$name.update` to `!command` - will cause `command` to be run. `command` can be any arbitrary shell - command that takes a single argument, namely the sha1 to update to. + This will make the submodules HEAD be detached unless `--attach` is + specified or `submodule.$name.attach` is set to `true`. The last setting + can always be overridden specifying `--detach`. Update mode can be + selected specifying `--checkout`, `--rebase` or `--merge` switches + or setting the key `submodule.$name.update` to `checkout`, `rebase`, + `merge` or `none`. `none` will cause the submodule to be skipped during + the update. Setting the key `submodule.$name.update` to `!command` will + cause `command` to be run. `command` can be any arbitrary shell command + that takes a single argument, namely the sha1 to update to. + If the submodule is not yet initialized, and you just want to use the setting as stored in .gitmodules, you can automatically initialize the @@ -270,6 +278,17 @@ OPTIONS be overridden by setting the `submodule.name.branch` option in either `.gitmodules` or `.git/config` (with `.git/config` taking precedence). + +--attach:: + This option is only valid for the add and update commands. Cause the + result of an add or update operation to be an attached HEAD. In the + update command , if `submodule.name.branch` is not set, it will + default to `master`. Note: for the update command `--attach` also + implies `--remote`. + +--detach:: + This option is only valid for the update command. Cause the result + of the update operation to be forcedly a detached HEAD. + This works for any of the supported update procedures (`--checkout`, `--rebase`, etc.). The only change is the source of the target SHA-1. diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt index f7be93f..e6c3360 100644 --- a/Documentation/gitmodules.txt +++ b/Documentation/gitmodules.txt @@ -37,8 +37,10 @@ submodule.name.url
