# Re: [PATCH] git-submodule add: Record branch name in .gitmodules 19 messages from 2012-10-23 to 2012-10-29. Participants: W. Trevor King, Jens Lehmann, Jeff King, Shawn Pearce, Phil Hord. Thread: https://gitlist.dev/t/31926 ## W. Trevor King, 2012-10-23 20:44 Subject: Re: [PATCH] git-submodule add: Record branch name in .gitmodules Message-ID: <20121023204437.GE28592@odin.tremily.us> URL: https://gitlist.dev/e/20121023204437.GE28592%40odin.tremily.us In-Reply-To: ``` On Tue, Oct 23, 2012 at 03:44:36PM -0400, W. Trevor King wrote: > On Tue, Oct 23, 2012 at 12:16:22PM -0700, Nahor wrote: > > On 2012-10-22 09:34, W. Trevor King wrote: > > For instance, the module may later be updated to a commit in branch B > > instead of branch A. Unless you remember to also update .gitmodule, you > > have then inconsistent information. > > But you're explicitly *using* the configured setting in > > git config --file $toplevel/.gitmodules submodule.$name.branch > > That should be a reminder that the configuration is important, and > you'll remember to change it. To make my case more cleanly, people already handle all the troublesome cases for branch.$name.remote, so handling similar upstream volatility for submodule.$name.branch should not be too difficult or surprising. On Tue, Oct 23, 2012 at 03:58:48PM -0400, Phil Hord wrote: > On Mon, Oct 22, 2012 at 6:55 PM, W. Trevor King wrote: > > How about -r/--record, with the recorded name being optional? > > > > --record-branch[=] > > I like that just fine. > > > This would satisfy Gerrit users that wanted to use '.', but also > > satisfy me with: > > > > git submodule add -rb=master foo bar > > > > However, there is a change that people would see that, and then use > > > > git submodule add -r -b=master foo bar > > > > which would checkout the HEAD from foo and store `-b=master` in > > submodule.$name.branch. > > I don't think it would. Ah, right, forcing the = attached case would mean they'd have to use git submodule add -r=-b=master which doesn't sound like the sort of thing you'd do accidentally. > Though I see in rev-parse--parseopts that the use of > optional-argument options "is discouraged". I'll work up a v2 patch and we'll see if anyone complains ;). -- 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-10-23 21:57 Subject: [PATCH v2] git-submodule add: Add -r/--record option. Message-ID: <1f6ee2966ffe0f58f4b96ae0efb2ffb13e2fa2d8.1351029479.git.wking@tremily.us> URL: https://gitlist.dev/e/1f6ee2966ffe0f58f4b96ae0efb2ffb13e2fa2d8.1351029479.git.wking%40tremily.us In-Reply-To: <20121023204437.GE28592@odin.tremily.us> ``` 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 this setting to “indicate the branch of a submodule project that when updated will trigger automatic update of its registered gitlink.” [1] I'm not clear on what that means, but they accept special values like '.', so their usage is not compatible with Ævar's proposal. 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..f9c74d6 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.1.g61a31f6.dirty ``` ## Jens Lehmann, 2012-10-24 19:15 Subject: Re: [PATCH v2] git-submodule add: Add -r/--record option. Message-ID: <50883E54.4080507@web.de> URL: https://gitlist.dev/e/50883E54.4080507%40web.de In-Reply-To: <1f6ee2966ffe0f58f4b96ae0efb2ffb13e2fa2d8.1351029479.git.wking@tremily.us> ``` I still fail to see what adding that functionality to the submodule command buys us (unless we also add code which really uses the branch setting). What's wrong with doing a simple: git config -f .gitmodules submodule..branch on the command line when you want to use the branch setting for your own purposes? You could easily wrap that into a helper script, no? Am 23.10.2012 23:57, schrieb W. Trevor King: > 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 this setting to > > “indicate the branch of a submodule project that when updated will > trigger automatic update of its registered gitlink.” [1] > > I'm not clear on what that means, but they accept special values like > '.', so their usage is not compatible with Ævar's proposal. > > 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..f9c74d6 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 > ``` ## W. Trevor King, 2012-10-25 00:53 Subject: Re: [PATCH v2] git-submodule add: Add -r/--record option. Message-ID: <20121025005307.GE801@odin.tremily.us> URL: https://gitlist.dev/e/20121025005307.GE801%40odin.tremily.us In-Reply-To: <50883E54.4080507@web.de> ``` On Wed, Oct 24, 2012 at 09:15:32PM +0200, Jens Lehmann wrote: > I still fail to see what adding that functionality to the submodule > command buys us (unless we also add code which really uses the branch > setting). What's wrong with doing a simple: > > git config -f .gitmodules submodule..branch > > on the command line when you want to use the branch setting for your > own purposes? You could easily wrap that into a helper script, no? Sure. But why maintain my own helper script if I can edit git-submodules.sh? It seems like a number of people are using this config option, and they generally store the same name in it that they use to create the submodule. This way I can save them time too. -- 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-10-25 22:14 Subject: Re: [PATCH v2] git-submodule add: Add -r/--record option. Message-ID: <20121025221431.GH13647@odin.tremily.us> URL: https://gitlist.dev/e/20121025221431.GH13647%40odin.tremily.us In-Reply-To: <1f6ee2966ffe0f58f4b96ae0efb2ffb13e2fa2d8.1351029479.git.wking@tremily.us> ``` Should I rebase this so it lands cleanly atop 38ae92e4 in next? commit 38ae92e4d027063b9b87e51a9bf12809d10066f6 Author: W. Trevor King Date: Tue Oct 23 17:00:21 2012 -0400 git-submodule: wrap branch option with "<>" in usage strings. -- 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 ``` ## Jeff King, 2012-10-26 14:00 Subject: Re: [PATCH v2] git-submodule add: Add -r/--record option. Message-ID: <20121026140056.GL1455@sigill.intra.peff.net> URL: https://gitlist.dev/e/20121026140056.GL1455%40sigill.intra.peff.net In-Reply-To: <20121025221431.GH13647@odin.tremily.us> ``` On Thu, Oct 25, 2012 at 06:14:31PM -0400, W. Trevor King wrote: > Should I rebase this so it lands cleanly atop 38ae92e4 in next? > > commit 38ae92e4d027063b9b87e51a9bf12809d10066f6 > Author: W. Trevor King > Date: Tue Oct 23 17:00:21 2012 -0400 > > git-submodule: wrap branch option with "<>" in usage strings. In general, it is not a good idea to base your patches on things in next, because it means your topic is held hostage to the one in next, which may or may not graduate to master. We can always do a merge later (and in this case, it is really just a one-line conflict). -Peff ``` ## Jens Lehmann, 2012-10-28 20:48 Subject: Re: [PATCH v2] git-submodule add: Add -r/--record option. Message-ID: <508D9A12.6010104@web.de> URL: https://gitlist.dev/e/508D9A12.6010104%40web.de In-Reply-To: <20121025005307.GE801@odin.tremily.us> ``` Am 25.10.2012 02:53, schrieb W. Trevor King: > On Wed, Oct 24, 2012 at 09:15:32PM +0200, Jens Lehmann wrote: >> I still fail to see what adding that functionality to the submodule >> command buys us (unless we also add code which really uses the branch >> setting). What's wrong with doing a simple: >> >> git config -f .gitmodules submodule..branch >> >> on the command line when you want to use the branch setting for your >> own purposes? You could easily wrap that into a helper script, no? > > Sure. But why maintain my own helper script if I can edit > git-submodules.sh? It seems like a number of people are using this > config option, and they generally store the same name in it that they > use to create the submodule. This way I can save them time too. But people are already using the "branch" setting in *different* ways: Am 23.10.2012 22:55, schrieb W. Trevor King: > As Phil pointed out, doing anything with this variable is ambiguous: > > 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 don't have a problem with the amount or complexity of the code being added, But by adding that option we may be giving the impression that it is officially sanctioned, or that it will be kept up to date by further submodule commands. I added Shawn to the CC, maybe he can comment on how the "branch" setting is used in Gerrit and what he thinks about adding code to set that with "git submodule add -r ..." to core git. ``` ## W. Trevor King, 2012-10-28 21:16 Subject: Re: [PATCH v2] git-submodule add: Add -r/--record option. Message-ID: <20121028211623.GE26675@odin.tremily.us> URL: https://gitlist.dev/e/20121028211623.GE26675%40odin.tremily.us In-Reply-To: <508D9A12.6010104@web.de> ``` On Sun, Oct 28, 2012 at 09:48:18PM +0100, Jens Lehmann wrote: > Am 25.10.2012 02:53, schrieb W. Trevor King: > > On Wed, Oct 24, 2012 at 09:15:32PM +0200, Jens Lehmann wrote: > >> I still fail to see what adding that functionality to the submodule > >> command buys us (unless we also add code which really uses the branch > >> setting). What's wrong with doing a simple: > >> > >> git config -f .gitmodules submodule..branch > >> > >> on the command line when you want to use the branch setting for your > >> own purposes? You could easily wrap that into a helper script, no? > > > > Sure. But why maintain my own helper script if I can edit > > git-submodules.sh? It seems like a number of people are using this > > config option, and they generally store the same name in it that they > > use to create the submodule. This way I can save them time too. > > But people are already using the "branch" setting in *different* ways: And they are usually storing the same string. Now, more easily. If they want a different string, it is also easier. If they don't want to use --record, they can do things however they were already doing them. I don't see the problem. > Am 23.10.2012 22:55, schrieb W. Trevor King: > > As Phil pointed out, doing anything with this variable is ambiguous: > > > > 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 don't have a problem with the amount or complexity of the code being > added, But by adding that option we may be giving the impression that it > is officially sanctioned, or that it will be kept up to date by further > submodule commands. Storing something there will be officially sanctioned. Using it for anything in particular will not be officially sanctioned. Phil's submodule_ export in foreach will expose the variable so the user can do whatever they think is appropriate with it, but it's still up to the user to give the option some kind of semantic meaning. > I added Shawn to the CC, maybe he can comment on how the "branch" > setting is used in Gerrit and what he thinks about adding code to > set that with "git submodule add -r ..." to core git. Good idea. 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 ``` ## Shawn Pearce, 2012-10-28 21:59 Subject: Re: [PATCH v2] git-submodule add: Add -r/--record option. Message-ID: URL: https://gitlist.dev/e/CAJo%3DhJt_A5FCCcvR%3DsZ5Ni%2B-ZGq%2BMjxqkONbh9k%2BA46xBH9jzA%40mail.gmail.com In-Reply-To: <508D9A12.6010104@web.de> ``` On Sun, Oct 28, 2012 at 1:48 PM, Jens Lehmann wrote: > Am 23.10.2012 22:55, schrieb W. Trevor King: >> As Phil pointed out, doing anything with this variable is ambiguous: >> >> 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 don't have a problem with the amount or complexity of the code being > added, But by adding that option we may be giving the impression that it > is officially sanctioned, or that it will be kept up to date by further > submodule commands. I added Shawn to the CC, maybe he can comment on how > the "branch" setting is used in Gerrit and what he thinks about adding > code to set that with "git submodule add -r ..." to core git. Looks like the Gerrit meaning is basically the same as Ævar's. Gerrit updates the parent project as if you had done: $ git submodule foreach 'git checkout $(git config --file $toplevel/.gitmodules submodule.$name.branch) && git pull' $ git commit -a -m "Updated submodules" $ git push and it does this automatically each time the submodule's branch is modified by the Gerrit server. On Tue, Oct 23, 2012 at 2:57 PM, W. Trevor King wrote: > I'm not clear on what that means, but they accept special values like > '.', so their usage is not compatible with Ævar's proposal. "." is a special value to mean use the parent project's branch name. So its more like this: $ git submodule foreach 'git checkout $(git --git-dir $toplevel/.git read-ref HEAD | sed s,^refs/heads/,,) && git pull' $ git commit -a -m "Updated submodules" $ git push We use "." in Gerrit to make branching an entire forest of projects easier. Setting up dev-fix-yy in the parent project will automatically track dev-fix-yy in each submodule. ``` ## W. Trevor King, 2012-10-28 22:34 Subject: Re: [PATCH v2] git-submodule add: Add -r/--record option. Message-ID: <20121028223431.GF26675@odin.tremily.us> URL: https://gitlist.dev/e/20121028223431.GF26675%40odin.tremily.us In-Reply-To: ``` On Sun, Oct 28, 2012 at 02:59:33PM -0700, Shawn Pearce wrote: > Looks like the Gerrit meaning is basically the same as Ævar's. Gerrit > updates the parent project as if you had done: > > $ git submodule foreach 'git checkout $(git config --file > $toplevel/.gitmodules submodule.$name.branch) && git pull' > $ git commit -a -m "Updated submodules" > $ git push Ah, good, then we *are* all using the option for the same thing. > On Tue, Oct 23, 2012 at 2:57 PM, W. Trevor King wrote: > > I'm not clear on what that means, but they accept special values like > > '.', so their usage is not compatible with Ævar's proposal. > > "." is a special value to mean use the parent project's branch name. > So its more like this: > > $ git submodule foreach 'git checkout $(git --git-dir $toplevel/.git > read-ref HEAD | sed s,^refs/heads/,,) && git pull' > $ git commit -a -m "Updated submodules" > $ git push > > We use "." in Gerrit to make branching an entire forest of projects > easier. Setting up dev-fix-yy in the parent project will automatically > track dev-fix-yy in each submodule. Ok. If we wanted "." expansion to be a general submodule thing, it would add a special case to Phil's submodule_ export. I don't think such a special case would be worth the mental overhead, but obviously the Gerrit folks think it is. I don't care either way on this one. 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 ``` ## Jeff King, 2012-10-29 05:34 Subject: Re: [PATCH v2] git-submodule add: Add -r/--record option. Message-ID: <20121029053401.GB30186@sigill.intra.peff.net> URL: https://gitlist.dev/e/20121029053401.GB30186%40sigill.intra.peff.net In-Reply-To: <20121028223431.GF26675@odin.tremily.us> ``` On Sun, Oct 28, 2012 at 06:34:31PM -0400, W. Trevor King wrote: > On Sun, Oct 28, 2012 at 02:59:33PM -0700, Shawn Pearce wrote: > > Looks like the Gerrit meaning is basically the same as Ævar's. Gerrit > > updates the parent project as if you had done: > > > > $ git submodule foreach 'git checkout $(git config --file > > $toplevel/.gitmodules submodule.$name.branch) && git pull' > > $ git commit -a -m "Updated submodules" > > $ git push > > Ah, good, then we *are* all using the option for the same thing. That makes me more comfortable. Your patch adds support for setting the variable initially. Does it need any special magic for maintenance, or is it something that would always be updated by hand? Right now, the variable is not an official git-submodule thing and it is OK to say "you are on your own by setting and using it from external tools". But as soon as we support setting it in the first place, it is reasonable to claim it as a bug if we do not keep it up to date for certain operations. I'm not familiar enough with the workflows around branching submodules to know whether any such operations actually exist. -Peff ``` ## W. Trevor King, 2012-10-29 10:45 Subject: Re: [PATCH v2] git-submodule add: Add -r/--record option. Message-ID: <20121029104544.GA2424@odin.tremily.us> URL: https://gitlist.dev/e/20121029104544.GA2424%40odin.tremily.us In-Reply-To: <20121029053401.GB30186@sigill.intra.peff.net> ``` On Mon, Oct 29, 2012 at 01:34:01AM -0400, Jeff King wrote: > On Sun, Oct 28, 2012 at 06:34:31PM -0400, W. Trevor King wrote: > > > On Sun, Oct 28, 2012 at 02:59:33PM -0700, Shawn Pearce wrote: > > > Looks like the Gerrit meaning is basically the same as Ævar's. Gerrit > > > updates the parent project as if you had done: > > > > > > $ git submodule foreach 'git checkout $(git config --file > > > $toplevel/.gitmodules submodule.$name.branch) && git pull' > > > $ git commit -a -m "Updated submodules" > > > $ git push > > > > Ah, good, then we *are* all using the option for the same thing. > > That makes me more comfortable. Your patch adds support for setting the > variable initially. Does it need any special magic for maintenance, or > is it something that would always be updated by hand? Everyone we've heard from so far interprets the setting as “pull from $branch in the remote repository $url to update the submodule”. With Phil's export, that would become $ git submodule foreach 'git checkout "$submodule_branch" && git pull' $ git commit -a -m "Updated submodules" $ git push As Nahor mentioned on the 23rd, there are a number of ways that the upstream branch could disappear, but Git has no way to know what the new branch setting should be. This means that even if we make “pull from $branch” interpretation official, we still couldn't do anything slick about updating it. So, yes, it will be updated by hand. -- 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 ``` ## Jeff King, 2012-10-29 10:58 Subject: Re: [PATCH v2] git-submodule add: Add -r/--record option. Message-ID: <20121029105855.GA15075@sigill.intra.peff.net> URL: https://gitlist.dev/e/20121029105855.GA15075%40sigill.intra.peff.net In-Reply-To: <20121029104544.GA2424@odin.tremily.us> ``` On Mon, Oct 29, 2012 at 06:45:44AM -0400, W. Trevor King wrote: > > > Ah, good, then we *are* all using the option for the same thing. > > > > That makes me more comfortable. Your patch adds support for setting the > > variable initially. Does it need any special magic for maintenance, or > > is it something that would always be updated by hand? > > Everyone we've heard from so far interprets the setting as “pull from > $branch in the remote repository $url to update the submodule”. With > Phil's export, that would become > > $ git submodule foreach 'git checkout "$submodule_branch" && git pull' > $ git commit -a -m "Updated submodules" > $ git push > > As Nahor mentioned on the 23rd, there are a number of ways that the > upstream branch could disappear, but Git has no way to know what the > new branch setting should be. This means that even if we make “pull > from $branch” interpretation official, we still couldn't do anything > slick about updating it. So, yes, it will be updated by hand. OK. Can you send an updated version of the patch that summarizes the situation in the commit message? I also think it is probably worth saying something in the documentation for the feature like "Note that this value is not actually used by git; however, some external tools and workflows may make use of it." -Peff ``` ## W. Trevor King, 2012-10-29 11:29 Subject: Re: [PATCH v2] git-submodule add: Add -r/--record option. Message-ID: <20121029112945.GD2424@odin.tremily.us> URL: https://gitlist.dev/e/20121029112945.GD2424%40odin.tremily.us In-Reply-To: <20121029105855.GA15075@sigill.intra.peff.net> ``` On Mon, Oct 29, 2012 at 06:58:55AM -0400, Jeff King wrote: > Can you send an updated version of the patch that summarizes the > situation in the commit message? Sure. Should I include Phil's $submodule_ export, or would you rather have that be a separate series? Phil, were you planning on rolling your patch into something more formal, or was your preliminary patch a suggestion for me to build from? -- 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 ``` ## Jeff King, 2012-10-29 11:43 Subject: Re: [PATCH v2] git-submodule add: Add -r/--record option. Message-ID: <20121029114310.GA16046@sigill.intra.peff.net> URL: https://gitlist.dev/e/20121029114310.GA16046%40sigill.intra.peff.net In-Reply-To: <20121029112945.GD2424@odin.tremily.us> ``` On Mon, Oct 29, 2012 at 07:29:45AM -0400, W. Trevor King wrote: > On Mon, Oct 29, 2012 at 06:58:55AM -0400, Jeff King wrote: > > Can you send an updated version of the patch that summarizes the > > situation in the commit message? > > Sure. Should I include Phil's $submodule_ export, or would > you rather have that be a separate series? I think it probably makes sense as a separate patch in the same series, since it is meant to support the same workflows. I am not sure it is sufficient as-is, though. It does not seem to ever clear variables, only set them, which means that values could leak across iterations of the loop, or down to recursive calls. E.g., when the first submodule has submodule.*.foo set but the second one does not, you will still end up with $submodule_foo set when you process the second one. -Peff ``` ## Phil Hord, 2012-10-29 17:38 Subject: Re: [PATCH v2] git-submodule add: Add -r/--record option. Message-ID: URL: https://gitlist.dev/e/CABURp0pFLi%2B2A%2B9wi-ZamiRze2u6z%2B6oyoCsNpWOLq_cq2L1rQ%40mail.gmail.com In-Reply-To: <20121029114310.GA16046@sigill.intra.peff.net> ``` On Mon, Oct 29, 2012 at 7:43 AM, Jeff King wrote: > On Mon, Oct 29, 2012 at 07:29:45AM -0400, W. Trevor King wrote: > >> On Mon, Oct 29, 2012 at 06:58:55AM -0400, Jeff King wrote: >> > Can you send an updated version of the patch that summarizes the >> > situation in the commit message? >> >> Sure. Should I include Phil's $submodule_ export, or would >> you rather have that be a separate series? > > I think it probably makes sense as a separate patch in the same series, > since it is meant to support the same workflows. I agree. I did expect to clean it up some, but also to suffer some review. Feel free to clean it up as you see fit and submit it with your series. > I am not sure it is sufficient as-is, though. It does not seem to ever > clear variables, only set them, which means that values could leak > across iterations of the loop, [...] E.g., when > the first submodule has submodule.*.foo set but the second one does not, > you will still end up with $submodule_foo set when you process the > second one. Good point. That should not happen. > or down to recursive calls. Frankly, I consider that to be a feature. However, I can see how it would be considered inconsistent in many ways, so it's probably best to squash it. :-\ Phil ``` ## Jeff King, 2012-10-29 21:36 Subject: Re: [PATCH v2] git-submodule add: Add -r/--record option. Message-ID: <20121029213652.GC20513@sigill.intra.peff.net> URL: https://gitlist.dev/e/20121029213652.GC20513%40sigill.intra.peff.net In-Reply-To: ``` On Mon, Oct 29, 2012 at 01:38:28PM -0400, Phil Hord wrote: > > I am not sure it is sufficient as-is, though. It does not seem to ever > > clear variables, only set them, which means that values could leak > > across iterations of the loop, [...] E.g., when > > the first submodule has submodule.*.foo set but the second one does not, > > you will still end up with $submodule_foo set when you process the > > second one. > > Good point. That should not happen. > > > or down to recursive calls. > > Frankly, I consider that to be a feature. However, I can see how it > would be considered inconsistent in many ways, so it's probably best > to squash it. :-\ I think it would depend on the semantics of the option. Some options would probably make sense to apply recursively, and some not. Maybe instead of blindly converting config into the environment, it should forward or clear specific known-meaning config. -Peff ``` ## Phil Hord, 2012-10-29 22:21 Subject: Re: [PATCH v2] git-submodule add: Add -r/--record option. Message-ID: URL: https://gitlist.dev/e/CABURp0otR2S1aOAWwnaFYFGRi_2cCBODbghck-BqUTw2B_ci3A%40mail.gmail.com In-Reply-To: <20121029213652.GC20513@sigill.intra.peff.net> ``` On Mon, Oct 29, 2012 at 5:36 PM, Jeff King wrote: > On Mon, Oct 29, 2012 at 01:38:28PM -0400, Phil Hord wrote: > >> > I am not sure it is sufficient as-is, though. It does not seem to ever >> > clear variables, only set them, which means that values could leak >> > across iterations of the loop, [...] E.g., when >> > the first submodule has submodule.*.foo set but the second one does not, >> > you will still end up with $submodule_foo set when you process the >> > second one. >> >> Good point. That should not happen. >> >> > or down to recursive calls. >> >> Frankly, I consider that to be a feature. However, I can see how it >> would be considered inconsistent in many ways, so it's probably best >> to squash it. :-\ > > I think it would depend on the semantics of the option. Some options > would probably make sense to apply recursively, and some not. > > Maybe instead of blindly converting config into the environment, it > should forward or clear specific known-meaning config. Well, that's where we started. I was aiming for the more generic "never needs updating" direction. P ``` ## Jeff King, 2012-10-29 22:27 Subject: Re: [PATCH v2] git-submodule add: Add -r/--record option. Message-ID: <20121029222759.GI20513@sigill.intra.peff.net> URL: https://gitlist.dev/e/20121029222759.GI20513%40sigill.intra.peff.net In-Reply-To: ``` On Mon, Oct 29, 2012 at 06:21:08PM -0400, Phil Hord wrote: > > Maybe instead of blindly converting config into the environment, it > > should forward or clear specific known-meaning config. > > Well, that's where we started. I was aiming for the more generic > "never needs updating" direction. Then I think you are probably stuck taking the conservative approach of not propagating recursively. -Peff ```