# [PATCH] git-submodule add: Record branch name in .gitmodules 12 messages from 2012-10-22 to 2012-10-26. Participants: W. Trevor King, Nahor, Jens Lehmann, Phil Hord, Jeff King. Thread: https://gitlist.dev/t/31906 ## W. Trevor King, 2012-10-22 16:34 Subject: [PATCH] git-submodule add: Record branch name in .gitmodules Message-ID: <61a31f6bc61d4df322a097e32ba472390c583a81.1350923683.git.wking@tremily.us> URL: https://gitlist.dev/e/61a31f6bc61d4df322a097e32ba472390c583a81.1350923683.git.wking%40tremily.us ``` From: "W. Trevor King" This removes a configuration step if you're trying to setup Ævar's $ git submodule foreach 'git checkout $(git config --file $toplevel/.gitmodules submodule.$name.branch) && git pull' workflow from commit f030c96d8643fa0a1a9b2bd9c2f36a77721fb61f Author: Ævar Arnfjörð Bjarmason Date: Fri May 21 16:10:10 2010 +0000 git-submodule foreach: Add $toplevel variable If you're not using that workflow, I see no harm in recording the branch used to determine the original submodule commit. Signed-off-by: W. Trevor King --- Documentation/git-submodule.txt | 3 ++- git-submodule.sh | 4 ++++ t/t7400-submodule-basic.sh | 1 + 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt index b4683bb..b9f437f 100644 --- a/Documentation/git-submodule.txt +++ b/Documentation/git-submodule.txt @@ -207,7 +207,8 @@ OPTIONS -b:: --branch:: - Branch of repository to add as submodule. + Branch of repository to add as submodule. The branch name is + recorded in .gitmodules for future reference. -f:: --force:: diff --git a/git-submodule.sh b/git-submodule.sh index ab6b110..fd15a54 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -366,6 +366,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 "$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..5031e2a 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 "$(git config -f .gitmodules submodule.submod-branch.branch)" = initial && git submodule init ) && -- 1.8.0.2.g09b91ca ``` ## W. Trevor King, 2012-10-22 22:55 Subject: Re: [PATCH] git-submodule add: Record branch name in .gitmodules Message-ID: <20121022225546.GO23101@odin.tremily.us> URL: https://gitlist.dev/e/20121022225546.GO23101%40odin.tremily.us In-Reply-To: ``` 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. For those to lazy to hunt down a reference, Gerrit uses submodule.$name.branch to “indicate the branch of a submodule project that when updated will trigger automatic update of its registered gitlink.” [1] They also have some extra sauce: “The branch value could be '.' if the submodule project branch has the same name as the destination branch of the commit having gitlinks/.gitmodules file. … Any git submodules which are added and not have the branch field available in the .gitmodules file will not be subscribed by Gerrit to automatically update the superproject.” > 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. > > git submodule add --branch=master --record-branch=master foo bar Ugh, I don't want to retype the branch name whenever I do this. How about -r/--record, with the recorded name being optional? --record-branch[=] 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. A more verbose, but less dangerous, option would be a boolean -r/--record that enables the recording of whatever was passed to -b/--branch. This looses the flexibility of running something like git submodule add --branch=master --record-branch=. foo bar but the Gerrit folks have gotten along OK without any branch recording so far ;). They can keep setting '.' the same way they always have. On a tangentially related note, it would be nice to set environment variables for each of the settings in submodule.$name during a foreach call. Then you could use git submodule foreach 'git checkout $branch && git pull' Perhaps you'd want to blacklist/whitelist or downcase settings names to avoid things like [submodule "foo"] PATH = /my/rootkit/ but the update line is much cleaner. Cheers, Trevor [1]: https://gerrit.googlesource.com/gerrit/+/master/Documentation/user-submodules.txt -- 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 ``` ## Nahor, 2012-10-23 19:16 Subject: Re: [PATCH] git-submodule add: Record branch name in .gitmodules Message-ID: <5086ED06.5020406@gmail.com> URL: https://gitlist.dev/e/5086ED06.5020406%40gmail.com In-Reply-To: <61a31f6bc61d4df322a097e32ba472390c583a81.1350923683.git.wking@tremily.us> ``` On 2012-10-22 09:34, W. Trevor King wrote: > From: "W. Trevor King" > > This removes a configuration step if you're trying to setup Ævar's > > $ git submodule foreach 'git checkout $(git config --file $toplevel/.gitmodules submodule.$name.branch) && git pull' > > workflow from > > commit f030c96d8643fa0a1a9b2bd9c2f36a77721fb61f > Author: Ævar Arnfjörð Bjarmason > Date: Fri May 21 16:10:10 2010 +0000 > > git-submodule foreach: Add $toplevel variable > > If you're not using that workflow, I see no harm in recording the > branch used to determine the original submodule commit. IMHO, the problem is that this works only for a very specific workflow. Normal git usage can very easily break this feature. 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. A similar issue arises if branch A is deleted from the module later (for instance because it has been merged in the master branch and is not useful anymore). Then .gitmodule points to a non-existant branch. Same thing if a branch is renamed. Last issue, the branch that exists in your local repository may not exist in someone else's repository, either because the branch is purely local, or because it has a different name on the remote repo. I think a better place to store that kind of information is using git-notes. That way, if the branch is renamed or deleted, you can easily update the old notes to use the new name instead. Nahor ``` ## W. Trevor King, 2012-10-23 19:44 Subject: Re: [PATCH] git-submodule add: Record branch name in .gitmodules Message-ID: <20121023194436.GD28592@odin.tremily.us> URL: https://gitlist.dev/e/20121023194436.GD28592%40odin.tremily.us In-Reply-To: <5086ED06.5020406@gmail.com> ``` 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. Plus, the text from git-pull should clearly display the branch you are pulling, which gives you a second change to notice if something is going wrong. > A similar issue arises if branch A is deleted from the module later (for > instance because it has been merged in the master branch and is not > useful anymore). Then .gitmodule points to a non-existant branch. > Same thing if a branch is renamed. All of these are possible, and all would reqire manual intervention to pick out a new branch to follow. Plus, this patch is not even about the auto-pull application, it's about storing the initially cloned branch name. What you do with that name afterwards is up to you ;). > I think a better place to store that kind of information is using > git-notes. That way, if the branch is renamed or deleted, you can easily > update the old notes to use the new name instead. Interesting. What would you attach the note too? 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 ``` ## Jens Lehmann, 2012-10-23 20:36 Subject: Re: [PATCH] git-submodule add: Record branch name in .gitmodules Message-ID: <5086FFDC.2050700@web.de> URL: https://gitlist.dev/e/5086FFDC.2050700%40web.de In-Reply-To: <5086ED06.5020406@gmail.com> ``` Am 23.10.2012 21:16, schrieb Nahor: > On 2012-10-22 09:34, W. Trevor King wrote: >> From: "W. Trevor King" >> >> This removes a configuration step if you're trying to setup Ævar's >> >> $ git submodule foreach 'git checkout $(git config --file $toplevel/.gitmodules submodule.$name.branch) && git pull' >> >> workflow from >> >> commit f030c96d8643fa0a1a9b2bd9c2f36a77721fb61f >> Author: Ævar Arnfjörð Bjarmason >> Date: Fri May 21 16:10:10 2010 +0000 >> >> git-submodule foreach: Add $toplevel variable >> >> If you're not using that workflow, I see no harm in recording the >> branch used to determine the original submodule commit. Except recording the branch name might raise expectations about what git will do with it. And as far as this patch goes, git won't do anything with it (yet). > IMHO, the problem is that this works only for a very specific workflow. Normal git usage can very easily break this feature. > > 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. > > A similar issue arises if branch A is deleted from the module later (for instance because it has been merged in the master branch and is not useful anymore). Then .gitmodule points to a non-existant branch. > Same thing if a branch is renamed. Those are valid points. The next "git submodule update" will leave the submodule with a detached HEAD, making the branch configuration rather meaningless (except for your "git submodule foreach ..." use case of course). > Last issue, the branch that exists in your local repository may not exist in someone else's repository, either because the branch is purely local, or because it has a different name on the remote repo. You'll always face this kind of problems with commits too when using submodules, so I don't see that as a problem here. > I think a better place to store that kind of information is using git-notes. That way, if the branch is renamed or deleted, you can easily update the old notes to use the new name instead. I can't comment on that, as I have never been using notes so far. But I'd rather see a patch series properly implementing the always-tip mode Ævar mentions in f030c96d86 (and which is requested by some users), especially the interesting parts: What should git record as commit in that case and how are "git status" and "git diff" going to handle submodules which shall follow a specific branch. I assume "git submodule update" is the right point in time to fetch that branch again and check out a newer branch tip if necessary, but should that commit be added to the superproject for that submodule automagically or not? This patch falls short of this, as it does the easy part but not the interesting ones ;-) ``` ## W. Trevor King, 2012-10-23 20:55 Subject: Re: [PATCH] git-submodule add: Record branch name in .gitmodules Message-ID: <20121023205524.GF28592@odin.tremily.us> URL: https://gitlist.dev/e/20121023205524.GF28592%40odin.tremily.us In-Reply-To: <5086FFDC.2050700@web.de> ``` On Tue, Oct 23, 2012 at 10:36:44PM +0200, Jens Lehmann wrote: > Except recording the branch name might raise expectations about what git > will do with it. And as far as this patch goes, git won't do anything > with it (yet). 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. On Tue, Oct 23, 2012 at 10:36:44PM +0200, Jens Lehmann wrote: > But I'd rather see a patch series properly implementing the always-tip > mode Ævar mentions in f030c96d86 (and which is requested by some users), > especially the interesting parts: What should git record as commit in > that case and how are "git status" and "git diff" going to handle > submodules which shall follow a specific branch. I assume "git submodule > update" is the right point in time to fetch that branch again and check > out a newer branch tip if necessary, but should that commit be added to > the superproject for that submodule automagically or not? This patch > falls short of this, as it does the easy part but not the interesting > ones ;-) I agree that I'm not working on always-tip. I'm just making that easier. For people that aren't interested in always-tip submodules (e.g. Gerrit folks), this patch is still useful. It would certainly be possible to build an always-tip implementation on top of submodule.$name.branch (as Ævar's one-liner does), but that would be another patch series. Personally, I think truly updates should be made explicitly, with a hand written commit message about why the updates are occuring. I also think that setting up and running auto-updates should be easy one-liners, not long, complicated ones ;). -- 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 ``` ## Nahor, 2012-10-23 21:45 Subject: Re: [PATCH] git-submodule add: Record branch name in .gitmodules Message-ID: <50871014.9060000@gmail.com> URL: https://gitlist.dev/e/50871014.9060000%40gmail.com In-Reply-To: <20121023194436.GD28592@odin.tremily.us> ``` On 2012-10-23 12:44, 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. From my experience with my colleagues at work, that's not going to happen. People are very good at forgetting to do something ;) > Plus, the text from git-pull should > clearly display the branch you are pulling, which gives you a second > change to notice if something is going wrong. That's assuming that the user knows the branch that should be pulled and that he's paying attention to the output and not just quick-scanning for errors/warnings. Again, from my experience, that's not going to be the case. Plus, there isn't always a human being behind a git-pull, e.g. build bots. >> I think a better place to store that kind of information is using >> git-notes. That way, if the branch is renamed or deleted, you can easily >> update the old notes to use the new name instead. > > Interesting. What would you attach the note too? To the commits in the super-repo where a module branch is selected, i.e.: - where a module was added - where the module changed branch - where a super-branch was merged and there was a conflict on the module's branch name ``` ## Nahor, 2012-10-23 22:02 Subject: Re: [PATCH] git-submodule add: Record branch name in .gitmodules Message-ID: <508713DE.7040801@gmail.com> URL: https://gitlist.dev/e/508713DE.7040801%40gmail.com In-Reply-To: <5086FFDC.2050700@web.de> ``` On 2012-10-23 13:36, Jens Lehmann wrote: > Am 23.10.2012 21:16, schrieb Nahor: >> Last issue, the branch that exists in your local repository may not >> exist in someone else's repository, either because the branch is >> purely local, or because it has a different name on the remote repo. > > You'll always face this kind of problems with commits too when using > submodules, so I don't see that as a problem here. Commits can't change or disappear during "normal" git operation (i.e. without using "git push -f" or "git branch -D"). A commit also has the same id in all the clones repository so there is no issue of a different name between the local and the remote repositories. ``` ## Phil Hord, 2012-10-24 18:12 Subject: Re: [PATCH] git-submodule add: Record branch name in .gitmodules Message-ID: URL: https://gitlist.dev/e/CABURp0rpPqeTJMYXmgMG2CmU%3DSzzvCMeX46rnMVB0EeB_LXPiA%40mail.gmail.com In-Reply-To: <20121022225546.GO23101@odin.tremily.us> ``` On Mon, Oct 22, 2012 at 6:55 PM, W. Trevor King wrote: > On a tangentially related note, it would be nice to set environment > variables for each of the settings in submodule.$name during a foreach > call. Then you could use > > git submodule foreach 'git checkout $branch && git pull' > > Perhaps you'd want to blacklist/whitelist or downcase settings names > to avoid things like > > [submodule "foo"] > PATH = /my/rootkit/ > > but the update line is much cleaner. This is ugly as can be, but it works in my meagre testing. It defines "submodule_path=/my/rootkit/" for the above bit of .gitmodules. That is, it adds definitions for 'submodule_' for each in .gitmodules in submodule.$name., but is lowercased and defanged (everything that's not already [a-z0-9] is replaced with underscore). For example, git submodule foreach 'echo $submodule_url' --- >8 --- diff --git c/git-submodule.sh i/git-submodule.sh index 6dd2338..79b3467 100755 --- c/git-submodule.sh +++ i/git-submodule.sh @@ -416,7 +416,15 @@ cmd_foreach() prefix="$prefix$sm_path/" 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 | sed -e 's/^[^a-z]/_/' -e 's/[^a-z0-9]/_/g') + printf 'submodule_%s=%s;\n' "$VAR_NAME" "'$VAR_VALUE'" + done) cd "$sm_path" && eval "$@" && if test -n "$recursive" ``` ## Jens Lehmann, 2012-10-24 19:10 Subject: Re: [PATCH] git-submodule add: Record branch name in .gitmodules Message-ID: <50883D31.6080705@web.de> URL: https://gitlist.dev/e/50883D31.6080705%40web.de In-Reply-To: <508713DE.7040801@gmail.com> ``` Am 24.10.2012 00:02, schrieb Nahor: > On 2012-10-23 13:36, Jens Lehmann wrote: >> Am 23.10.2012 21:16, schrieb Nahor: >>> Last issue, the branch that exists in your local repository may not >>> exist in someone else's repository, either because the branch is >>> purely local, or because it has a different name on the remote repo. >> >> You'll always face this kind of problems with commits too when using >> submodules, so I don't see that as a problem here. > > Commits can't change or disappear during "normal" git operation (i.e. without using "git push -f" or "git branch -D"). > A commit also has the same id in all the clones repository so there is no issue of a different name between the local and the remote repositories. But if you forget to push a submodule commit it won't exist in someone else's repository and so he won't be able to update the submodule after checking out a commit in the superproject that records that unpushed submodule commit. ``` ## W. Trevor King, 2012-10-25 01:12 Subject: Re: [PATCH] git-submodule add: Record branch name in .gitmodules Message-ID: <20121025011227.GH801@odin.tremily.us> URL: https://gitlist.dev/e/20121025011227.GH801%40odin.tremily.us In-Reply-To: ``` On Wed, Oct 24, 2012 at 02:12:18PM -0400, Phil Hord wrote: > + VAR_NAME=$(printf '%s' > "$VAR_NAME" | tr A-Z a-z | sed -e 's/^[^a-z]/_/' -e 's/[^a-z0-9]/_/g') Is there a reason why you use printf instead of echo? Also, this sort of name cleaning should probably live in a new function: clean_environment_variable() or some such. Is there a git-utility-functions.sh library hiding somewhere in the source? ;) -- 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 13:37 Subject: Re: [PATCH] git-submodule add: Record branch name in .gitmodules Message-ID: <20121026133742.GA10829@sigill.intra.peff.net> URL: https://gitlist.dev/e/20121026133742.GA10829%40sigill.intra.peff.net In-Reply-To: <20121025011227.GH801@odin.tremily.us> ``` On Wed, Oct 24, 2012 at 09:12:27PM -0400, W. Trevor King wrote: > On Wed, Oct 24, 2012 at 02:12:18PM -0400, Phil Hord wrote: > > + VAR_NAME=$(printf '%s' > > "$VAR_NAME" | tr A-Z a-z | sed -e 's/^[^a-z]/_/' -e 's/[^a-z0-9]/_/g') > > Is there a reason why you use printf instead of echo? Sadly, echo is not entirely portable for arbitrary content. We have run into problems with backslashes being interpreted differently (bash requires "echo -e" to interpret them, dash will always do it, and POSIX leaves it vague). > Also, this sort of name cleaning should probably live in a new > function: > > clean_environment_variable() > > or some such. Is there a git-utility-functions.sh library hiding > somewhere in the source? ;) git-sh-setup? -Peff ```