# [PATCH v3 1/3] git-submodule add: Add -r/--record option 49 messages from 2012-11-09 to 2012-11-29. Participants: W. Trevor King, Junio C Hamano, Heiko Voigt, Sascha Cunz, Jens Lehmann, Phil Hord. Thread: https://gitlist.dev/t/32045 ## W. Trevor King, 2012-11-09 03:35 Subject: [PATCH v3 0/3] git-submodule add: Add -r/--record option Message-ID: URL: https://gitlist.dev/e/cover.1352431674.git.wking%40tremily.us In-Reply-To: <20121029222759.GI20513@sigill.intra.peff.net> ``` From: "W. Trevor King" Here's my revised patch. Changes from v2: * Revised Ævar-vs-Gerrit usage to show agreement, following Shawn's comments. * Added a cleaned up version of Phil's $submodule_* export patch, with docs and tests. * Added a caveat to the -r/--record documentation to make it explicit that submodule..branch is not used internally by Git. Give an example of how the user may use it explicitly for Ævar-style updates. W. Trevor King (3): git-submodule add: Add -r/--record option git-submodule foreach: export .gitmodules settings as variables git-submodule: Motivate --record with an example use case Documentation/git-submodule.txt | 22 +++++++++++++++++++++- git-sh-setup.sh | 20 ++++++++++++++++++++ git-submodule.sh | 35 ++++++++++++++++++++++++++++++++++- t/t7400-submodule-basic.sh | 25 +++++++++++++++++++++++++ t/t7407-submodule-foreach.sh | 29 +++++++++++++++++++++++++++++ 5 files changed, 129 insertions(+), 2 deletions(-) mode change 100644 => 100755 git-sh-setup.sh -- 1.8.0.3.gc2eb43a ``` ## W. Trevor King, 2012-11-09 03:35 Subject: [PATCH v3 1/3] git-submodule add: Add -r/--record option Message-ID: URL: https://gitlist.dev/e/fb2d915cf60160c200b84df88c6112c1c2d4eefd.1352431674.git.wking%40tremily.us In-Reply-To: ``` From: "W. Trevor King" This option allows you to record a submodule..branch option in .gitmodules. Git does not currently use this configuration option for anything, but users have used it for several things, so it makes sense to add some syntactic sugar for initializing the value. Current consumers: Ævar uses this setting to designate the upstream branch for pulling submodule updates: $ git submodule foreach 'git checkout $(git config --file $toplevel/.gitmodules submodule.$name.branch) && git pull' as he describes in commit f030c96d8643fa0a1a9b2bd9c2f36a77721fb61f Author: Ævar Arnfjörð Bjarmason Date: Fri May 21 16:10:10 2010 +0000 git-submodule foreach: Add $toplevel variable Gerrit uses the same interpretation for the setting, but because Gerrit has direct access to the subproject repositories, it updates the superproject repositories automatically when a subproject changes. Gerrit also accepts the special value '.', which it expands into the superproject's branch name. By remaining agnostic on the variable usage, this patch makes submodule setup more convenient for all parties. [1] https://gerrit.googlesource.com/gerrit/+/master/Documentation/user-submodules.txt Signed-off-by: W. Trevor King --- Documentation/git-submodule.txt | 11 ++++++++++- git-submodule.sh | 19 ++++++++++++++++++- t/t7400-submodule-basic.sh | 25 +++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 2 deletions(-) diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt index b4683bb..cbec363 100644 --- a/Documentation/git-submodule.txt +++ b/Documentation/git-submodule.txt @@ -9,7 +9,7 @@ git-submodule - Initialize, update or inspect submodules SYNOPSIS -------- [verse] -'git submodule' [--quiet] add [-b branch] [-f|--force] +'git submodule' [--quiet] add [-b branch] [--record[=]] [-f|--force] [--reference ] [--] [] 'git submodule' [--quiet] status [--cached] [--recursive] [--] [...] 'git submodule' [--quiet] init [--] [...] @@ -209,6 +209,15 @@ OPTIONS --branch:: Branch of repository to add as submodule. +-r:: +--record:: + Record a branch name used as `submodule..branch` in + `.gitmodules` for future reference. If you do not list an explicit + name here, the name given with `--branch` will be recorded. If that + is not set either, `HEAD` will be recorded. Because the branch name + is optional, you must use the equal-sign form (`-r=`), not + `-r `. + -f:: --force:: This option is only valid for add and update commands. diff --git a/git-submodule.sh b/git-submodule.sh index ab6b110..bc33112 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -5,7 +5,7 @@ # Copyright (c) 2007 Lars Hjemli dashless=$(basename "$0" | sed -e 's/-/ /') -USAGE="[--quiet] add [-b branch] [-f|--force] [--reference ] [--] [] +USAGE="[--quiet] add [-b branch] [--record[=]] [-f|--force] [--reference ] [--] [] or: $dashless [--quiet] status [--cached] [--recursive] [--] [...] or: $dashless [--quiet] init [--] [...] or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--rebase] [--reference ] [--merge] [--recursive] [--] [...] @@ -20,6 +20,8 @@ require_work_tree command= branch= +record_branch= +record_branch_empty= force= reference= cached= @@ -257,6 +259,12 @@ cmd_add() branch=$2 shift ;; + -r | --record) + record_branch_empty=true + ;; + -r=* | --record=*) + record_branch="${1#*=}" + ;; -f | --force) force=$1 ;; @@ -328,6 +336,11 @@ cmd_add() git ls-files --error-unmatch "$sm_path" > /dev/null 2>&1 && die "$(eval_gettext "'\$sm_path' already exists in the index")" + if test -z "$record_branch" && test "$record_branch_empty" = "true" + then + record_branch="${branch:=HEAD}" + fi + if test -z "$force" && ! git add --dry-run --ignore-missing "$sm_path" > /dev/null 2>&1 then eval_gettextln "The following path is ignored by one of your .gitignore files: @@ -366,6 +379,10 @@ Use -f if you really want to add it." >&2 git config -f .gitmodules submodule."$sm_path".path "$sm_path" && git config -f .gitmodules submodule."$sm_path".url "$repo" && + if test -n "$branch" + then + git config -f .gitmodules submodule."$sm_path".branch "$record_branch" + fi && git add --force .gitmodules || die "$(eval_gettext "Failed to register submodule '\$sm_path'")" } diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh index 5397037..88ae74c 100755 --- a/t/t7400-submodule-basic.sh +++ b/t/t7400-submodule-basic.sh @@ -133,6 +133,7 @@ test_expect_success 'submodule add --branch' ' ( cd addtest && git submodule add -b initial "$submodurl" submod-branch && + test -z "$(git config -f .gitmodules submodule.submod-branch.branch)" && git submodule init ) && @@ -211,6 +212,30 @@ test_expect_success 'submodule add with ./, /.. and // in path' ' test_cmp empty untracked ' +test_expect_success 'submodule add --record' ' + ( + cd addtest && + git submodule add -r "$submodurl" submod-record-head && + test "$(git config -f .gitmodules submodule.submod-record-head.branch)" = "HEAD" + ) +' + +test_expect_success 'submodule add --record --branch' ' + ( + cd addtest && + git submodule add -r -b initial "$submodurl" submod-auto-record && + test "$(git config -f .gitmodules submodule.submod-auto-record.branch)" = "initial" + ) +' + +test_expect_success 'submodule add --record= --branch' ' + ( + cd addtest && + git submodule add -r=final -b initial "$submodurl" submod-record && + test "$(git config -f .gitmodules submodule.submod-record.branch)" = "final" + ) +' + test_expect_success 'setup - add an example entry to .gitmodules' ' GIT_CONFIG=.gitmodules \ git config submodule.example.url git://example.com/init.git -- 1.8.0.3.gc2eb43a ``` ## W. Trevor King, 2012-11-09 03:35 Subject: [PATCH v3 2/3] git-submodule foreach: export .gitmodules settings as variables Message-ID: <2121ce36cf4eb02385255cbd5b0bbd1dcc803113.1352431675.git.wking@tremily.us> URL: https://gitlist.dev/e/2121ce36cf4eb02385255cbd5b0bbd1dcc803113.1352431675.git.wking%40tremily.us In-Reply-To: ``` From: "W. Trevor King" This makes it easy to access per-submodule variables. For example, git submodule foreach 'git checkout $(git config --file $toplevel/.gitmodules submodule.$name.branch) && git pull' can now be reduced to git submodule foreach 'git checkout $submodule_branch && git pull' Every submodule.. setting from .gitmodules is available as a $submodule_ variable. These variables are not propagated recursively into nested submodules. Signed-off-by: W. Trevor King Based-on-patch-by: Phil Hord --- Documentation/git-submodule.txt | 3 +++ git-sh-setup.sh | 20 ++++++++++++++++++++ git-submodule.sh | 16 ++++++++++++++++ t/t7407-submodule-foreach.sh | 29 +++++++++++++++++++++++++++++ 4 files changed, 68 insertions(+) mode change 100644 => 100755 git-sh-setup.sh diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt index cbec363..9a99826 100644 --- a/Documentation/git-submodule.txt +++ b/Documentation/git-submodule.txt @@ -175,6 +175,9 @@ foreach:: $path is the name of the submodule directory relative to the superproject, $sha1 is the commit as recorded in the superproject, and $toplevel is the absolute path to the top-level of the superproject. + In addition, every submodule.. setting from .gitmodules + is available as the variable $submodule_. These + variables are not propagated recursively into nested submodules. Any submodules defined in the superproject but not checked out are ignored by this command. Unless given `--quiet`, foreach prints the name of each submodule before evaluating the command. diff --git a/git-sh-setup.sh b/git-sh-setup.sh old mode 100644 new mode 100755 index ee0e0bc..179a920 --- a/git-sh-setup.sh +++ b/git-sh-setup.sh @@ -222,6 +222,26 @@ clear_local_git_env() { unset $(git rev-parse --local-env-vars) } +# Remove any suspect characters from a user-generated variable name. +sanitize_variable_name() { + VAR_NAME="$1" + printf '%s' "$VAR_NAME" | + sed -e 's/^[^a-zA-Z]/_/' -e 's/[^a-zA-Z0-9]/_/g' +} + +# Return a command for setting a new variable. +# Neither the variable name nor the variable value passed to this +# function need to be sanitized. You need to eval the returned +# string, because new variables set by the function itself don't +# effect the calling process. +set_user_variable() { + VAR_NAME="$1" + VAR_VALUE="$2" + VAR_NAME=$(sanitize_variable_name "$VAR_NAME") + VAR_VALUE=$(printf '%s' "$VAR_VALUE" | + sed -e 's/\\/\\\\/g' -e 's/"/\\"/g') + printf '%s=%s;\n' "$VAR_NAME" "\"$VAR_VALUE\"" +} # Platform specific tweaks to work around some commands case $(uname -s) in diff --git a/git-submodule.sh b/git-submodule.sh index bc33112..e4d26f9 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -434,8 +434,24 @@ cmd_foreach() clear_local_git_env # we make $path available to scripts ... path=$sm_path + + # make all submodule variables available to scripts + eval $( + git config -f .gitmodules --get-regexp "^submodule\.${name}\..*" | + sed -e "s|^submodule\.${name}\.||" | + while read VAR_NAME VAR_VALUE ; do + VAR_NAME=$(printf '%s' "$VAR_NAME" | tr A-Z a-z) + set_user_variable "submodule_${VAR_NAME}" "$VAR_VALUE" + done) + UNSET_CMD=$(set | + sed -n -e 's|^\(submodule_[a-z_]*\)=.*$|\1|p' | + while read VAR_NAME ; do + printf 'unset %s;\n' "$VAR_NAME" + done) + cd "$sm_path" && eval "$@" && + eval "$UNSET_CMD" && if test -n "$recursive" then cmd_foreach "--recursive" "$@" diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh index 9b69fe2..46ac746 100755 --- a/t/t7407-submodule-foreach.sh +++ b/t/t7407-submodule-foreach.sh @@ -313,4 +313,33 @@ test_expect_success 'command passed to foreach --recursive retains notion of std test_cmp expected actual ' +cat > expect < ../actual + ) && + test_i18ncmp expect actual +' +# +#"echo \$toplevel-\$name-\$submodule_path-\$submodule_url" + test_done -- 1.8.0.3.gc2eb43a ``` ## W. Trevor King, 2012-11-09 03:35 Subject: [PATCH v3 3/3] git-submodule: Motivate --record with an example use case Message-ID: URL: https://gitlist.dev/e/ca0fc739741b72b50641e382a6162a829447237f.1352431675.git.wking%40tremily.us In-Reply-To: ``` From: "W. Trevor King" Signed-off-by: W. Trevor King --- Documentation/git-submodule.txt | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt index 9a99826..d4e993f 100644 --- a/Documentation/git-submodule.txt +++ b/Documentation/git-submodule.txt @@ -220,6 +220,14 @@ OPTIONS is not set either, `HEAD` will be recorded. Because the branch name is optional, you must use the equal-sign form (`-r=`), not `-r `. ++ +The recorded setting is not actually used by git; however, some +external tools and workflows may make use of it. For example, if the +upstream branches still exist and you have a recorded branch setting +for each of your submodules, you can update all of the submodules to +the current branch tips with: ++ + git submodule foreach 'git checkout $submodule_branch && git pull' -f:: --force:: -- 1.8.0.3.gc2eb43a ``` ## Junio C Hamano, 2012-11-09 07:34 Subject: Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option Message-ID: <7v390jqlep.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7v390jqlep.fsf%40alter.siamese.dyndns.org In-Reply-To: ``` "W. Trevor King" writes: > By remaining agnostic on the variable usage, this patch makes > submodule setup more convenient for all parties. I personally do not think "remaining agnostic on the usage" is a good thing, at least for any option to commands at the higher level on the stack, such as "git submodule". I am afraid that giving an easier way to set up a variable with undefined semantics may make setup more confusing for all parties. One party gives one specific meaning to the field, while another party uses it for something slightly different. I would not object to "git config submodule.$name.branch $value", on the other hand. "git config" can be used to set a piece of data that has specific meaning, but as a low-level tool, it is not _limited_ to variables that have defined meaning. ``` ## Heiko Voigt, 2012-11-09 16:29 Subject: Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option Message-ID: <20121109162919.GA922@book.hvoigt.net> URL: https://gitlist.dev/e/20121109162919.GA922%40book.hvoigt.net In-Reply-To: <7v390jqlep.fsf@alter.siamese.dyndns.org> ``` Hi, On Thu, Nov 08, 2012 at 11:34:54PM -0800, Junio C Hamano wrote: > "W. Trevor King" writes: > > > By remaining agnostic on the variable usage, this patch makes > > submodule setup more convenient for all parties. > > I personally do not think "remaining agnostic on the usage" is a > good thing, at least for any option to commands at the higher level > on the stack, such as "git submodule". I am afraid that giving an > easier way to set up a variable with undefined semantics may make > setup more confusing for all parties. One party gives one specific > meaning to the field, while another party uses it for something > slightly different. > > I would not object to "git config submodule.$name.branch $value", on > the other hand. "git config" can be used to set a piece of data > that has specific meaning, but as a low-level tool, it is not > _limited_ to variables that have defined meaning. I think we should agree on a behavior for this option and implement it the same time when add learns about it. When we were discussing floating submodules as an important option for the gerrit people I already started to implement a proof of concept. Please have a look here: https://github.com/hvoigt/git/commits/hv/floating_submodules AFAIK this does not yet implement the same behaviour the gerrit tools offer for this option. The main reason behind that was because I do not know the typical workflow behind such an option. So I am open to changes. Maybe you can use or base your work on this implementation for submodule update. Without submodule update using this option I think it would be better to implement this option in the tool you are using instead of submodule add. Everything else feels incomplete to me. Cheers Heiko ``` ## Heiko Voigt, 2012-11-09 16:45 Subject: Re: [PATCH v3 2/3] git-submodule foreach: export .gitmodules settings as variables Message-ID: <20121109164516.GB922@book.hvoigt.net> URL: https://gitlist.dev/e/20121109164516.GB922%40book.hvoigt.net In-Reply-To: <2121ce36cf4eb02385255cbd5b0bbd1dcc803113.1352431675.git.wking@tremily.us> ``` Hi, On Thu, Nov 08, 2012 at 10:35:13PM -0500, W. Trevor King wrote: > From: "W. Trevor King" > > This makes it easy to access per-submodule variables. For example, > > git submodule foreach 'git checkout $(git config --file $toplevel/.gitmodules submodule.$name.branch) && git pull' > > can now be reduced to > > git submodule foreach 'git checkout $submodule_branch && git pull' What other use cases are there? Would the need for this maybe go away once you had floating submodules following branches? The whole thing looks like its adding some complex code which is not so easy to read. I would like to make sure its worth it. > diff --git a/git-submodule.sh b/git-submodule.sh > index bc33112..e4d26f9 100755 > --- a/git-submodule.sh > +++ b/git-submodule.sh > @@ -434,8 +434,24 @@ cmd_foreach() > clear_local_git_env > # we make $path available to scripts ... > path=$sm_path > + > + # make all submodule variables available to scripts > + eval $( > + git config -f .gitmodules --get-regexp "^submodule\.${name}\..*" | For completeness you should make the variables possible to override by repository from the local repository configuration like all other submodule options that are read directly from .gitmodules. Cheers Heiko ``` ## W. Trevor King, 2012-11-10 18:44 Subject: Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option Message-ID: <20121110184437.GC2739@mjolnir> URL: https://gitlist.dev/e/20121110184437.GC2739%40mjolnir In-Reply-To: <7v390jqlep.fsf@alter.siamese.dyndns.org> ``` On Thu, Nov 08, 2012 at 11:34:54PM -0800, Junio C Hamano wrote: > "W. Trevor King" writes: > > > By remaining agnostic on the variable usage, this patch makes > > submodule setup more convenient for all parties. > > I personally do not think "remaining agnostic on the usage" is a > good thing, at least for any option to commands at the higher level > on the stack, such as "git submodule". I am afraid that giving an > easier way to set up a variable with undefined semantics may make > setup more confusing for all parties. One party gives one specific > meaning to the field, while another party uses it for something > slightly different. > > I would not object to "git config submodule.$name.branch $value", on > the other hand. "git config" can be used to set a piece of data > that has specific meaning, but as a low-level tool, it is not > _limited_ to variables that have defined meaning. This is what I'm doing now: $ git submodule add -b $ git config --file .gitmodules submodule..branch $ git submodule foreach 'git checkout $(git config --file $toplevel/.gitmodules submodule.$name.branch) && git pull' With my second patch (Phil's config export), that becomes $ git submodule add -b $ git config --file .gitmodules submodule..branch $ git submodule foreach 'git checkout $submodule_branch && git pull' With my first patch, that becomes $ git submodule add -rb $ git submodule foreach 'git checkout $submodule_branch && git pull' This seems pretty useful to me, but I'm still using submodule..branch explicitly as a user, and Git is not interpreting the option directly. Users are free to store whatever they like in that option, and use it however they wish: $ git submodule foreach 'do-crazy-stuff.sh $submodule_branch' If we need a semantic interpretation to justify -r/--record, everyone that's chimed in so far has agreed on the same interpretation. I wouldn't be averse to $ git submodule add -rb $ git submodule pull-branch which makes the foreach pull logic internal. However, there has been a reasonable amount of resistance to this workflow in the past, so I thought that a patch series that avoided a semantic interpretation would be more acceptable. If neither an agnostic -r/--record or a semantic pull-branch command are acceptable, I suppose we'll have to drop my first and third patches and only keep the second. 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 ``` ## W. Trevor King, 2012-11-10 19:02 Subject: Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option Message-ID: <20121110190232.GD2739@mjolnir> URL: https://gitlist.dev/e/20121110190232.GD2739%40mjolnir In-Reply-To: <20121110184437.GC2739@mjolnir> ``` On Fri, Nov 09, 2012 at 05:29:27PM +0100, Heiko Voigt wrote: > I think we should agree on a behavior for this option and implement it > the same time when add learns about it. When we were discussing floating > submodules as an important option for the gerrit people I already started > to implement a proof of concept. Please have a look here: > > https://github.com/hvoigt/git/commits/hv/floating_submodules After skimming through this, something like $ git submodule update --pull would probably be better than introducing a new command: On Sat, Nov 10, 2012 at 01:44:37PM -0500, W. Trevor King wrote: > $ git submodule pull-branch I think "floating submodules" is a misleading name for this feature though, since the checkout SHA is explicitly specified. We're just making it more convenient to explicitly update the SHA. How about "tracking submodules"? -- 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 ``` ## W. Trevor King, 2012-11-10 19:11 Subject: Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option Message-ID: <20121110191111.GE2739@mjolnir> URL: https://gitlist.dev/e/20121110191111.GE2739%40mjolnir In-Reply-To: <20121109104607.GC4406@ftbfs.org> ``` On Fri, Nov 09, 2012 at 02:46:07AM -0800, Matt Kraai wrote: > On Thu, Nov 08, 2012 at 10:35:12PM -0500, W. Trevor King wrote: > > @@ -366,6 +379,10 @@ Use -f if you really want to add it." >&2 > > > > git config -f .gitmodules submodule."$sm_path".path "$sm_path" && > > git config -f .gitmodules submodule."$sm_path".url "$repo" && > > + if test -n "$branch" > > + then > > + git config -f .gitmodules submodule."$sm_path".branch "$record_branch" > > + fi && > > git add --force .gitmodules || > > die "$(eval_gettext "Failed to register submodule '\$sm_path'")" > > } > > Should the if condition test that $record_branch is not the empty > string instead of testing that $branch is not the empty string? It > seems like this will set submodule."$sm_path".branch to the empty > string if -b is specified and no -r option is specified. Oops, thanks for catching that. Will fix with v4, once we figure out what to do about the semantic-pull situation. -- 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 ``` ## W. Trevor King, 2012-11-10 19:21 Subject: Re: [PATCH v3 2/3] git-submodule foreach: export .gitmodules settings as variables Message-ID: <20121110192135.GF2739@mjolnir> URL: https://gitlist.dev/e/20121110192135.GF2739%40mjolnir In-Reply-To: <20121109164516.GB922@book.hvoigt.net> ``` On Fri, Nov 09, 2012 at 05:45:22PM +0100, Heiko Voigt wrote: > > can now be reduced to > > > > git submodule foreach 'git checkout $submodule_branch && git pull' > > What other use cases are there? Would the need for this maybe go away > once you had floating submodules following branches? None that I can think of, but I don't use submodules very much. The idea of easily-accessible per-submodule configuration variables strikes me as pretty useful, but I agree the code is a bit ugly. Actually, I think exporting environment variables and calling the foreach command in a subshell would be better than the current local variables and eval. The subshell would also make variable cleanup irrelevant, which would make for a cleaner patch. > For completeness you should make the variables possible to override by > repository from the local repository configuration like all other > submodule options that are read directly from .gitmodules. Good idea (I wasn't aware of the override before). Will do in v4. -- 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 ``` ## Junio C Hamano, 2012-11-11 10:33 Subject: Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option Message-ID: <7vzk2oo2d2.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vzk2oo2d2.fsf%40alter.siamese.dyndns.org In-Reply-To: <20121110184437.GC2739@mjolnir> ``` "W. Trevor King" writes: > On Thu, Nov 08, 2012 at 11:34:54PM -0800, Junio C Hamano wrote: > >> I would not object to "git config submodule.$name.branch $value", on >> the other hand. "git config" can be used to set a piece of data >> that has specific meaning, but as a low-level tool, it is not >> _limited_ to variables that have defined meaning. > > This is what I'm doing now: > > $ git submodule add -b > $ git config --file .gitmodules submodule..branch > $ git submodule foreach 'git checkout $(git config --file $toplevel/.gitmodules submodule.$name.branch) && git pull' > > With my second patch (Phil's config export), that becomes > > $ git submodule add -b > $ git config --file .gitmodules submodule..branch > $ git submodule foreach 'git checkout $submodule_branch && git pull' > > With my first patch, that becomes > > $ git submodule add -rb > $ git submodule foreach 'git checkout $submodule_branch && git pull' > > This seems pretty useful to me,... Ah, this reminds me of another thing I noticed when I saw that patch. The change seems to think "branch" is the _only_ thing the user might want to record per submodule upon "git submodule add". As an interface to muck with an uninterpreted random configuration, it squats on a good option name for setting one single and arbitrary variable---quite a selfish change that is not acceptable. Calling the option "--record-branch-for-submodule" or something more specific might alleviate the problem, but then it would become even less useful as a short-hand for "config submodule.$name.branch", I would suspect. On the other hand, if this were one small part of a series to define the "tip following mode" where (at least) (1) "git submodule update [$path]" makes sure that the checkout of the submodule at $path matches the commit at the tip of the branch named by submodule.$name.branch in .gitmodules of the superproject, instead of the commit that is recorded in the index of the superproject; and (2) "git diff [$path]" and friends in the superproject compares the HEAD of the checkout of the submodule at $path with the tip of the branch named by submodule.$name.branch in .gitmodules of the superproject, instead of the commit that is recorded in the index of the superproject. and the option were called something like "--follow-branch=$branch", it would make much more sense for its initial implementation to set the name of the branch to submodule.$name.branch variable. Later iterations of such a feature may want to do more than just setting that single variable but that is a part of the implementation detail of the tip following mode the users do not have to know about, just like setting the submodule.$name.branch variable is. So in that sense, too, I would be somewhat unhappy to see this change in the current form to go in. ``` ## W. Trevor King, 2012-11-11 15:00 Subject: Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option Message-ID: <20121111150047.GA22608@odin.tremily.us> URL: https://gitlist.dev/e/20121111150047.GA22608%40odin.tremily.us In-Reply-To: <7vzk2oo2d2.fsf@alter.siamese.dyndns.org> ``` On Sun, Nov 11, 2012 at 02:33:45AM -0800, Junio C Hamano wrote: > The change seems to think "branch" is the _only_ thing the user > might want to record per submodule upon "git submodule add". I felt that earlier floating/tracking submodule patches were biting off more than they could chew, so I was looking for a lightweight fix to make the tracking workflow easier. It seems like I ended up with something that is too lightweight ;). > On the other hand, if this were one small part of a series to define > the "tip following mode" where (at least) > > (1) "git submodule update [$path]" makes sure that the checkout of > the submodule at $path matches the commit at the tip of the > branch named by submodule.$name.branch in .gitmodules of the > superproject, instead of the commit that is recorded in the > index of the superproject; and As I mentioned earlier, I think $ git submodule update [$path] should keep its current “checkout the already-registered SHA” functionality, with $ git submodule update --pull [$path] pulling the tracked branch. I'll add a patch implementing this to v4. In order to avoid losing (or creating) local-only submodule commits, I'll probably bail (with an error) on non-fast-forward pulls. Can anyone else think of other safety concerns? This means that I'll probably drop Phil's $submodule_* export in v4, because the only explicit use we have for it is this branch tracking. I still think it is a useful idea, but it may not be useful enough to be worth the complexity. > (2) "git diff [$path]" and friends in the superproject compares the > HEAD of the checkout of the submodule at $path with the tip of > the branch named by submodule.$name.branch in .gitmodules of > the superproject, instead of the commit that is recorded in the > index of the superproject. > Hmm. “git diff” compares the working tree with the local HEAD (just a SHA for submodules), so I don't think it should care about the status of a remote branch. This sounds like you want something like: $ git submodule foreach 'git diff origin/$submodule_branch' Perhaps this is enough motivation for keeping $submodule_* exports? > and the option were called something like "--follow-branch=$branch", > … I'll replace -r/--record with --follow-branch in v4. -- 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 ``` ## Heiko Voigt, 2012-11-17 15:04 Subject: Re: Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option Message-ID: <20121117150441.GA7695@book.hvoigt.net> URL: https://gitlist.dev/e/20121117150441.GA7695%40book.hvoigt.net In-Reply-To: <20121110190232.GD2739@mjolnir> ``` Hi, sorry for the late reply but my git time is limited. On Sat, Nov 10, 2012 at 02:02:32PM -0500, W. Trevor King wrote: > On Fri, Nov 09, 2012 at 05:29:27PM +0100, Heiko Voigt wrote: > > I think we should agree on a behavior for this option and implement it > > the same time when add learns about it. When we were discussing floating > > submodules as an important option for the gerrit people I already started > > to implement a proof of concept. Please have a look here: > > > > https://github.com/hvoigt/git/commits/hv/floating_submodules > > After skimming through this, something like > > $ git submodule update --pull > > would probably be better than introducing a new command: Yeah along the lines of that, but one thing to keep in mind: We already have --rebase and --merge which do slightly different things (I think). Adding --pull here should behave similar to them. Like fetch and merge is the same to pull without submodules. If I am understanding your goal correctly your --pull would be different. On the other hand: A --pull makes no sense if we apply it to the existing --merge option since it merges the recorded sha1 into the current HEAD. Just a fetch would not really make a difference. Thinking along the existing options I would probably still expect --pull to merge something into the current HEAD. So maybe we have to iron out where this command/option should go. But changing that once we have a patch to discuss should not be that much work. So please proceed with --pull and once we know exactly what it does we can polish that. > On Sat, Nov 10, 2012 at 01:44:37PM -0500, W. Trevor King wrote: > > $ git submodule pull-branch > > I think "floating submodules" is a misleading name for this feature > though, since the checkout SHA is explicitly specified. We're just > making it more convenient to explicitly update the SHA. How about > "tracking submodules"? Until now we have always called this workflow floating submodules. I imaging since the submodule floats to the newest revision (whatever the user chooses that to be) instead of staying at the recorded sha1. "tracking submodules" sounds strange to me since the term tracked in git is mainly used in combination with exact recorded history (e.g. tracking branch). Since it is about *not* checking out the recorded sha1 but something that can change I think that could cause confusion. I think floating is a more unambiguous term and already known on the list. Cheers Heiko ``` ## Heiko Voigt, 2012-11-17 15:30 Subject: Re: Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option Message-ID: <20121117153007.GB7695@book.hvoigt.net> URL: https://gitlist.dev/e/20121117153007.GB7695%40book.hvoigt.net In-Reply-To: <20121111150047.GA22608@odin.tremily.us> ``` Hi, On Sun, Nov 11, 2012 at 10:00:48AM -0500, W. Trevor King wrote: > On Sun, Nov 11, 2012 at 02:33:45AM -0800, Junio C Hamano wrote: > In order to avoid losing (or creating) local-only submodule commits, > I'll probably bail (with an error) on non-fast-forward pulls. Can > anyone else think of other safety concerns? That sounds like a good thing to do. We can allow more flexibility later if people come up with usecases. > This means that I'll probably drop Phil's $submodule_* export in v4, > because the only explicit use we have for it is this branch tracking. > I still think it is a useful idea, but it may not be useful enough to > be worth the complexity. Yes lets concentrate on the branch following first. > > (2) "git diff [$path]" and friends in the superproject compares the > > HEAD of the checkout of the submodule at $path with the tip of > > the branch named by submodule.$name.branch in .gitmodules of > > the superproject, instead of the commit that is recorded in the > > index of the superproject. > > > > Hmm. ???git diff??? compares the working tree with the local HEAD (just a > SHA for submodules), so I don't think it should care about the status > of a remote branch. This sounds like you want something like: > > $ git submodule foreach 'git diff origin/$submodule_branch' > > Perhaps this is enough motivation for keeping $submodule_* exports? > > > and the option were called something like "--follow-branch=$branch", > > ??? I am not sure if hiding changes to the recorded SHA1 from the user is such a useful thing. In the first step I would like it if it was kept simple and only the submodule update machinery learned to follow a branch. If that results in local changes that should be shown. The user is still in charge of recording the updated SHA1 in his commit. >From what I have heard of projects using this: They usually still have something that records the SHA1s on a regular basis. Thinking further, why not record them in git? We could add an option to update which creates such a commit. Since git is all about changes I am hesitant to hide them from the user. > I'll replace -r/--record with --follow-branch in v4. Sounds good. Cheers Heiko ``` ## W. Trevor King, 2012-11-17 19:20 Subject: Re: Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option Message-ID: <20121117192026.GI22234@odin.tremily.us> URL: https://gitlist.dev/e/20121117192026.GI22234%40odin.tremily.us In-Reply-To: <20121117153007.GB7695@book.hvoigt.net> ``` On Sat, Nov 17, 2012 at 04:04:42PM +0100, Heiko Voigt wrote: > > On Sat, Nov 10, 2012 at 01:44:37PM -0500, W. Trevor King wrote: > > > $ git submodule pull-branch > > > > I think "floating submodules" is a misleading name for this feature > > though, since the checkout SHA is explicitly specified. We're just > > making it more convenient to explicitly update the SHA. How about > > "tracking submodules"? > > Until now we have always called this workflow floating submodules. I > imaging since the submodule floats to the newest revision (whatever the > user chooses that to be) instead of staying at the recorded sha1. > > "tracking submodules" sounds strange to me since the term tracked in git > is mainly used in combination with exact recorded history (e.g. tracking > branch). Since it is about *not* checking out the recorded sha1 but > something that can change I think that could cause confusion. > > I think floating is a more unambiguous term and already known on the > list. I had been getting the impression that floating submodules would automatically update without explicit user intervention. After re-reading your initial floating submodules post, it looks like we do match up after the mapping: Git Heiko Trevor --------- ----------------- ------------- update update --checkout update update update --pull So I'll go back to "floating" ;). On Sat, Nov 17, 2012 at 04:30:07PM +0100, Heiko Voigt wrote: > > > (2) "git diff [$path]" and friends in the superproject compares the > > > HEAD of the checkout of the submodule at $path with the tip of > > > the branch named by submodule.$name.branch in .gitmodules of > > > the superproject, instead of the commit that is recorded in the > > > index of the superproject. > > > > > > > Hmm. ???git diff??? compares the working tree with the local HEAD (just a > > SHA for submodules), so I don't think it should care about the status > > of a remote branch. This sounds like you want something like: > > > > $ git submodule foreach 'git diff origin/$submodule_branch' > > > > Perhaps this is enough motivation for keeping $submodule_* exports? > > > > > and the option were called something like "--follow-branch=$branch", > > > ??? > > I am not sure if hiding changes to the recorded SHA1 from the user is > such a useful thing. In the first step I would like it if it was kept > simple and only the submodule update machinery learned to follow a > branch. If that results in local changes that should be shown. The user > is still in charge of recording the updated SHA1 in his commit. I understand what you're warning against here, or what it has to do with "git diff". > From what I have heard of projects using this: They usually still have > something that records the SHA1s on a regular basis. Thinking further, > why not record them in git? We could add an option to update which > creates such a commit. I think it's best to have users craft their own commit messages explaining why the branch was updated. That said, an auto-generated hint (a la "git merge") would probably be a useful extra feature. -- 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 ``` ## Heiko Voigt, 2012-11-17 21:31 Subject: Re: Re: Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option Message-ID: <20121117213130.GC7695@book.hvoigt.net> URL: https://gitlist.dev/e/20121117213130.GC7695%40book.hvoigt.net In-Reply-To: <20121117192026.GI22234@odin.tremily.us> ``` On Sat, Nov 17, 2012 at 02:20:27PM -0500, W. Trevor King wrote: > On Sat, Nov 17, 2012 at 04:30:07PM +0100, Heiko Voigt wrote: > > > > (2) "git diff [$path]" and friends in the superproject compares the > > > > HEAD of thecheckout of the submodule at $path with the tip of > > > > the branch named by submodule.$name.branch in .gitmodules of > > > > the superproject, instead of the commit that is recorded in the > > > > index of the superproject. > > > > > > > > > > Hmm. ???git diff??? compares the working tree with the local HEAD (just a > > > SHA for submodules), so I don't think it should care about the status > > > of a remote branch. This sounds like you want something like: > > > > > > $ git submodule foreach 'git diff origin/$submodule_branch' > > > > > > Perhaps this is enough motivation for keeping $submodule_* exports? > > > > > > > and the option were called something like "--follow-branch=$branch", > > > > ??? > > > > I am not sure if hiding changes to the recorded SHA1 from the user is > > such a useful thing. In the first step I would like it if it was kept > > simple and only the submodule update machinery learned to follow a > > branch. If that results in local changes that should be shown. The user > > is still in charge of recording the updated SHA1 in his commit. > > I understand what you're warning against here, or what it has to do > with "git diff". Is there a not missing here? Reads somehow like that. What I am talking about is the suggestion of Junio. Instead of showing a diff if the SHA1 is different we show a diff if the checkout in the worktree is different from the tip of the configured branch. That would hide the fact that a submodule has changed during a submodule update operation. > > From what I have heard of projects using this: They usually still have > > something that records the SHA1s on a regular basis. Thinking further, > > why not record them in git? We could add an option to update which > > creates such a commit. > > I think it's best to have users craft their own commit messages > explaining why the branch was updated. That said, an auto-generated > hint (a la "git merge") would probably be a useful extra feature. I have the same opinion. Commits should always be created by humans so you have someone to blame/ask why. But I guess there are people that expect this to be automatic. One argument somehow goes along the lines: "I already created a commit in the submodule why do I need to create another one in the superproject? Just follow the HEAD revision!" They think in subversions "submodules" which are merely pointers to other svn repositories without any revision information. I am unsure if its good to support this the same way. Another use case is big projects that have so many submodules that creating superproject commits would create to much maintenance work. They want to have their integration server make those commits. That would already be supported with update checking out the branch tips and the commit is just one extra thing to do by the integration server. So I think it should be fine just to teach update to checkout the configured branch tips (or forward them to their tracking branch tips) and leave the rest to the user. Cheers Heiko ``` ## W. Trevor King, 2012-11-17 22:00 Subject: Re: Re: Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option Message-ID: <20121117220007.GJ22234@odin.tremily.us> URL: https://gitlist.dev/e/20121117220007.GJ22234%40odin.tremily.us In-Reply-To: <20121117213130.GC7695@book.hvoigt.net> ``` On Sat, Nov 17, 2012 at 10:31:30PM +0100, Heiko Voigt wrote: > On Sat, Nov 17, 2012 at 02:20:27PM -0500, W. Trevor King wrote: > > On Sat, Nov 17, 2012 at 04:30:07PM +0100, Heiko Voigt wrote: > > > > > (2) "git diff [$path]" and friends in the superproject compares the > > > > > HEAD of thecheckout of the submodule at $path with the tip of > > > > > the branch named by submodule.$name.branch in .gitmodules of > > > > > the superproject, instead of the commit that is recorded in the > > > > > index of the superproject. > > > > > > > > > > > > > Hmm. ???git diff??? compares the working tree with the local HEAD (just a > > > > SHA for submodules), so I don't think it should care about the status > > > > of a remote branch. This sounds like you want something like: > > > > > > > > $ git submodule foreach 'git diff origin/$submodule_branch' > > > > > > > > Perhaps this is enough motivation for keeping $submodule_* exports? > > > > > > > > > and the option were called something like "--follow-branch=$branch", > > > > > ??? > > > > > > I am not sure if hiding changes to the recorded SHA1 from the user is > > > such a useful thing. In the first step I would like it if it was kept > > > simple and only the submodule update machinery learned to follow a > > > branch. If that results in local changes that should be shown. The user > > > is still in charge of recording the updated SHA1 in his commit. > > > > I understand what you're warning against here, or what it has to do > > with "git diff". > > Is there a not missing here? Thanks. I'd meant to say "I don't understand…". > What I am talking about is the suggestion of Junio. Instead of > showing a diff if the SHA1 is different we show a diff if the > checkout in the worktree is different from the tip of the configured > branch. That would hide the fact that a submodule has changed during > a submodule update operation. Ahh, now I understand. I agree that comparing to the remote tip is a bad idea. -- 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 ``` ## Junio C Hamano, 2012-11-20 00:49 Subject: Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option Message-ID: <7vd2z9t7y2.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vd2z9t7y2.fsf%40alter.siamese.dyndns.org In-Reply-To: <20121117192026.GI22234@odin.tremily.us> ``` "W. Trevor King" writes: >> From what I have heard of projects using this: They usually still have >> something that records the SHA1s on a regular basis. Thinking further, >> why not record them in git? We could add an option to update which >> creates such a commit. > > I think it's best to have users craft their own commit messages > explaining why the branch was updated. That said, an auto-generated > hint (a la "git merge") would probably be a useful extra feature. I am not quite sure I agree. When the project says "Use the tip of 'bar' branch for the submodule 'foo'" at the top-level, does an individual user who is not working on the submodule 'foo' but merely is using it have any clue as to why the submodule's 'foo' branch 'foo' moved, or does he necessarily even care? For such a user working at the top-level superproject, or working on one part of the project, possibly on a submodule other than 'foo', wouldn't the natural thing to do would be to run "git pull" at the top-level, maybe with "--recursive" to update the top-level and all the submodules to start the day. Now, since somebody created the top-level commit you have just pulled and checked out, other people may have worked on submodule 'foo' [*1*]. What should happen on "git submodule update foo"? It would notice that the submodule 'foo' is set to float, and would check out the tip of the branch 'bar', not the commit recorded in the top-level superproject, in the working tree for 'foo', no? What should appear in "git diff"? The working tree taken as a whole is different from what the superproject's commit describes (which is the state the person who created the superproject wanted to record) even though this user does not have anything to do with the change at 'foo' from the recorded commit to the current tip of 'bar'. What would his description for the reason why the branch was updated? I think I would agree that "git diff" should not hide such changes (after all, when this user records his change to the overall project in the top-level supermodule, he will be recording the state with the commit at the tip of 'bar' checked out in the working tree of the submodule 'foo'), but I am not sure if the user can say anything sensible, other than "tip of 'bar' branch in submodule 'foo' was changed by others", in the resulting commit. [Footnote] *1* This may look like a non-issue if you assume that the person who updates the 'bar' branch of submodule 'foo' always updates the gitlink in the superproject's commit to point at that updated commit, but that assumption is flawed; the submodule project is a project on its own and can be worked on without what other projects bind it as their submodules. ``` ## W. Trevor King, 2012-11-20 01:16 Subject: Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option Message-ID: <20121120011628.GD321@odin.tremily.us> URL: https://gitlist.dev/e/20121120011628.GD321%40odin.tremily.us In-Reply-To: <7vd2z9t7y2.fsf@alter.siamese.dyndns.org> ``` On Mon, Nov 19, 2012 at 04:49:09PM -0800, Junio C Hamano wrote: > "W. Trevor King" writes: > > >> From what I have heard of projects using this: They usually still have > >> something that records the SHA1s on a regular basis. Thinking further, > >> why not record them in git? We could add an option to update which > >> creates such a commit. > > > > I think it's best to have users craft their own commit messages > > explaining why the branch was updated. That said, an auto-generated > > hint (a la "git merge") would probably be a useful extra feature. > > I am not quite sure I agree. When the project says "Use the tip of > 'bar' branch for the submodule 'foo'" at the top-level, does an > individual user who is not working on the submodule 'foo' but merely > is using it have any clue as to why the submodule's 'foo' branch > 'foo' moved, or does he necessarily even care? If he doesn't care, why is he updating the submodule gitlink? > Now, since somebody created the top-level commit you have just > pulled and checked out, other people may have worked on submodule > 'foo' [*1*]. What should happen on "git submodule update foo"? If the 'foo' checkout is not the one listed in the superproject's .gitmodules, the update should bail with an appropriate error message, and let the user sort things out. $ git submodule update --pull foo error: Your local changes to the following submodule would be overwritten by update:… This is similar to how Git currently bails on dirty-tree branch switches: $ git checkout my-branch error: Your local changes to the following files would be overwritten by checkout:… Without "--pull", the update command is intended to checkout the hash specified in .gitmodules. If you've committed some local work in foo and then explicitly ask for an update, I suppose you get clobbered. > What should appear in "git diff"? The working tree taken as a whole > is different from what the superproject's commit describes (which is > the state the person who created the superproject wanted to record) > even though this user does not have anything to do with the change > at 'foo' from the recorded commit to the current tip of 'bar'. What > would his description for the reason why the branch was updated? The submodule content is not part of the superproject. All the superproject has is a gitlink. If the gitlink hasn't changed, "git diff" in the superproject shouldn't say anything. I'll probably have time to write up v4 over the weekend. Maybe having a more explicit example will clear things up. -- 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 ``` ## Junio C Hamano, 2012-11-20 05:39 Subject: Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option Message-ID: <7v1ufou92h.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7v1ufou92h.fsf%40alter.siamese.dyndns.org In-Reply-To: <20121120011628.GD321@odin.tremily.us> ``` "W. Trevor King" writes: > On Mon, Nov 19, 2012 at 04:49:09PM -0800, Junio C Hamano wrote: >> "W. Trevor King" writes: >> ... >> > I think it's best to have users craft their own commit messages >> > explaining why the branch was updated. That said, an auto-generated >> > hint (a la "git merge") would probably be a useful extra feature. >> >> I am not quite sure I agree. When the project says "Use the tip of >> 'bar' branch for the submodule 'foo'" at the top-level, does an >> individual user who is not working on the submodule 'foo' but merely >> is using it have any clue as to why the submodule's 'foo' branch >> 'foo' moved, or does he necessarily even care? > > If he doesn't care, why is he updating the submodule gitlink? He may not be updating the gitlink with "git add foo" at the top-level superproject level. He is just using that submodule as part of the larger whole as he is working on either the top-level or some other submodule. And checkout of 'foo' is necessary in the working tree for him to work in the larger context of the project, and 'foo' is set to float at the tip of its 'bar' branch. And that checkout results in a commit that is different from the commit the gitlink suggests, perhaps because somebody worked in 'foo' submodule and advanced the tip of branch 'bar'. So: - at the top-level superproject level, entry 'foo' in the HEAD tree points at an older commit; - 'foo/.git/HEAD' points at refs/heads/bar, which matches the working tree of 'foo' and the index foo/.git/index.. I am not sure what should happen to the entry 'foo' in the index of the top-level superproject after such a 'submodule floats at the tip' checkout, but I imagine that it must match the contents of foo/.git/HEAD's tree. Otherwise, "git diff" at the top-level would report local changes. When committing his work at the top-level, he will see that 'foo' gitlink is updated in that commit; after all that combination is the context in which his work was done. Or are you envisioning that such a check-out will and should show a local difference at the submodule 'foo' by leaving the index of the top-level superproject unchanged, and the user should refrain from using "git commit -a" to avoid having to describe the changes made on the 'bar' branch in the meantime in his top-level commit? That is certainly fine by me (I am no a heavy submodule user to begin with), but I am not sure if that is useful and helpful to the submodule users. ``` ## W. Trevor King, 2012-11-20 12:19 Subject: Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option Message-ID: <20121120121912.GC7096@odin.tremily.us> URL: https://gitlist.dev/e/20121120121912.GC7096%40odin.tremily.us In-Reply-To: <7v1ufou92h.fsf@alter.siamese.dyndns.org> ``` On Mon, Nov 19, 2012 at 09:39:34PM -0800, Junio C Hamano wrote: > "W. Trevor King" writes: > > > On Mon, Nov 19, 2012 at 04:49:09PM -0800, Junio C Hamano wrote: > >> "W. Trevor King" writes: > >> ... > >> > I think it's best to have users craft their own commit messages > >> > explaining why the branch was updated. That said, an auto-generated > >> > hint (a la "git merge") would probably be a useful extra feature. > >> > >> I am not quite sure I agree. When the project says "Use the tip of > >> 'bar' branch for the submodule 'foo'" at the top-level, does an > >> individual user who is not working on the submodule 'foo' but merely > >> is using it have any clue as to why the submodule's 'foo' branch > >> 'foo' moved, or does he necessarily even care? > > > > If he doesn't care, why is he updating the submodule gitlink? > > He may not be updating the gitlink with "git add foo" at the > top-level superproject level. He is just using that submodule as > part of the larger whole as he is working on either the top-level or > some other submodule. And checkout of 'foo' is necessary in the > working tree for him to work in the larger context of the project, > and 'foo' is set to float at the tip of its 'bar' branch. And that > checkout results in a commit that is different from the commit the > gitlink suggests, perhaps because somebody worked in 'foo' submodule > and advanced the tip of branch 'bar'. The superproject gitlink should only be updated after $ git submodule update --pull A plain $ git submodule update would still checkout the previously-recorded SHA, not the new upstream tip. The uncaring user should skip the "--pull", and there will be no superproject changes to worry about. > Or are you envisioning that such a check-out will and should show a > local difference at the submodule 'foo' by leaving the index of the > top-level superproject unchanged, A plain "git submodule update" will, yes. And this will clobber any changes that have happened in the submodule directory and its index (because the user explicitly asked to checkout the superproject-recorded SHA) > and the user should refrain from using "git commit -a" to avoid > having to describe the changes made on the 'bar' branch in the > meantime in his top-level commit? What would "git commit -a" be picking up? Nothing in the superproject has changed? > That is certainly fine by me (I am no a heavy submodule user to > begin with), but I am not sure if that is useful and helpful to the > submodule users. The benefit is that Ævar's $ git submodule foreach 'git checkout $(git config --file $toplevel/.gitmodules submodule.$name.branch) && git pull' becomes $ git submodule update --pull Still an explicit pull, but much easier to remember. -- 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 ``` ## Junio C Hamano, 2012-11-20 19:52 Subject: Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option Message-ID: <7vhaokrr01.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vhaokrr01.fsf%40alter.siamese.dyndns.org In-Reply-To: <20121120121912.GC7096@odin.tremily.us> ``` "W. Trevor King" writes: > The superproject gitlink should only be updated after > > $ git submodule update --pull > > A plain > > $ git submodule update > > would still checkout the previously-recorded SHA, not the new upstream > tip. Hrm, doesn't it make the "float at the tip of a branch" mode useless, though? ``` ## Heiko Voigt, 2012-11-23 15:55 Subject: Re: Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option Message-ID: <20121123155521.GB14509@book.hvoigt.net> URL: https://gitlist.dev/e/20121123155521.GB14509%40book.hvoigt.net In-Reply-To: <7vhaokrr01.fsf@alter.siamese.dyndns.org> ``` On Tue, Nov 20, 2012 at 11:52:46AM -0800, Junio C Hamano wrote: > "W. Trevor King" writes: > > > The superproject gitlink should only be updated after > > > > $ git submodule update --pull > > > > A plain > > > > $ git submodule update > > > > would still checkout the previously-recorded SHA, not the new upstream > > tip. > > Hrm, doesn't it make the "float at the tip of a branch" mode > useless, though? How about having a branch config option and reusing our submodule.$name.update option for specifying whether the user wants to always float to the tip of the branch? 1. If submodule.$name.update is pull it would checkout the specified tip. 2. If submodule.$name.update is checkout or none it would do the usual thing and you need to specify --pull to get the tip. I am still a little bit undecided about an automatically crafted commit. At $dayjob we sometimes update submodules to their tip without any superproject changes just to make sure we use the newest version. Most of the time the commit messages are along the lines of "updated submodule x to master". On one hand Junio is right that the person updating to the newest submodule stuff has no clue what to write in this message. On the other hand someone might as well just use this functionality to get all the tips of all the submodules checked out. He then individually decides which changes to take by using add but will then still use a commit message like the one above. So currently I am more on the "have an automatically generated commit message" side. Its in a similar corner like merge commits, that are also generated, for me. We could have it as the default and a --no-commit option (like merge) for people that want to stage submodules individually. Cheers Heiko ``` ## Heiko Voigt, 2012-11-23 16:03 Subject: Re: Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option Message-ID: <20121123160301.GC14509@book.hvoigt.net> URL: https://gitlist.dev/e/20121123160301.GC14509%40book.hvoigt.net In-Reply-To: <20121120121912.GC7096@odin.tremily.us> ``` On Tue, Nov 20, 2012 at 07:19:12AM -0500, W. Trevor King wrote: > The benefit is that Ævar's > > $ git submodule foreach 'git checkout $(git config --file $toplevel/.gitmodules submodule.$name.branch) && git pull' > > becomes > > $ git submodule update --pull There is an important question still unanswered here for me: How does the submodule get the configuration what the local branch tracks on the remote side? Cheers Heiko ``` ## W. Trevor King, 2012-11-23 16:23 Subject: Re: Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option Message-ID: <20121123162329.GF2806@odin.tremily.us> URL: https://gitlist.dev/e/20121123162329.GF2806%40odin.tremily.us In-Reply-To: <20121123160301.GC14509@book.hvoigt.net> ``` On Fri, Nov 23, 2012 at 04:55:21PM +0100, Heiko Voigt wrote: > On Tue, Nov 20, 2012 at 11:52:46AM -0800, Junio C Hamano wrote: > > "W. Trevor King" writes: > > > > > The superproject gitlink should only be updated after > > > > > > $ git submodule update --pull > > > > > > A plain > > > > > > $ git submodule update > > > > > > would still checkout the previously-recorded SHA, not the new upstream > > > tip. > > > > Hrm, doesn't it make the "float at the tip of a branch" mode > > useless, though? > > How about having a branch config option and reusing our > submodule.$name.update option for specifying whether the user wants to > always float to the tip of the branch? I'm adding "update --pull" as one of the update options in v4, which I am writing up as we speak ;). > 1. If submodule.$name.update is pull it would checkout the specified tip. and pull from the submodule's upstream. This doesn't need the recorded $sha1, so I may have to rework the current if (clear_local_git_env; cd "$sm_path" && $command "$sha1") > 2. If submodule.$name.update is checkout or none it would do the usual > thing and you need to specify --pull to get the tip. Exactly. > So currently I am more on the "have an automatically generated > commit message" side. Its in a similar corner like merge commits, that > are also generated, for me. We could have it as the default and a > --no-commit option (like merge) for people that want to stage submodules > individually. This sounds reasonable, but I'd like to postpone message-generation sugar until we get the basic functionality ironed out. On Fri, Nov 23, 2012 at 05:03:01PM +0100, Heiko Voigt wrote: > On Tue, Nov 20, 2012 at 07:19:12AM -0500, W. Trevor King wrote: > > The benefit is that Ævar's > > > > $ git submodule foreach 'git checkout $(git config --file $toplevel/.gitmodules submodule.$name.branch) && git pull' > > > > becomes > > > > $ git submodule update --pull > > There is an important question still unanswered here for me: How does > the submodule get the configuration what the local branch tracks on the > remote side? A good point ;). I'm actaully using submodule..branch to store the submodule's local branch name. The remote branch name for the pull is implicit, and defaults to something setup according to branch.autosetupmerge (I think). If you want to get more complicated than this, we'll probably have to add submodule..branch and submodule..remote sections to augment the submodule..branch setting. I'm not sure this is worth it. -- 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 ``` ## W. Trevor King, 2012-11-23 16:30 Subject: Re: Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option Message-ID: <20121123163024.GG2806@odin.tremily.us> URL: https://gitlist.dev/e/20121123163024.GG2806%40odin.tremily.us In-Reply-To: <20121123162329.GF2806@odin.tremily.us> ``` On Fri, Nov 23, 2012 at 11:23:29AM -0500, W. Trevor King wrote: > On Fri, Nov 23, 2012 at 05:03:01PM +0100, Heiko Voigt wrote: > > There is an important question still unanswered here for me: How does > > the submodule get the configuration what the local branch tracks on the > > remote side? > > A good point ;). I'm actaully using submodule..branch to store > the submodule's local branch name. The remote branch name for the > pull is implicit, and defaults to something setup according to > branch.autosetupmerge (I think). If you want to get more complicated > than this, we'll probably have to add submodule..branch and > submodule..remote sections to augment the > submodule..branch setting. I'm not sure this is worth it. These settings are currently stored in .git/modules//config What we're missing is a place to store them in the .gitmodules file. I'll poke around in the module-config initialization and wait for inspiration ;). -- 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 ``` ## Sascha Cunz, 2012-11-23 17:24 Subject: Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option Message-ID: <10919221.ErD9qLRjsD@blacky> URL: https://gitlist.dev/e/10919221.ErD9qLRjsD%40blacky In-Reply-To: <20121123155521.GB14509@book.hvoigt.net> ``` Am Freitag, 23. November 2012, 16:55:21 schrieb Heiko Voigt: > I am still a little bit undecided about an automatically crafted commit. > > At $dayjob we sometimes update submodules to their tip without any > superproject changes just to make sure we use the newest version. Most > of the time the commit messages are along the lines of "updated > submodule x to master". > > On one hand Junio is right that the person updating to the newest > submodule stuff has no clue what to write in this message. I've been thinking about that for a while, when I started using submodules. In the end, I concluded, that what I really want to see in the commit message, is something similar to $(git shortlog $OLD_SHA1..$NEW_SHA1). I've scripted that and taught my CI-Server to do it automatically, if possible. So most of the time, I really don't want an "automatically crafted commit" whenever something causes the tip of a submodule to be at a new SHA1. Just my $.02, though. Sascha ``` ## W. Trevor King, 2012-11-23 17:54 Subject: Re: Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option Message-ID: <20121123175402.GH2806@odin.tremily.us> URL: https://gitlist.dev/e/20121123175402.GH2806%40odin.tremily.us In-Reply-To: <20121123162329.GF2806@odin.tremily.us> ``` On Fri, Nov 23, 2012 at 11:23:29AM -0500, W. Trevor King wrote: > On Fri, Nov 23, 2012 at 04:55:21PM +0100, Heiko Voigt wrote: > > On Tue, Nov 20, 2012 at 11:52:46AM -0800, Junio C Hamano wrote: > > > "W. Trevor King" writes: > > > > > > > The superproject gitlink should only be updated after > > > > > > > > $ git submodule update --pull > > > > > > > > A plain > > > > > > > > $ git submodule update > > > > > > > > would still checkout the previously-recorded SHA, not the new upstream > > > > tip. > > > > > > Hrm, doesn't it make the "float at the tip of a branch" mode > > > useless, though? > > > > How about having a branch config option and reusing our > > submodule.$name.update option for specifying whether the user wants to > > always float to the tip of the branch? > > I'm adding "update --pull" as one of the update options in v4, which I > am writing up as we speak ;). On second thought, this does not seem to be a good idea. The current fancy update styles (--rebase, --merge) are both for cases where you have local commits in the submodule and are trying to incorporate new gitlinks from an updated superproject into the submodule's checked out branch: superproject $ cd submod superproject $ git checkout next submod $ …hack hack hack… submod $ git commit … submod $ cd .. …upstream superproject changes… superproject $ git pull …updated SHA1 for submod gitlink… superproject $ git submodule update --merge …merge superproject's gitlink SHA1 into local submod branch… My submodule..branch option gives a local branch to check out: …upstream submod changes… superproject $ git cd ssubmodule update --pull …fetch upstream submod changes and ff-merge into local submodule..branch… This seems suitably distinct that bundling it with the other update options will just add confusion. So, let's rethink this approach. I'm trying to pull the upstream version of my local submod branch. The difficulties with this are: 1. Checking out a local branch (from the default detached state) to do something on it requires an ungainly: $ git submodule foreach 'git checkout $(git config --file $toplevel/.gitmodules submodule.$name.branch) && …' 2. The remote pulling behavior is configured in .git/modules//config, which is not tracked in the repository itself. I'm ok with forcing local users to handle 2 manually (or implicitly), but 1 is crazy. Addin submodule..branch explicitly to .gitmodules is a step towards fixing 1, but submod pull doesn't match an existing submodules-implemented workflow. Perhaps a better choice would be to borrow the implicit-local-checkout behaviour used by --rebase and --merge. We could add $ git submodule update --branch to checkout the gitlinked SHA1 as submodule..branch in each of the submodules, leaving the submodules on the .gitmodules-configured branch. Effectively (for each submodule): $ git branch -f $branch $sha1 $ git checkout $branch Then I could use $ git submodule foreach 'git pull' to update my submodule tracking branches (without further "git submodule" restructuring). This would help everyone that doesn't like the detached head state (me and --rebase/--merge users). I could avoid implementing "update --pull", and all of the difficulty in configuring upstream merge choices (2) would be punted to the user making local edits in .git/modules//config. 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 ``` ## W. Trevor King, 2012-11-26 21:00 Subject: [PATCH v4 0/4] git-submodule add: Add --local-branch option Message-ID: URL: https://gitlist.dev/e/cover.1353962698.git.wking%40tremily.us In-Reply-To: <20121123175402.GH2806@odin.tremily.us> ``` From: "W. Trevor King" On Fri, Nov 23, 2012 at 12:54:02PM -0500, W. Trevor King wrote: > We could add > > $ git submodule update --branch > > to checkout the gitlinked SHA1 as submodule..branch in each of > the submodules, leaving the submodules on the .gitmodules-configured > branch. Effectively (for each submodule): > > $ git branch -f $branch $sha1 > $ git checkout $branch I haven't gotten any feedback on this as an idea, but perhaps someone will comment on it as a patch series ;). Changes since v3: * --record=… is now --local-branch=… * Dropped patches 2 ($submodule_ export) and 3 (motivating documentation) * Added local git-config overrides of .gitmodules' submodule..branch * Added `submodule update --branch` Because you need to recurse through submodules for `update --branch` even if "$subsha1" == "$sha1", I had to amend the conditional controlling that block. This broke one of the existing tests, which I "fixed" in patch 4. I think a proper fix would involve rewriting (clear_local_git_env; cd "$sm_path" && ( (rev=$(git rev-list -n 1 $sha1 --not --all 2>/dev/null) && test -z "$rev") || git-fetch)) || die "$(eval_gettext "Unable to fetch in submodule path '\$sm_path'")" but I'm not familiar enough with rev-list to want to dig into that yet. If feedback for the earlier three patches is positive, I'll work up a clean fix and resubmit. W. Trevor King (4): git-submodule add: Add --local-branch option git-submodule init: Record submodule..branch in repository config. git-submodule update: Add --branch option Hack fix for 'submodule update does not fetch already present commits' Documentation/config.txt | 9 ++--- Documentation/git-submodule.txt | 32 ++++++++++++----- Documentation/gitmodules.txt | 5 +++ git-submodule.sh | 76 +++++++++++++++++++++++++++++++++-------- t/t7400-submodule-basic.sh | 43 +++++++++++++++++++++++ t/t7406-submodule-update.sh | 50 ++++++++++++++++++++++++++- 6 files changed, 187 insertions(+), 28 deletions(-) -- 1.8.0.3.g95edff1.dirty ``` ## W. Trevor King, 2012-11-26 21:00 Subject: [PATCH v4 1/4] git-submodule add: Add --local-branch option Message-ID: <15e0581e0cb0bb42bf84e8e195597e46d3457a93.1353962698.git.wking@tremily.us> URL: https://gitlist.dev/e/15e0581e0cb0bb42bf84e8e195597e46d3457a93.1353962698.git.wking%40tremily.us In-Reply-To: ``` From: "W. Trevor King" This option allows you to record a submodule..branch option in .gitmodules. Git does not currently use this configuration option for anything, but users have used it for several things, so it makes sense to add some syntactic sugar for initializing the value. Current consumers: Ævar uses this setting to designate the local branch to checkout when pulling submodule updates: $ git submodule foreach 'git checkout $(git config --file $toplevel/.gitmodules submodule.$name.branch) && git pull' as he describes in commit f030c96d8643fa0a1a9b2bd9c2f36a77721fb61f Author: Ævar Arnfjörð Bjarmason Date: Fri May 21 16:10:10 2010 +0000 git-submodule foreach: Add $toplevel variable Gerrit uses the same interpretation for the setting, but because Gerrit has direct access to the subproject repositories, it updates the superproject repositories automatically when a subproject changes. Gerrit also accepts the special value '.', which it expands into the superproject's branch name. Earlier version of this patch remained agnostic on the variable usage, but this was deemed potentially confusing. Future patches in this series will extend the submodule command to use the stored value internally. [1] https://gerrit.googlesource.com/gerrit/+/master/Documentation/user-submodules.txt Signed-off-by: W. Trevor King --- Documentation/git-submodule.txt | 12 ++++++++++-- Documentation/gitmodules.txt | 5 +++++ git-submodule.sh | 19 ++++++++++++++++++- t/t7400-submodule-basic.sh | 25 +++++++++++++++++++++++++ 4 files changed, 58 insertions(+), 3 deletions(-) diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt index b4683bb..d0b4436 100644 --- a/Documentation/git-submodule.txt +++ b/Documentation/git-submodule.txt @@ -9,8 +9,8 @@ git-submodule - Initialize, update or inspect submodules SYNOPSIS -------- [verse] -'git submodule' [--quiet] add [-b branch] [-f|--force] - [--reference ] [--] [] +'git submodule' [--quiet] add [-b branch] [--local-branch[=]] + [-f|--force] [--reference ] [--] [] 'git submodule' [--quiet] status [--cached] [--recursive] [--] [...] 'git submodule' [--quiet] init [--] [...] 'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--rebase] @@ -209,6 +209,14 @@ OPTIONS --branch:: Branch of repository to add as submodule. +--local-branch:: + Record a branch name used as `submodule..branch` in + `.gitmodules` for future reference. If you do not list an explicit + name here, the name given with `--branch` will be recorded. If that + is not set either, `HEAD` will be recorded. Because the branch name + is optional, you must use the equal-sign form + (`--local-branch=`), not `--local-branch `. + -f:: --force:: This option is only valid for add and update commands. diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt index 4effd78..840ccfe 100644 --- a/Documentation/gitmodules.txt +++ b/Documentation/gitmodules.txt @@ -47,6 +47,11 @@ submodule..update:: This config option is overridden if 'git submodule update' is given the '--merge', '--rebase' or '--checkout' options. +submodule..branch:: + A local branch name for the submodule (to avoid headless operation). + Set with the "--local-branch" option to "git submodule add", or + directly using "git config". + submodule..fetchRecurseSubmodules:: This option can be used to control recursive fetching of this submodule. If this option is also present in the submodules entry in diff --git a/git-submodule.sh b/git-submodule.sh index ab6b110..6eed008 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -5,7 +5,7 @@ # Copyright (c) 2007 Lars Hjemli dashless=$(basename "$0" | sed -e 's/-/ /') -USAGE="[--quiet] add [-b branch] [-f|--force] [--reference ] [--] [] +USAGE="[--quiet] add [-b branch] [--local-branch[=]] [-f|--force] [--reference ] [--] [] or: $dashless [--quiet] status [--cached] [--recursive] [--] [...] or: $dashless [--quiet] init [--] [...] or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--rebase] [--reference ] [--merge] [--recursive] [--] [...] @@ -20,6 +20,8 @@ require_work_tree command= branch= +local_branch= +local_branch_empty= force= reference= cached= @@ -257,6 +259,12 @@ cmd_add() branch=$2 shift ;; + --local-branch) + local_branch_empty=true + ;; + --local-branch=*) + local_branch="${1#*=}" + ;; -f | --force) force=$1 ;; @@ -328,6 +336,11 @@ cmd_add() git ls-files --error-unmatch "$sm_path" > /dev/null 2>&1 && die "$(eval_gettext "'\$sm_path' already exists in the index")" + if test -z "$local_branch" && test "$local_branch_empty" = "true" + then + local_branch="${branch:=HEAD}" + fi + if test -z "$force" && ! git add --dry-run --ignore-missing "$sm_path" > /dev/null 2>&1 then eval_gettextln "The following path is ignored by one of your .gitignore files: @@ -366,6 +379,10 @@ Use -f if you really want to add it." >&2 git config -f .gitmodules submodule."$sm_path".path "$sm_path" && git config -f .gitmodules submodule."$sm_path".url "$repo" && + if test -n "$local_branch" + then + git config -f .gitmodules submodule."$sm_path".branch "$local_branch" + fi && git add --force .gitmodules || die "$(eval_gettext "Failed to register submodule '\$sm_path'")" } diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh index 5397037..fc08647 100755 --- a/t/t7400-submodule-basic.sh +++ b/t/t7400-submodule-basic.sh @@ -133,6 +133,7 @@ test_expect_success 'submodule add --branch' ' ( cd addtest && git submodule add -b initial "$submodurl" submod-branch && + test -z "$(git config -f .gitmodules submodule.submod-branch.branch)" && git submodule init ) && @@ -211,6 +212,30 @@ test_expect_success 'submodule add with ./, /.. and // in path' ' test_cmp empty untracked ' +test_expect_success 'submodule add --local-branch' ' + ( + cd addtest && + git submodule add --local-branch "$submodurl" submod-follow-head && + test "$(git config -f .gitmodules submodule.submod-follow-head.branch)" = "HEAD" + ) +' + +test_expect_success 'submodule add --local-branch --branch' ' + ( + cd addtest && + git submodule add --local-branch -b initial "$submodurl" submod-auto-follow && + test "$(git config -f .gitmodules submodule.submod-auto-follow.branch)" = "initial" + ) +' + +test_expect_success 'submodule add --local-branch= --branch' ' + ( + cd addtest && + git submodule add --local-branch=final -b initial "$submodurl" submod-follow && + test "$(git config -f .gitmodules submodule.submod-follow.branch)" = "final" + ) +' + test_expect_success 'setup - add an example entry to .gitmodules' ' GIT_CONFIG=.gitmodules \ git config submodule.example.url git://example.com/init.git -- 1.8.0.3.g95edff1.dirty ``` ## W. Trevor King, 2012-11-26 21:00 Subject: [PATCH v4 2/4] git-submodule init: Record submodule..branch in repository config. Message-ID: <6734714e90064b3932126565e3027d7edcf45d51.1353962698.git.wking@tremily.us> URL: https://gitlist.dev/e/6734714e90064b3932126565e3027d7edcf45d51.1353962698.git.wking%40tremily.us In-Reply-To: ``` From: "W. Trevor King" This allows users to override the .gitmodules value with a per-repository value. Signed-off-by: W. Trevor King --- Documentation/config.txt | 9 +++++---- git-submodule.sh | 7 +++++++ t/t7400-submodule-basic.sh | 18 ++++++++++++++++++ 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 11f320b..1304499 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1994,10 +1994,11 @@ status.submodulesummary:: submodule..path:: submodule..url:: submodule..update:: - The path within this project, URL, and the updating strategy - for a submodule. These variables are initially populated - by 'git submodule init'; edit them to override the - URL and other values found in the `.gitmodules` file. See +submodule..branch:: + The path within this project, URL, the updating strategy, and the + local branch name for a submodule. These variables are initially + populated by 'git submodule init'; edit them to override the URL and + other values found in the `.gitmodules` file. See linkgit:git-submodule[1] and linkgit:gitmodules[5] for details. submodule..fetchRecurseSubmodules:: diff --git a/git-submodule.sh b/git-submodule.sh index 6eed008..c51b6ae 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -505,6 +505,13 @@ cmd_init() test -n "$(git config submodule."$name".update)" || git config submodule."$name".update "$upd" || die "$(eval_gettext "Failed to register update mode for submodule path '\$sm_path'")" + + # Copy "branch" setting when it is not set yet + branch="$(git config -f .gitmodules submodule."$name".branch)" + test -z "$branch" || + test -n "$(git config submodule."$name".branch)" || + git config submodule."$name".branch "$branch" || + die "$(eval_gettext "Failed to register branch for submodule path '\$sm_path'")" done } diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh index fc08647..3dc8237 100755 --- a/t/t7400-submodule-basic.sh +++ b/t/t7400-submodule-basic.sh @@ -236,6 +236,24 @@ test_expect_success 'submodule add --local-branch= --branch' ' ) ' +test_expect_success 'init should register submodule branch in .git/config' ' + ( + cd addtest && + git submodule init && + test "$(git config submodule.submod-follow.branch)" = "final" + ) +' + +test_expect_success 'local config should override .gitmodules branch' ' + ( + cd addtest && + rm -fr submod-follow && + git config submodule.submod-follow.branch initial + git submodule init && + test "$(git config submodule.submod-follow.branch)" = "initial" + ) +' + test_expect_success 'setup - add an example entry to .gitmodules' ' GIT_CONFIG=.gitmodules \ git config submodule.example.url git://example.com/init.git -- 1.8.0.3.g95edff1.dirty ``` ## W. Trevor King, 2012-11-26 21:00 Subject: [PATCH v4 3/4] git-submodule update: Add --branch option Message-ID: <95edff1c97c513c555652014f9c2bbf61c8e7560.1353962698.git.wking@tremily.us> URL: https://gitlist.dev/e/95edff1c97c513c555652014f9c2bbf61c8e7560.1353962698.git.wking%40tremily.us In-Reply-To: ``` From: "W. Trevor King" This allows users to checkout the current superproject-recorded-submodule-sha as a branch, avoiding the detached head state that the standard submodule update creates. This may be useful for the existing --rebase/--merge workflows which already avoid detached heads. It is also useful if you want easy tracking of upstream branches. The particular upstream branch to be tracked is configured locally with .git/modules//config. With the new option Ævar's suggested $ git submodule foreach 'git checkout $(git config --file $toplevel/.gitm odules submodule.$name.branch) && git pull' reduces to a $ git submodule update --branch after each supermodule .gitmodules edit, and a $ git submodule foreach 'git pull' whenever you feel like updating the submodules. Your still on you're own to commit (or not) the updated submodule hashes in the superproject's .gitmodules. Signed-off-by: W. Trevor King --- Documentation/git-submodule.txt | 20 +++++++++++------ git-submodule.sh | 48 +++++++++++++++++++++++++++++---------- t/t7406-submodule-update.sh | 50 ++++++++++++++++++++++++++++++++++++++++- 3 files changed, 98 insertions(+), 20 deletions(-) diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt index d0b4436..34392a1 100644 --- a/Documentation/git-submodule.txt +++ b/Documentation/git-submodule.txt @@ -13,7 +13,7 @@ SYNOPSIS [-f|--force] [--reference ] [--] [] 'git submodule' [--quiet] status [--cached] [--recursive] [--] [...] 'git submodule' [--quiet] init [--] [...] -'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--rebase] +'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--branch] [--rebase] [--reference ] [--merge] [--recursive] [--] [...] 'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) ] [commit] [--] [...] @@ -136,11 +136,11 @@ init:: 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`. + checkout the commit specified in the index of the containing + repository. This will make the submodules HEAD be detached unless + `--branch`, `--rebase`, `--merge` is specified or the key + `submodule.$name.update` is set to `branch`, `rebase`, `merge` or + `none`. `none` can be overridden by specifying `--checkout`. + If the submodule is not yet initialized, and you just want to use the setting as stored in .gitmodules, you can automatically initialize the @@ -207,7 +207,13 @@ OPTIONS -b:: --branch:: - Branch of repository to add as submodule. + When used with the add command, gives the branch of repository to + add as submodule. ++ +When used with the update command, checks out a branch named +`submodule..branch` (as set by `--local-branch`) pointing at the +current HEAD SHA-1. This is useful for commands like `update +--rebase` that do not work on detached heads. --local-branch:: Record a branch name used as `submodule..branch` in diff --git a/git-submodule.sh b/git-submodule.sh index c51b6ae..28eb4b1 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -8,7 +8,7 @@ dashless=$(basename "$0" | sed -e 's/-/ /') USAGE="[--quiet] add [-b branch] [--local-branch[=]] [-f|--force] [--reference ] [--] [] or: $dashless [--quiet] status [--cached] [--recursive] [--] [...] or: $dashless [--quiet] init [--] [...] - or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--rebase] [--reference ] [--merge] [--recursive] [--] [...] + or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--branch] [--rebase] [--reference ] [--merge] [--recursive] [--] [...] or: $dashless [--quiet] summary [--cached|--files] [--summary-limit ] [commit] [--] [...] or: $dashless [--quiet] foreach [--recursive] or: $dashless [--quiet] sync [--] [...]" @@ -539,6 +539,9 @@ cmd_update() -f|--force) force=$1 ;; + -b|--branch) + update="branch" + ;; -r|--rebase) update="rebase" ;; @@ -593,6 +596,7 @@ cmd_update() fi name=$(module_name "$sm_path") || exit url=$(git config submodule."$name".url) + branch=$(git config submodule."$name".branch) if ! test -z "$update" then update_module=$update @@ -627,7 +631,7 @@ Maybe you want to use 'update --init'?")" die "$(eval_gettext "Unable to find current revision in submodule path '\$sm_path'")" fi - if test "$subsha1" != "$sha1" -o -n "$force" + if test "$subsha1" != "$sha1" -o -n "$force" -o "$update_module" = "branch" then subforce=$force # If we don't already have a -f flag and the submodule has never been checked out @@ -650,16 +654,21 @@ Maybe you want to use 'update --init'?")" case ";$cloned_modules;" in *";$name;"*) # then there is no local change to integrate - update_module= ;; + case "$update_module" in + rebase|merge) + update_module= + ;; + esac + ;; esac must_die_on_failure= case "$update_module" in rebase) command="git rebase" - die_msg="$(eval_gettext "Unable to rebase '\$sha1' in submodule path '\$sm_path'")" + die_msg="$(eval_gettext "Unable to rebase '\$sha1' in submodule path '\$sm_path'")" say_msg="$(eval_gettext "Submodule path '\$sm_path': rebased into '\$sha1'")" - must_die_on_failure=yes + must_die_on_failure=yes ;; merge) command="git merge" @@ -674,15 +683,30 @@ Maybe you want to use 'update --init'?")" ;; esac - if (clear_local_git_env; cd "$sm_path" && $command "$sha1") + if test "$subsha1" != "$sha1" -o -n "$force" then - say "$say_msg" - elif test -n "$must_die_on_failure" + if (clear_local_git_env; cd "$sm_path" && $command "$sha1") + then + say "$say_msg" + elif test -n "$must_die_on_failure" + then + die_with_status 2 "$die_msg" + else + err="${err};$die_msg" + continue + fi + fi + + if test "$update_module" = "branch" -a -n "$branch" then - die_with_status 2 "$die_msg" - else - err="${err};$die_msg" - continue + if (clear_local_git_env; cd "$sm_path" && + git branch -f "$branch" "$sha1" && + git checkout "$branch") + then + say "$(eval_gettext "Submodule path '\$sm_path': checked out branch '\$branch'")" + else + err="${err};$(eval_gettext "Unable to checkout branch '\$branch' in submodule path '\$sm_path'")" + fi fi fi diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh index 1542653..c876a8b 100755 --- a/t/t7406-submodule-update.sh +++ b/t/t7406-submodule-update.sh @@ -6,7 +6,8 @@ test_description='Test updating submodules This test verifies that "git submodule update" detaches the HEAD of the -submodule and "git submodule update --rebase/--merge" does not detach the HEAD. +submodule and "git submodule update --branch/--rebase/--merge" does not +detach the HEAD. ' . ./test-lib.sh @@ -135,6 +136,53 @@ test_expect_success 'submodule update --force forcibly checks out submodules' ' ) ' +test_expect_success 'submodule update --branch detaches without submodule..branch' ' + (cd super/submodule && + git checkout master + ) && + (cd super && + (cd submodule && + compare_head + ) && + git submodule update --branch submodule && + (cd submodule && + test "$(git status -s file)" = "" + ) + ) +' + +test_expect_success 'submodule update --branch staying on master' ' + (cd super/submodule && + git checkout master + ) && + (cd super && + (cd submodule && + compare_head + ) && + git config submodule.submodule.branch master + git submodule update --branch submodule && + cd submodule && + test "refs/heads/master" = "$(git symbolic-ref -q HEAD)" && + compare_head + ) +' + +test_expect_success 'submodule update --branch creating a new branch' ' + (cd super/submodule && + git checkout master + ) && + (cd super && + (cd submodule && + compare_head + ) && + git config submodule.submodule.branch new-branch + git submodule update --branch submodule && + cd submodule && + test "refs/heads/new-branch" = "$(git symbolic-ref -q HEAD)" && + compare_head + ) +' + test_expect_success 'submodule update --rebase staying on master' ' (cd super/submodule && git checkout master -- 1.8.0.3.g95edff1.dirty ``` ## W. Trevor King, 2012-11-26 21:00 Subject: [PATCH v4 4/4] Hack fix for 'submodule update does not fetch already present commits' Message-ID: <30459164cc221165a20cd4a54daac76ddb101269.1353962698.git.wking@tremily.us> URL: https://gitlist.dev/e/30459164cc221165a20cd4a54daac76ddb101269.1353962698.git.wking%40tremily.us In-Reply-To: ``` From: "W. Trevor King" --- git-submodule.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-submodule.sh b/git-submodule.sh index 28eb4b1..f4a681c 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -640,7 +640,7 @@ Maybe you want to use 'update --init'?")" subforce="-f" fi - if test -z "$nofetch" + if test -z "$nofetch" -a "$subsha1" != "$sha1" then # Run fetch only if $sha1 isn't present or it # is not reachable from a ref. -- 1.8.0.3.g95edff1.dirty ``` ## Heiko Voigt, 2012-11-27 18:31 Subject: Re: [PATCH v4 0/4] git-submodule add: Add --local-branch option Message-ID: <20121127183125.GA4185@book.hvoigt.net> URL: https://gitlist.dev/e/20121127183125.GA4185%40book.hvoigt.net In-Reply-To: ``` Hi, On Mon, Nov 26, 2012 at 04:00:15PM -0500, W. Trevor King wrote: > From: "W. Trevor King" > > On Fri, Nov 23, 2012 at 12:54:02PM -0500, W. Trevor King wrote: > > We could add > > > > $ git submodule update --branch > > > > to checkout the gitlinked SHA1 as submodule..branch in each of > > the submodules, leaving the submodules on the .gitmodules-configured > > branch. Effectively (for each submodule): > > > > $ git branch -f $branch $sha1 > > $ git checkout $branch > > I haven't gotten any feedback on this as an idea, but perhaps someone > will comment on it as a patch series ;). I am not sure I understand you correctly. You are suggesting that the branch option as an alias for the registered SHA1 in the superproject? I though the goal of your series was that you want to track submodules branch which come from the remote side? Doing the above does not assist you much in that does it? I would think more of some convention like: $ git checkout -t origin/$branch when first initialising the submodule with e.g. $ git submodule update --init --branch Then later calls of $ git submodule update --branch would have a branch configured to pull from. I imagine that results in a similar behavior gerrit is doing on the server side? > Changes since v3: > > * --record=??? is now --local-branch=??? > * Dropped patches 2 ($submodule_ export) and 3 (motivating documentation) > * Added local git-config overrides of .gitmodules' submodule..branch > * Added `submodule update --branch` I would prefer if we could squash all these commits together into one since it seems to me one logical step, using the new variable for update belongs together with its configuration on initialization. How about reusing the -b|--branch option for add? Since we only change the behavior when submodule.$name.update is set to branch it seems reasonable to me. Opinions? > Because you need to recurse through submodules for `update --branch` > even if "$subsha1" == "$sha1", I had to amend the conditional > controlling that block. This broke one of the existing tests, which I > "fixed" in patch 4. I think a proper fix would involve rewriting > > (clear_local_git_env; cd "$sm_path" && > ( (rev=$(git rev-list -n 1 $sha1 --not --all 2>/dev/null) && > test -z "$rev") || git-fetch)) || > die "$(eval_gettext "Unable to fetch in submodule path '\$sm_path'")" > > but I'm not familiar enough with rev-list to want to dig into that > yet. If feedback for the earlier three patches is positive, I'll work > up a clean fix and resubmit. You probably need to separate your handling here. The comparison of the currently checked out sha1 and the recorded sha1 is an optimization which skips unnecessary fetching in case the submodules commits are already correct. This code snippet checks whether the to be checked out sha1 is already local and also skips the fetch if it is. We should not break that. Maybe we need an else block here and possibly extract the current code inside the if statement into a function. E.g. that the final code looks something like this: if test "$subsha1" != "$sha1" then handle_on_demand_fetch_update ... else handle_tracked_branch_update ... fi Not sure about the function names though. If we decide to go that route: The extraction into a function should go in an extra preparation patch which does not change any functionality. I will reply to the patches for further comments. Cheers Heiko ``` ## Heiko Voigt, 2012-11-27 18:51 Subject: Re: [PATCH v4 3/4] git-submodule update: Add --branch option Message-ID: <20121127185142.GB4185@book.hvoigt.net> URL: https://gitlist.dev/e/20121127185142.GB4185%40book.hvoigt.net In-Reply-To: <95edff1c97c513c555652014f9c2bbf61c8e7560.1353962698.git.wking@tremily.us> ``` On Mon, Nov 26, 2012 at 04:00:18PM -0500, W. Trevor King wrote: > From: "W. Trevor King" > > This allows users to checkout the current > superproject-recorded-submodule-sha as a branch, avoiding the detached > head state that the standard submodule update creates. This may be > useful for the existing --rebase/--merge workflows which already avoid > detached heads. > > It is also useful if you want easy tracking of upstream branches. The > particular upstream branch to be tracked is configured locally with > .git/modules//config. With the new option Ævar's suggested > > $ git submodule foreach 'git checkout $(git config --file $toplevel/.gitm > odules submodule.$name.branch) && git pull' > > reduces to a > > $ git submodule update --branch > > after each supermodule .gitmodules edit, and a > > $ git submodule foreach 'git pull' > > whenever you feel like updating the submodules. Your still on you're > own to commit (or not) the updated submodule hashes in the > superproject's .gitmodules. > > Signed-off-by: W. Trevor King > --- > Documentation/git-submodule.txt | 20 +++++++++++------ > git-submodule.sh | 48 +++++++++++++++++++++++++++++---------- > t/t7406-submodule-update.sh | 50 ++++++++++++++++++++++++++++++++++++++++- > 3 files changed, 98 insertions(+), 20 deletions(-) > > diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt > index d0b4436..34392a1 100644 > --- a/Documentation/git-submodule.txt > +++ b/Documentation/git-submodule.txt > @@ -13,7 +13,7 @@ SYNOPSIS > [-f|--force] [--reference ] [--] [] > 'git submodule' [--quiet] status [--cached] [--recursive] [--] [...] > 'git submodule' [--quiet] init [--] [...] > -'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--rebase] > +'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--branch] [--rebase] > [--reference ] [--merge] [--recursive] [--] [...] > 'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) ] > [commit] [--] [...] > @@ -136,11 +136,11 @@ init:: > > 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`. > + checkout the commit specified in the index of the containing > + repository. This will make the submodules HEAD be detached unless > + `--branch`, `--rebase`, `--merge` is specified or the key > + `submodule.$name.update` is set to `branch`, `rebase`, `merge` or > + `none`. `none` can be overridden by specifying `--checkout`. > + > If the submodule is not yet initialized, and you just want to use the > setting as stored in .gitmodules, you can automatically initialize the > @@ -207,7 +207,13 @@ OPTIONS > > -b:: > --branch:: > - Branch of repository to add as submodule. > + When used with the add command, gives the branch of repository to > + add as submodule. > ++ > +When used with the update command, checks out a branch named > +`submodule..branch` (as set by `--local-branch`) pointing at the > +current HEAD SHA-1. This is useful for commands like `update > +--rebase` that do not work on detached heads. Since you are reusing this option for update it further convinces me that reusing it for add makes sense and simplifies the logic for users. I think an optional argument for --branch would be nice in the update case: $ git submodule update --branch=master would then allow a user that has not configured anything (except the branch tracking info in the submodule of course) to pull all submodules master branches. > diff --git a/git-submodule.sh b/git-submodule.sh > index c51b6ae..28eb4b1 100755 > --- a/git-submodule.sh > +++ b/git-submodule.sh > @@ -627,7 +631,7 @@ Maybe you want to use 'update --init'?")" > die "$(eval_gettext "Unable to find current revision in submodule path '\$sm_path'")" > fi > > - if test "$subsha1" != "$sha1" -o -n "$force" > + if test "$subsha1" != "$sha1" -o -n "$force" -o "$update_module" = "branch" As said before I think separating your code from the current update logic will simplify the handling below. > then > subforce=$force > # If we don't already have a -f flag and the submodule has never been checked out > @@ -650,16 +654,21 @@ Maybe you want to use 'update --init'?")" > case ";$cloned_modules;" in > *";$name;"*) > # then there is no local change to integrate > - update_module= ;; > + case "$update_module" in > + rebase|merge) > + update_module= > + ;; > + esac > + ;; > esac > > must_die_on_failure= > case "$update_module" in > rebase) > command="git rebase" > - die_msg="$(eval_gettext "Unable to rebase '\$sha1' in submodule path '\$sm_path'")" > + die_msg="$(eval_gettext "Unable to rebase '\$sha1' in submodule path '\$sm_path'")" > say_msg="$(eval_gettext "Submodule path '\$sm_path': rebased into '\$sha1'")" > - must_die_on_failure=yes > + must_die_on_failure=yes Please always cleanup whitespace changes. > ;; > merge) > command="git merge" > @@ -674,15 +683,30 @@ Maybe you want to use 'update --init'?")" > ;; > esac > > then > - die_with_status 2 "$die_msg" > - else > - err="${err};$die_msg" > - continue > + if (clear_local_git_env; cd "$sm_path" && > + git branch -f "$branch" "$sha1" && > + git checkout "$branch") You wrote in earlier emails that you wanted to protect the user from non-fastforward changes. So I would expect a $ git pull --ff-only here and the setup of that in the initialization of the submodule. BTW, I am more and more convinced that an automatically manufactured commit on update with --branch should be the default. What do other think? Sascha raised a concern that he would not want this, but as far as I understood he let the CI-server do that so I see no downside to natively adding that to git. People who want to manually craft those commits can still amend the generated commit. Since this is all about helping people keeping their submodules updated why not go the full way? Cheers Heiko ``` ## W. Trevor King, 2012-11-27 19:01 Subject: Re: [PATCH v4 0/4] git-submodule add: Add --local-branch option Message-ID: <20121127190105.GQ10656@odin.tremily.us> URL: https://gitlist.dev/e/20121127190105.GQ10656%40odin.tremily.us In-Reply-To: <20121127183125.GA4185@book.hvoigt.net> ``` On Tue, Nov 27, 2012 at 07:31:25PM +0100, Heiko Voigt wrote: > On Mon, Nov 26, 2012 at 04:00:15PM -0500, W. Trevor King wrote: > > From: "W. Trevor King" > > > > On Fri, Nov 23, 2012 at 12:54:02PM -0500, W. Trevor King wrote: > > > We could add > > > > > > $ git submodule update --branch > > > > > > to checkout the gitlinked SHA1 as submodule..branch in each of > > > the submodules, leaving the submodules on the .gitmodules-configured > > > branch. Effectively (for each submodule): > > > > > > $ git branch -f $branch $sha1 > > > $ git checkout $branch > > > > I haven't gotten any feedback on this as an idea, but perhaps someone > > will comment on it as a patch series ;). > > I am not sure I understand you correctly. You are suggesting that the > branch option as an alias for the registered SHA1 in the superproject? > > I though the goal of your series was that you want to track submodules > branch which come from the remote side? That's what I'd initially thought, but when I went to implement `update --pull`, I realized that $ git submodule foreach 'git checkout $(git config --file $toplevel/.gitmodules submodule.$name.branch) && …' is using submodule..branch as the local branch name. The remote branch name was actually setup in .git/modules//config during the initial "clone -b …". The v4 series leaves the remote branch amigious, but it helps you point the local branch at the right hash so that future calls to $ git submodule foreach 'git pull' can use the branch's .git/modules//config settings. > I would think more of some convention like: > > $ git checkout -t origin/$branch > > when first initialising the submodule with e.g. > > $ git submodule update --init --branch > > Then later calls of > > $ git submodule update --branch > > would have a branch configured to pull from. I imagine that results in > a similar behavior gerrit is doing on the server side? That sounds like it's doing pretty much the same thing. Can you think of a test that would distinguish it from my current v4 implementation? > > Changes since v3: > > > > * --record=??? is now --local-branch=??? > > * Dropped patches 2 ($submodule_ export) and 3 (motivating documentation) > > * Added local git-config overrides of .gitmodules' submodule..branch > > * Added `submodule update --branch` > > I would prefer if we could squash all these commits together into one > since it seems to me one logical step, using the new variable for update > belongs together with its configuration on initialization. > > How about reusing the -b|--branch option for add? Since we only change > the behavior when submodule.$name.update is set to branch it seems > reasonable to me. Opinions? That was the approach I used in v1, but people were concerned that we would be stomping on previously unclaimed config space. Since noone has pointed out other uses besides Gerrit's very similar case, I'm not sure if that is still an issue. > > Because you need to recurse through submodules for `update --branch` > > even if "$subsha1" == "$sha1", I had to amend the conditional > > controlling that block. This broke one of the existing tests, which I > > "fixed" in patch 4. I think a proper fix would involve rewriting > > > > (clear_local_git_env; cd "$sm_path" && > > ( (rev=$(git rev-list -n 1 $sha1 --not --all 2>/dev/null) && > > test -z "$rev") || git-fetch)) || > > die "$(eval_gettext "Unable to fetch in submodule path '\$sm_path'")" > > > > but I'm not familiar enough with rev-list to want to dig into that > > yet. If feedback for the earlier three patches is positive, I'll work > > up a clean fix and resubmit. > > You probably need to separate your handling here. The comparison of the > currently checked out sha1 and the recorded sha1 is an optimization > which skips unnecessary fetching in case the submodules commits are > already correct. This code snippet checks whether the to be checked out > sha1 is already local and also skips the fetch if it is. We should not > break that. Agreed. However, determining if the target $sha1 is local should have nothing to do with the current checked out $subsha1. Thanks for the feedback! Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy ``` ## W. Trevor King, 2012-11-27 19:04 Subject: Re: [PATCH v4 0/4] git-submodule add: Add --local-branch option Message-ID: <20121127190400.GA15213@odin.tremily.us> URL: https://gitlist.dev/e/20121127190400.GA15213%40odin.tremily.us In-Reply-To: <20121127183125.GA4185@book.hvoigt.net> ``` On Tue, Nov 27, 2012 at 07:31:25PM +0100, Heiko Voigt wrote: > I would prefer if we could squash all these commits together into > one since it seems to me one logical step, using the new variable > for update belongs together with its configuration on > initialization. Works for me. I could also try to rework the patch boundaries if a monolithic patch is not acceptable. I agree that the current documentation assignments are fairly arbitrary. If I don't hear from anyone in favor of keeping them separate, v5 will be monolithic. -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy ``` ## Heiko Voigt, 2012-11-27 19:16 Subject: Re: Re: [PATCH v4 0/4] git-submodule add: Add --local-branch option Message-ID: <20121127191628.GC4185@book.hvoigt.net> URL: https://gitlist.dev/e/20121127191628.GC4185%40book.hvoigt.net In-Reply-To: <20121127183125.GA4185@book.hvoigt.net> ``` Hi, I just realized that I gave you an confusing suggestion. On Tue, Nov 27, 2012 at 07:31:25PM +0100, Heiko Voigt wrote: > if test "$subsha1" != "$sha1" > then > handle_on_demand_fetch_update ... > else > handle_tracked_branch_update ... > fi That obviously does not work. Here I meant of course something like: if test "$update_module" = "branch" then handle_tracked_branch_update ... else handle_on_demand_fetch_update ... fi Cheers Heiko ``` ## W. Trevor King, 2012-11-27 20:21 Subject: Re: [PATCH v4 3/4] git-submodule update: Add --branch option Message-ID: <20121127202103.GD15213@odin.tremily.us> URL: https://gitlist.dev/e/20121127202103.GD15213%40odin.tremily.us In-Reply-To: <20121127185142.GB4185@book.hvoigt.net> ``` On Tue, Nov 27, 2012 at 07:51:42PM +0100, Heiko Voigt wrote: > On Mon, Nov 26, 2012 at 04:00:18PM -0500, W. Trevor King wrote: > > -b:: > > --branch:: > > - Branch of repository to add as submodule. > > + When used with the add command, gives the branch of repository to > > + add as submodule. > > ++ > > +When used with the update command, checks out a branch named > > +`submodule..branch` (as set by `--local-branch`) pointing at the > > +current HEAD SHA-1. This is useful for commands like `update > > +--rebase` that do not work on detached heads. > > Since you are reusing this option for update it further convinces me > that reusing it for add makes sense and simplifies the logic for users. > > I think an optional argument for --branch would be nice in the update > case: > > $ git submodule update --branch=master > > would then allow a user that has not configured anything (except the > branch tracking info in the submodule of course) to pull all submodules > master branches. Sounds good to me. Remember that this is checking the branch and pointing it at $sha1 (preparing for the pull), not pulling remote branches. The pull happens in a later $ git submodules foreach 'git pull' > > diff --git a/git-submodule.sh b/git-submodule.sh > > index c51b6ae..28eb4b1 100755 > > --- a/git-submodule.sh > > +++ b/git-submodule.sh > > @@ -627,7 +631,7 @@ Maybe you want to use 'update --init'?")" > > die "$(eval_gettext "Unable to find current revision in submodule path '\$sm_path'")" > > fi > > > > - if test "$subsha1" != "$sha1" -o -n "$force" > > + if test "$subsha1" != "$sha1" -o -n "$force" -o "$update_module" = "branch" > > As said before I think separating your code from the current update > logic will simplify the handling below. This felt less invasive (it avoids duplicating the recursion logic), but I don't mind breaking it into a separate function/block. > > must_die_on_failure= > > case "$update_module" in > > rebase) > > command="git rebase" > > - die_msg="$(eval_gettext "Unable to rebase '\$sha1' in submodule path '\$sm_path'")" > > + die_msg="$(eval_gettext "Unable to rebase '\$sha1' in submodule path '\$sm_path'")" > > say_msg="$(eval_gettext "Submodule path '\$sm_path': rebased into '\$sha1'")" > > - must_die_on_failure=yes > > + must_die_on_failure=yes > > Please always cleanup whitespace changes. Oops, sloppy me. Will fix. > > then > > - die_with_status 2 "$die_msg" > > - else > > - err="${err};$die_msg" > > - continue > > + if (clear_local_git_env; cd "$sm_path" && > > + git branch -f "$branch" "$sha1" && > > + git checkout "$branch") > > You wrote in earlier emails that you wanted to protect the user from > non-fastforward changes. So I would expect a > > $ git pull --ff-only I'm not pulling here, I'm doing a regular `submodule update`, and after that's done I checkout the branch pointing at the $sha1 to which the branch was just updated. All the submodule-state-clobbering caveats of a usual `submodule update` still apply to this new `submodule update --branch`, and I'm fine with that. > BTW, I am more and more convinced that an automatically manufactured > commit on update with --branch should be the default. Again, there's nothing to update. The pull happens in a separate step. Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy ``` ## W. Trevor King, 2012-11-27 21:18 Subject: Re: [PATCH v4 4/4] Hack fix for 'submodule update does not fetch already present commits' Message-ID: <20121127211851.GE15213@odin.tremily.us> URL: https://gitlist.dev/e/20121127211851.GE15213%40odin.tremily.us In-Reply-To: <30459164cc221165a20cd4a54daac76ddb101269.1353962698.git.wking@tremily.us> ``` On Tue, Nov 27, 2012 at 02:01:05PM -0500, W. Trevor King wrote: > On Tue, Nov 27, 2012 at 07:31:25PM +0100, Heiko Voigt wrote: > > On Mon, Nov 26, 2012 at 04:00:15PM -0500, W. Trevor King wrote: > > > Because you need to recurse through submodules for `update --branch` > > > even if "$subsha1" == "$sha1", I had to amend the conditional > > > controlling that block. This broke one of the existing tests, which I > > > "fixed" in patch 4. I think a proper fix would involve rewriting > > > > > > (clear_local_git_env; cd "$sm_path" && > > > ( (rev=$(git rev-list -n 1 $sha1 --not --all 2>/dev/null) && > > > test -z "$rev") || git-fetch)) || > > > die "$(eval_gettext "Unable to fetch in submodule path '\$sm_path'")" > > > > > > but I'm not familiar enough with rev-list to want to dig into that > > > yet. If feedback for the earlier three patches is positive, I'll work > > > up a clean fix and resubmit. > > > > You probably need to separate your handling here. The comparison of the > > currently checked out sha1 and the recorded sha1 is an optimization > > which skips unnecessary fetching in case the submodules commits are > > already correct. This code snippet checks whether the to be checked out > > sha1 is already local and also skips the fetch if it is. We should not > > break that. > > Agreed. However, determining if the target $sha1 is local should have > nothing to do with the current checked out $subsha1. Erm, I clearly wasn't getting enough sleep heading into yesterday, because when I drop the hack patch #4, reinstall, and retest, I no longer get the bad-fetch error. I'm not quite sure what was going on, but please pretend I never mentioned it ;). -- 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 ``` ## Jens Lehmann, 2012-11-27 23:19 Subject: Re: [PATCH v4 2/4] git-submodule init: Record submodule..branch in repository config. Message-ID: <50B54A68.60309@web.de> URL: https://gitlist.dev/e/50B54A68.60309%40web.de In-Reply-To: <6734714e90064b3932126565e3027d7edcf45d51.1353962698.git.wking@tremily.us> ``` Am 26.11.2012 22:00, schrieb W. Trevor King: > From: "W. Trevor King" > > This allows users to override the .gitmodules value with a > per-repository value. Your intentions makes lots of sense, but your patch does more than that. Copying the branch setting into .git/config sets the initial branch setting into stone. That makes it impossible to have a branch "foo" in the superproject using a branch "bar" in a submodule and another superproject branch "frotz" using branch "nitfol" for the same submodule. You should use the branch setting from .git/config if present and fall back to the branch setting from .gitmodules if not, which would enable the user to have her own setting if she doesn't like what upstream provides but would still enable others to follow different submodule branches in different superproject branches. > Signed-off-by: W. Trevor King > --- > Documentation/config.txt | 9 +++++---- > git-submodule.sh | 7 +++++++ > t/t7400-submodule-basic.sh | 18 ++++++++++++++++++ > 3 files changed, 30 insertions(+), 4 deletions(-) > > diff --git a/Documentation/config.txt b/Documentation/config.txt > index 11f320b..1304499 100644 > --- a/Documentation/config.txt > +++ b/Documentation/config.txt > @@ -1994,10 +1994,11 @@ status.submodulesummary:: > submodule..path:: > submodule..url:: > submodule..update:: > - The path within this project, URL, and the updating strategy > - for a submodule. These variables are initially populated > - by 'git submodule init'; edit them to override the > - URL and other values found in the `.gitmodules` file. See > +submodule..branch:: > + The path within this project, URL, the updating strategy, and the > + local branch name for a submodule. These variables are initially > + populated by 'git submodule init'; edit them to override the URL and > + other values found in the `.gitmodules` file. See > linkgit:git-submodule[1] and linkgit:gitmodules[5] for details. > > submodule..fetchRecurseSubmodules:: > diff --git a/git-submodule.sh b/git-submodule.sh > index 6eed008..c51b6ae 100755 > --- a/git-submodule.sh > +++ b/git-submodule.sh > @@ -505,6 +505,13 @@ cmd_init() > test -n "$(git config submodule."$name".update)" || > git config submodule."$name".update "$upd" || > die "$(eval_gettext "Failed to register update mode for submodule path '\$sm_path'")" > + > + # Copy "branch" setting when it is not set yet > + branch="$(git config -f .gitmodules submodule."$name".branch)" > + test -z "$branch" || > + test -n "$(git config submodule."$name".branch)" || > + git config submodule."$name".branch "$branch" || > + die "$(eval_gettext "Failed to register branch for submodule path '\$sm_path'")" > done > } > > diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh > index fc08647..3dc8237 100755 > --- a/t/t7400-submodule-basic.sh > +++ b/t/t7400-submodule-basic.sh > @@ -236,6 +236,24 @@ test_expect_success 'submodule add --local-branch= --branch' ' > ) > ' > > +test_expect_success 'init should register submodule branch in .git/config' ' > + ( > + cd addtest && > + git submodule init && > + test "$(git config submodule.submod-follow.branch)" = "final" > + ) > +' > + > +test_expect_success 'local config should override .gitmodules branch' ' > + ( > + cd addtest && > + rm -fr submod-follow && > + git config submodule.submod-follow.branch initial > + git submodule init && > + test "$(git config submodule.submod-follow.branch)" = "initial" > + ) > +' > + > test_expect_success 'setup - add an example entry to .gitmodules' ' > GIT_CONFIG=.gitmodules \ > git config submodule.example.url git://example.com/init.git > ``` ## Heiko Voigt, 2012-11-27 23:28 Subject: Re: Re: [PATCH v4 0/4] git-submodule add: Add --local-branch option Message-ID: <20121127232858.GA4742@book.hvoigt.net> URL: https://gitlist.dev/e/20121127232858.GA4742%40book.hvoigt.net In-Reply-To: <20121127190105.GQ10656@odin.tremily.us> ``` Hi, On Tue, Nov 27, 2012 at 02:01:05PM -0500, W. Trevor King wrote: > On Tue, Nov 27, 2012 at 07:31:25PM +0100, Heiko Voigt wrote: > The v4 series leaves the remote branch amigious, but it helps you > point the local branch at the right hash so that future calls to > > $ git submodule foreach 'git pull' > > can use the branch's .git/modules//config settings. But IMO thats the functionality which should be implemented in submodule update and not left to the user. > > I would think more of some convention like: > > > > $ git checkout -t origin/$branch > > > > when first initialising the submodule with e.g. > > > > $ git submodule update --init --branch > > > > Then later calls of > > > > $ git submodule update --branch > > > > would have a branch configured to pull from. I imagine that results in > > a similar behavior gerrit is doing on the server side? > > That sounds like it's doing pretty much the same thing. Can you think > of a test that would distinguish it from my current v4 implementation? Well the main difference is that gerrit is automatically updating the superproject AFAIK. I would like it if we could implement the same workflow support in the submodule script. It seems to me that this is already proven to be useful workflow. I do not have a test but a small draft diff (completely untested, quick and dirty) to illustrate the approach I am talking about. You can find the whole change at https://github.com/hvoigt/git/commits/hv/floating_submodules_draft and the interesting patch for easy commenting below[1]. > > How about reusing the -b|--branch option for add? Since we only change > > the behavior when submodule.$name.update is set to branch it seems > > reasonable to me. Opinions? > > That was the approach I used in v1, but people were concerned that we > would be stomping on previously unclaimed config space. Since noone > has pointed out other uses besides Gerrit's very similar case, I'm not > sure if that is still an issue. Could you point me to that mail? I cannot seem to find it in my archive. > > > Because you need to recurse through submodules for `update --branch` > > > even if "$subsha1" == "$sha1", I had to amend the conditional > > > controlling that block. This broke one of the existing tests, which I > > > "fixed" in patch 4. I think a proper fix would involve rewriting > > > > > > (clear_local_git_env; cd "$sm_path" && > > > ( (rev=$(git rev-list -n 1 $sha1 --not --all 2>/dev/null) && > > > test -z "$rev") || git-fetch)) || > > > die "$(eval_gettext "Unable to fetch in submodule path '\$sm_path'")" > > > > > > but I'm not familiar enough with rev-list to want to dig into that > > > yet. If feedback for the earlier three patches is positive, I'll work > > > up a clean fix and resubmit. > > > > You probably need to separate your handling here. The comparison of the > > currently checked out sha1 and the recorded sha1 is an optimization > > which skips unnecessary fetching in case the submodules commits are > > already correct. This code snippet checks whether the to be checked out > > sha1 is already local and also skips the fetch if it is. We should not > > break that. > > Agreed. However, determining if the target $sha1 is local should have > nothing to do with the current checked out $subsha1. See my draft or the diff below for an illustration of the splitup. Cheers Heiko [1] diff --git a/git-submodule.sh b/git-submodule.sh index 9ad4370..3fa1465 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -183,6 +183,7 @@ module_clone() sm_path=$1 url=$2 reference="$3" + branch="$4" quiet= if test -n "$GIT_QUIET" then @@ -209,6 +210,8 @@ module_clone() clear_local_git_env git clone $quiet -n ${reference:+"$reference"} \ --separate-git-dir "$gitdir" "$url" "$sm_path" + test -n "$branch" && (cd $sm_path && + git checkout -t origin/$branch) ) || die "$(eval_gettext "Clone of '\$url' into submodule path '\$sm_path' failed")" fi @@ -361,7 +364,7 @@ Use -f if you really want to add it." >&2 else - module_clone "$sm_path" "$realrepo" "$reference" || exit + module_clone "$sm_path" "$realrepo" "$reference" "$local_branch" || exit ( clear_local_git_env cd "$sm_path" && @@ -577,6 +580,12 @@ handle_on_demand_update () { fi } +handle_tracking_branch_update () { + (clear_local_git_env; cd "$sm_path" && + git-checkout $branch && git-pull --ff-only) || + die "$(eval_gettext "Unable to pull branch '\$branch' in submodule path '\$sm_path'")" +} + # # Update each submodule path to correct revision, using clone and checkout as needed # @@ -648,6 +657,7 @@ cmd_update() cloned_modules= module_list "$@" | { err= + floating_submodules= while read mode sha1 stage sm_path do die_if_unmatched "$mode" @@ -684,7 +694,7 @@ Maybe you want to use 'update --init'?")" if ! test -d "$sm_path"/.git -o -f "$sm_path"/.git then - module_clone "$sm_path" "$url" "$reference"|| exit + module_clone "$sm_path" "$url" "$reference" "$branch" || exit cloned_modules="$cloned_modules;$name" subsha1= else @@ -693,7 +703,13 @@ Maybe you want to use 'update --init'?")" die "$(eval_gettext "Unable to find current revision in submodule path '\$sm_path'")" fi - handle_on_demand_update + if test "$update_module" = "branch" + then + handle_tracking_branch_update + floating_submodules="$floating_submodules $sm_path" + else + handle_on_demand_update + fi if test -n "$recursive" then @@ -727,6 +743,11 @@ Maybe you want to use 'update --init'?")" IFS=$OIFS exit 1 fi + if test -n "$floating_submodules" + then + git add $floating_submodules && + git commit -m "Updated submodules" + fi } } ``` ## W. Trevor King, 2012-11-28 00:40 Subject: Re: [PATCH v4 2/4] git-submodule init: Record submodule..branch in repository config. Message-ID: <20121128004025.GF15213@odin.tremily.us> URL: https://gitlist.dev/e/20121128004025.GF15213%40odin.tremily.us In-Reply-To: <50B54A68.60309@web.de> ``` On Wed, Nov 28, 2012 at 12:19:04AM +0100, Jens Lehmann wrote: > Am 26.11.2012 22:00, schrieb W. Trevor King: > > From: "W. Trevor King" > > > > This allows users to override the .gitmodules value with a > > per-repository value. > > [snip problems]. You should use the branch setting from .git/config > if present and fall back to the branch setting from .gitmodules if > not, which would enable the user to have her own setting if she > doesn't like what upstream provides but would still enable others to > follow different submodule branches in different superproject > branches. Sounds good. Will fix in v5. -- 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 ``` ## W. Trevor King, 2012-11-28 02:42 Subject: Re: Re: [PATCH v4 0/4] git-submodule add: Add --local-branch option Message-ID: <20121128024205.GG15213@odin.tremily.us> URL: https://gitlist.dev/e/20121128024205.GG15213%40odin.tremily.us In-Reply-To: <20121127232858.GA4742@book.hvoigt.net> ``` On Wed, Nov 28, 2012 at 12:28:58AM +0100, Heiko Voigt wrote: > On Tue, Nov 27, 2012 at 02:01:05PM -0500, W. Trevor King wrote: > > On Tue, Nov 27, 2012 at 07:31:25PM +0100, Heiko Voigt wrote: > > The v4 series leaves the remote branch amigious, but it helps you > > point the local branch at the right hash so that future calls to > > > > $ git submodule foreach 'git pull' > > > > can use the branch's .git/modules//config settings. > > But IMO thats the functionality which should be implemented in submodule > update and not left to the user. Then you might need submodule..local-branch, submodule..remote-repository, and submodule..remote-branch to configure $ git checkout submodule..local-branch $ git pull submodule..remote-repository submodule..remote-branch and this would ignore the $sha1 stored in the gitlink (which all of the other update commands use). This ignoring-the-$sha1 bit made me think that a built-in pull wasn't a good fit for 'submodule update'. Maybe if it went into a new 'submodule pull'? Then users have a clear distinction: * 'update' to push superproject $sha1 changes into the submodules * 'pull' to push upstream-branch changes into the submodules > > > I would think more of some convention like: > > > > > > $ git checkout -t origin/$branch > > > > > > when first initialising the submodule with e.g. > > > > > > $ git submodule update --init --branch > > > > > > Then later calls of > > > > > > $ git submodule update --branch > > > > > > would have a branch configured to pull from. I imagine that results in > > > a similar behavior gerrit is doing on the server side? > > > > That sounds like it's doing pretty much the same thing. Can you think > > of a test that would distinguish it from my current v4 implementation? > > Well the main difference is that gerrit is automatically updating the > superproject AFAIK. I would like it if we could implement the same > workflow support in the submodule script. It seems to me that this is > already proven to be useful workflow. Ah, sorry, I meant the configuring which remote branch you were pulling from happens at submodule initialization (via .git/modules/…) for both your workflow and my v4. You're right that having a builtin pull is different from my v4. > https://github.com/hvoigt/git/commits/hv/floating_submodules_draft I looked over this before, but maybe not thoroughly enough ;). > > > How about reusing the -b|--branch option for add? Since we only change > > > the behavior when submodule.$name.update is set to branch it seems > > > reasonable to me. Opinions? > > > > That was the approach I used in v1, but people were concerned that we > > would be stomping on previously unclaimed config space. Since noone > > has pointed out other uses besides Gerrit's very similar case, I'm not > > sure if that is still an issue. > > Could you point me to that mail? I cannot seem to find it in my archive. Hmm. It seems like Phil's initial response was (accidentally?) off list. The relevant portion was: On Mon, Oct 22, 2012 at 06:03:53PM -0400, Phil Hord wrote: > Some projects now use the 'branch' config value to record the tracking > branch for the submodule. Some ascribe different meaning to the > configuration if the value is given vs. undefined. For example, see > the Gerrit submodule-subscription mechanism. This change will cause > those workflows to behave differently than they do now. > > I do like the idea, but I wish it had a different name for the > recording. Maybe --record-branch=${BRANCH} as an extra switch so the > action is explicitly requested. As I said, I'm happy to go back to --branch if opinions have changed. On Wed, Nov 28, 2012 at 12:28:58AM +0100, Heiko Voigt wrote: > On Tue, Nov 27, 2012 at 02:01:05PM -0500, W. Trevor King wrote: > > On Tue, Nov 27, 2012 at 07:31:25PM +0100, Heiko Voigt wrote: > > > > Because you need to recurse through submodules for `update --branch` > > > > even if "$subsha1" == "$sha1", I had to amend the conditional > > > > controlling that block. This broke one of the existing tests, which I > > > > "fixed" in patch 4. I think a proper fix would involve rewriting > > > > > > > > (clear_local_git_env; cd "$sm_path" && > > > > ( (rev=$(git rev-list -n 1 $sha1 --not --all 2>/dev/null) && > > > > test -z "$rev") || git-fetch)) || > > > > die "$(eval_gettext "Unable to fetch in submodule path '\$sm_path'")" > > > > > > > > but I'm not familiar enough with rev-list to want to dig into that > > > > yet. If feedback for the earlier three patches is positive, I'll work > > > > up a clean fix and resubmit. > > > > > > You probably need to separate your handling here. The comparison of the > > > currently checked out sha1 and the recorded sha1 is an optimization > > > which skips unnecessary fetching in case the submodules commits are > > > already correct. This code snippet checks whether the to be checked out > > > sha1 is already local and also skips the fetch if it is. We should not > > > break that. > > > > Agreed. However, determining if the target $sha1 is local should have > > nothing to do with the current checked out $subsha1. > > See my draft or the diff below for an illustration of the splitup. > > [snip diff] This looks fine, but my current --branch implementation (which doesn't pull) is only a thin branch-checkout layer on top of the standard `update` functionality. I'm still unsure if built-in pulls are worth the configuration trouble. I'll sleep on it. Maybe I'll feel better about them tomorrow ;). 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 ``` ## W. Trevor King, 2012-11-29 16:12 Subject: [RFC] git-submodule update: Add --commit option Message-ID: <20121129161216.GB23580@odin.tremily.us> URL: https://gitlist.dev/e/20121129161216.GB23580%40odin.tremily.us In-Reply-To: <20121127185142.GB4185@book.hvoigt.net> ``` This option triggers automatic commits when `submodule update` changes any gitlinked submodule SHA-1s. The commit message contains a `shortlog` summary of the changes for each changed submodule. --- On Tue, Nov 27, 2012 at 07:51:42PM +0100, Heiko Voigt wrote: > BTW, I am more and more convinced that an automatically manufactured > commit on update with --branch should be the default. What do other > think? Sascha raised a concern that he would not want this, but as far as > I understood he let the CI-server do that so I see no downside to > natively adding that to git. People who want to manually craft those > commits can still amend the generated commit. Since this is all about > helping people keeping their submodules updated why not go the full way? Here's a first pass (without documentation) for automatic commits on submodule updates. There have been a number of requests for automatically-committed submodule updates due to submodule upstreams. This patch shows how you can do that (if applied with my `submodule update --remote` series), and reuse the same logic to automatically commit changes due to local submodule changes (as shown here in the new test). I think the logic is pretty good, but the implementation is pretty ugly due to POSIX shell variable limitations. I'm basically trying to pass an array of [(name, sm_path, sha1, subsha1), ...] into commit_changes(). I though about perling-out in commit_changes(), but I lack sufficient perl-fu to know how to tie clear_local_git_env, cd, and shortlog up in a single open2 call. If anyone can give me some implementation pointers, that would be very helpful. This is against v1.8.0 (without my --remote series). To apply on top of the --remote series, you'd have to save the original gitlinked $sha1 and use that original value when constructing changed_modules. I can attach this to the end of the --remote series if desired, but I think this patch could also stand on its own. Obviously this still needs documentation, etc., but I wanted feedback on the implementation before I started digging into that. Cheers, Trevor --- git-submodule.sh | 67 ++++++++++++++++++++++++++++++++++++++++++++- t/t7406-submodule-update.sh | 19 +++++++++++++ 2 files changed, 85 insertions(+), 1 deletion(-) diff --git a/git-submodule.sh b/git-submodule.sh index ab6b110..d9a59af 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -8,7 +8,7 @@ dashless=$(basename "$0" | sed -e 's/-/ /') USAGE="[--quiet] add [-b branch] [-f|--force] [--reference ] [--] [] or: $dashless [--quiet] status [--cached] [--recursive] [--] [...] or: $dashless [--quiet] init [--] [...] - or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--rebase] [--reference ] [--merge] [--recursive] [--] [...] + or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--commit] [--rebase] [--reference ] [--merge] [--recursive] [--] [...] or: $dashless [--quiet] summary [--cached|--files] [--summary-limit ] [commit] [--] [...] or: $dashless [--quiet] foreach [--recursive] or: $dashless [--quiet] sync [--] [...]" @@ -21,6 +21,7 @@ require_work_tree command= branch= force= +commit= reference= cached= recursive= @@ -240,6 +241,52 @@ module_clone() } # +# Commit changed submodule gitlinks +# +# $1 = name-a;sha1-a;subsha1-a\n[name-b;sha1-b;subsha1-b\n...] +# +commit_changes() +{ + echo "commiting $1" + OIFS="$IFS" + IFS=";" + paths=$(echo "$1" | + while read name sm_path sha1 subsha1 + do + echo "$sm_path" + done + ) + names=$(echo "$1" | + while read name sm_path sha1 subsha1 + do + printf ' %s' "$name" + done + ) + summary="$(eval_gettext "Updated submodules:")$names" + body=$(echo "$1" | + while read name sm_path sha1 subsha1 + do + if test "$name" = "$sm_path" + then + printf 'Changes to %s:\n\n' "$name" + else + printf 'Changes to %s (%s):\n\n' "$name" "$sm_path" + fi + ( + clear_local_git_env + cd "$sm_path" && + git shortlog "${sha1}..${subsha1}" || + die "$(eval_gettext "Unable to generate shortlog in submodule path '\$sm_path'")" + ) + done + ) + IFS="$OIFS" + message="$(printf '%s\n\n%s\n' "$summary" "$body")" + echo "message: [$message]" + git commit -m "$message" $paths +} + +# # Add a new submodule to the working tree, .gitmodules and the index # # $@ = repo path @@ -515,6 +562,9 @@ cmd_update() -f|--force) force=$1 ;; + --commit) + commit=1 + ;; -r|--rebase) update="rebase" ;; @@ -557,6 +607,7 @@ cmd_update() fi cloned_modules= + changed_modules= module_list "$@" | { err= while read mode sha1 stage sm_path @@ -660,6 +711,15 @@ Maybe you want to use 'update --init'?")" err="${err};$die_msg" continue fi + + subsha1=$(clear_local_git_env; cd "$sm_path" && + git rev-parse --verify HEAD) || + die "$(eval_gettext "Unable to find new revision in submodule path '\$sm_path'")" + + if test "$subsha1" != "$sha1" + then + changed_modules=$(printf '%s%s\n' "$changed_modules" "$name;$sm_path;$sha1;$subsha1") + fi fi if test -n "$recursive" @@ -680,6 +740,11 @@ Maybe you want to use 'update --init'?")" fi done + if test -z "$err" -a -n "$commit" -a -n "$changed_modules" + then + commit_changes "$changed_modules" + fi + if test -n "$err" then OIFS=$IFS diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh index 1542653..4c8bb5d 100755 --- a/t/t7406-submodule-update.sh +++ b/t/t7406-submodule-update.sh @@ -163,6 +163,25 @@ test_expect_success 'submodule update --merge staying on master' ' ) ' +test_expect_success 'submodule update --commit --rebase should commit gitlink changes' ' + (cd super/submodule && + git reset --hard HEAD~1 && + echo "local change" > local-file && + git add local-file && + test_tick && + git commit -m "local change" + ) && + (cd super && + git submodule update --commit --rebase submodule && + test "$(git log -1 --oneline)" = "bbdbe2d Updated submodules: submodule" + ) && + (cd submodule && + git remote add super-submodule ../super/submodule && + git pull super-submodule master + ) && + test "a" = "b" +' + test_expect_success 'submodule update - rebase in .git/config' ' (cd super && git config submodule.submodule.update rebase -- 1.8.0.1.gaaf2ac7.dirty ``` ## W. Trevor King, 2012-11-29 16:21 Subject: Re: [RFC] git-submodule update: Add --commit option Message-ID: <20121129162154.GA27409@odin.tremily.us> URL: https://gitlist.dev/e/20121129162154.GA27409%40odin.tremily.us In-Reply-To: <20121129161216.GB23580@odin.tremily.us> ``` On Thu, Nov 29, 2012 at 11:12:16AM -0500, W. Trevor King wrote: > + test "a" = "b" This kills the test (with --immediate) so you can look at the generated commit. If you actually want the test to pass (e.g. if this becomes a PATCH and not an RFC), this line should be removed. -- 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 ``` ## W. Trevor King, 2012-11-29 16:27 Subject: Re: [RFC] git-submodule update: Add --commit option Message-ID: <20121129162751.GB27409@odin.tremily.us> URL: https://gitlist.dev/e/20121129162751.GB27409%40odin.tremily.us In-Reply-To: <20121129161216.GB23580@odin.tremily.us> ``` On Thu, Nov 29, 2012 at 11:12:16AM -0500, W. Trevor King wrote: > + test "$(git log -1 --oneline)" = "bbdbe2d Updated submodules: submodule" s/bbdbe2d/cd69713/ I forgot to update the SHA-1 here after tweaking the commit message format. I'd like to rewrite this test so it won't use the SHA-1, but this was the quickest way to check that the commit message and gitlink were both changed appropriately. -- 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 ``` ## Phil Hord, 2012-11-29 18:51 Subject: Re: Re: [PATCH v4 0/4] git-submodule add: Add --local-branch option Message-ID: URL: https://gitlist.dev/e/CABURp0rAJ3mVf2pN-B7NqBDysdCaZGfm2EHSWKr5iynzF6dA-g%40mail.gmail.com In-Reply-To: <20121128024205.GG15213@odin.tremily.us> ``` On Tue, Nov 27, 2012 at 6:28 PM, Heiko Voigt wrote: > > Hi, > > On Tue, Nov 27, 2012 at 02:01:05PM -0500, W. Trevor King wrote: > > On Tue, Nov 27, 2012 at 07:31:25PM +0100, Heiko Voigt wrote: > > The v4 series leaves the remote branch amigious, but it helps you > > point the local branch at the right hash so that future calls to > > > > $ git submodule foreach 'git pull' > > > > can use the branch's .git/modules//config settings. > > But IMO thats the functionality which should be implemented in submodule > update and not left to the user. > > > > I would think more of some convention like: > > > > > > $ git checkout -t origin/$branch > > > > > > when first initialising the submodule with e.g. > > > > > > $ git submodule update --init --branch > > > > > > Then later calls of > > > > > > $ git submodule update --branch > > > > > > would have a branch configured to pull from. I imagine that results in > > > a similar behavior gerrit is doing on the server side? > > > > That sounds like it's doing pretty much the same thing. Can you think > > of a test that would distinguish it from my current v4 implementation? > > Well the main difference is that gerrit is automatically updating the > superproject AFAIK. I would like it if we could implement the same > workflow support in the submodule script. It seems to me that this is > already proven to be useful workflow. It is proven in Gerrit, but Gerrit implements a central-server workflow. That is, only Gerrit ever floats the submodules, and he pushes the result for everyone else to share. I fear the consequences of everyone pulling submodules and then later trying to merge superprojects with someone else's breadcrumbs. Do you have some idea how this would be handled? Phil ps. Apologies for my lateness on this topic. I'm trying to catch up now. pps. Re-sent since Gmail has hidden the "plain text" option in a different place, now. On Tue, Nov 27, 2012 at 9:42 PM, W. Trevor King wrote: > On Wed, Nov 28, 2012 at 12:28:58AM +0100, Heiko Voigt wrote: >> On Tue, Nov 27, 2012 at 02:01:05PM -0500, W. Trevor King wrote: >> > On Tue, Nov 27, 2012 at 07:31:25PM +0100, Heiko Voigt wrote: >> > The v4 series leaves the remote branch amigious, but it helps you >> > point the local branch at the right hash so that future calls to >> > >> > $ git submodule foreach 'git pull' >> > >> > can use the branch's .git/modules//config settings. >> >> But IMO thats the functionality which should be implemented in submodule >> update and not left to the user. > > Then you might need submodule..local-branch, > submodule..remote-repository, and submodule..remote-branch > to configure > > $ git checkout submodule..local-branch > $ git pull submodule..remote-repository submodule..remote-branch > > and this would ignore the $sha1 stored in the gitlink (which all of > the other update commands use). This ignoring-the-$sha1 bit made me > think that a built-in pull wasn't a good fit for 'submodule update'. > Maybe if it went into a new 'submodule pull'? Then users have a clear > distinction: > > * 'update' to push superproject $sha1 changes into the submodules > * 'pull' to push upstream-branch changes into the submodules > >> > > I would think more of some convention like: >> > > >> > > $ git checkout -t origin/$branch >> > > >> > > when first initialising the submodule with e.g. >> > > >> > > $ git submodule update --init --branch >> > > >> > > Then later calls of >> > > >> > > $ git submodule update --branch >> > > >> > > would have a branch configured to pull from. I imagine that results in >> > > a similar behavior gerrit is doing on the server side? >> > >> > That sounds like it's doing pretty much the same thing. Can you think >> > of a test that would distinguish it from my current v4 implementation? >> >> Well the main difference is that gerrit is automatically updating the >> superproject AFAIK. I would like it if we could implement the same >> workflow support in the submodule script. It seems to me that this is >> already proven to be useful workflow. > > Ah, sorry, I meant the configuring which remote branch you were > pulling from happens at submodule initialization (via .git/modules/…) > for both your workflow and my v4. > > You're right that having a builtin pull is different from my v4. > >> https://github.com/hvoigt/git/commits/hv/floating_submodules_draft > > I looked over this before, but maybe not thoroughly enough ;). > >> > > How about reusing the -b|--branch option for add? Since we only change >> > > the behavior when submodule.$name.update is set to branch it seems >> > > reasonable to me. Opinions? >> > >> > That was the approach I used in v1, but people were concerned that we >> > would be stomping on previously unclaimed config space. Since noone >> > has pointed out other uses besides Gerrit's very similar case, I'm not >> > sure if that is still an issue. >> >> Could you point me to that mail? I cannot seem to find it in my archive. > > Hmm. It seems like Phil's initial response was (accidentally?) off > list. The relevant portion was: > > On Mon, Oct 22, 2012 at 06:03:53PM -0400, Phil Hord wrote: >> Some projects now use the 'branch' config value to record the tracking >> branch for the submodule. Some ascribe different meaning to the >> configuration if the value is given vs. undefined. For example, see >> the Gerrit submodule-subscription mechanism. This change will cause >> those workflows to behave differently than they do now. >> >> I do like the idea, but I wish it had a different name for the >> recording. Maybe --record-branch=${BRANCH} as an extra switch so the >> action is explicitly requested. > > As I said, I'm happy to go back to --branch if opinions have changed. > > On Wed, Nov 28, 2012 at 12:28:58AM +0100, Heiko Voigt wrote: >> On Tue, Nov 27, 2012 at 02:01:05PM -0500, W. Trevor King wrote: >> > On Tue, Nov 27, 2012 at 07:31:25PM +0100, Heiko Voigt wrote: >> > > > Because you need to recurse through submodules for `update --branch` >> > > > even if "$subsha1" == "$sha1", I had to amend the conditional >> > > > controlling that block. This broke one of the existing tests, which I >> > > > "fixed" in patch 4. I think a proper fix would involve rewriting >> > > > >> > > > (clear_local_git_env; cd "$sm_path" && >> > > > ( (rev=$(git rev-list -n 1 $sha1 --not --all 2>/dev/null) && >> > > > test -z "$rev") || git-fetch)) || >> > > > die "$(eval_gettext "Unable to fetch in submodule path '\$sm_path'")" >> > > > >> > > > but I'm not familiar enough with rev-list to want to dig into that >> > > > yet. If feedback for the earlier three patches is positive, I'll work >> > > > up a clean fix and resubmit. >> > > >> > > You probably need to separate your handling here. The comparison of the >> > > currently checked out sha1 and the recorded sha1 is an optimization >> > > which skips unnecessary fetching in case the submodules commits are >> > > already correct. This code snippet checks whether the to be checked out >> > > sha1 is already local and also skips the fetch if it is. We should not >> > > break that. >> > >> > Agreed. However, determining if the target $sha1 is local should have >> > nothing to do with the current checked out $subsha1. >> >> See my draft or the diff below for an illustration of the splitup. >> >> [snip diff] > > This looks fine, but my current --branch implementation (which doesn't > pull) is only a thin branch-checkout layer on top of the standard > `update` functionality. I'm still unsure if built-in pulls are worth > the configuration trouble. I'll sleep on it. Maybe I'll feel better > about them tomorrow ;). > > 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 ```