Stefan Beller [off-list ref] writes:
Gerrit has a "superproject subscription" feature[1], that triggers a
commit in a superproject that is subscribed to its submodules.
Conceptually this Gerrit feature can be done on the client side with
Git via:
git -C <superproject> submodule update --remote --force
git -C <superproject> commit -a -m "Update submodules"
git -C <superproject> push
for each branch in the superproject.
Which I imagine would be useful if you only have one submodule. If
you work on submodules A and B and then want to give the updated
superproject that binds the latest in both A and B as an atomic
update, the three lines above would not quite work, no?
To ease the configuration in Gerrit
a special value of "." has been introduced for the submodule.<name>.branch
to mean the same branch as the superproject[2], such that you can create a
new branch on both superproject and the submodule and this feature
continues to work on that new branch.
Now we have find projects in the wild with such a .gitmodules file.
That's annoying.
To have Git working well with these, we imitate the behavior and
look up the superprojects branch name if the submodules branch is
configured to ".". In projects that do not use Gerrit, this value
whould be never configured as "." is not a valid branch name.
I find that the last sentence is somewhat misleading. I agree it is
justifiable that using "." as the name to trigger a new (to us)
feature is safe, because such a setting wouldn't have meant anything
useful without this change, but I initially misread it and thought
you added "are we using Gerrit? Error out if we are not" logic,
which is not the case here.
quoted hunk
diff --git a/git-submodule.sh b/git-submodule.sh
index 4ec7546..1eb33ad 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -590,7 +590,6 @@ cmd_update()
name=$(git submodule--helper name "$sm_path") || exit
url=$(git config submodule."$name".url)
- branch=$(get_submodule_config "$name" branch master)
if ! test -z "$update"
then
update_module=$update
@@ -616,6 +615,14 @@ cmd_update()
if test -n "$remote"
then
+ branch=$(get_submodule_config "$name" branch master)
+ if test "$branch" = "."
+ then
+ if ! branch=$(git symbolic-ref --short -q HEAD)
+ then
+ die "$(eval_gettext "submodule branch configured to inherit branch from superproject, but it's not on any branch")"
+ fi
+ fi
I see that you narrowed the scope of "$branch" (which is only used
when $remote exists), but it is a bit annoying to see that change
mixed with "now a dot means something different" change.
I wonder if the above 8-line block wants to be encapsulated to
become a part of "git submodule--helper" interface, though. IOW,
branch=$(git submodule--helper branch "$name") or something?
+test_expect_success 'submodule update --remote should fetch upstream changes with .' '
+ (cd super &&
+ git config -f .gitmodules submodule."submodule".branch "." &&
+ git add .gitmodules &&
+ git commit -m "submodules: update from the respective superproject branch"
+ ) &&
(cd submodule &&
+ echo line4a >> file &&
+ git add file &&
+ test_tick &&
+ git commit -m "upstream line4a" &&
+ git checkout -b test-branch &&
+ test_commit on-test-branch
+ ) &&
+ (cd super &&
+ git submodule update --remote --force submodule &&
+ (cd submodule &&
+ test "$(git log -1 --oneline)" = "$(GIT_DIR=../../submodule/.git git log -1 --oneline master)"
A few issues:
* A crash in "git log" would not be noticed with this. Perhaps
git log -1 --oneline $one_way_to_invoke >expect &&
git log -1 --oneline $the_other_way >actual &&
test_cmp expect actual
would be better?
* What exactly is this testing? The current branch (in submodule)
pointing at the same commit as the tip of 'master'? Or the
current branch _is_ 'master'?
* What exactly is the reason why one has GIT_DIR=... and the other
does not? I do not think this a place to test that "gitdir: "
in .git points at the right place, so it must be testing
something else, but I cannot guess.
+ ) &&
git checkout -b test-branch &&
+ git submodule update --remote --force submodule &&
+ (cd submodule &&
+ test "$(git log -1 --oneline)" = "$(GIT_DIR=../../submodule/.git git log -1 --oneline test-branch)"
+ ) &&
+ git checkout master &&
+ git branch -d test-branch
+ )
+'
+
+test_expect_success 'branch = . does not confuse the rest of update' '
+ (cd super &&
+ git checkout --detach &&
+ # update is not confused by branch="." even if the the superproject
+ # is not on any branch currently
+ git submodule update &&
+ git revert HEAD &&
"revert" is rather unusual thing to see in the test. Also I am not
sure why cmd_update that now has an explicit check to die when
branch is set to "." and the head is detached is expected "not" to
be confused. Perhaps I misread the main part of the patch?
Puzzled.
+ git checkout master
+ )
+'
+
+test_expect_success 'local config should override .gitmodules branch' '
+ (cd submodule &&
+ git checkout test-branch &&
echo line5 >> file &&
git add file &&
test_tick &&
On Thu, Jul 28, 2016 at 12:10 PM, Junio C Hamano [off-list ref] wrote:
Stefan Beller [off-list ref] writes:
quoted
Gerrit has a "superproject subscription" feature[1], that triggers a
commit in a superproject that is subscribed to its submodules.
Conceptually this Gerrit feature can be done on the client side with
Git via:
git -C <superproject> submodule update --remote --force
git -C <superproject> commit -a -m "Update submodules"
git -C <superproject> push
for each branch in the superproject.
Which I imagine would be useful if you only have one submodule. If
you work on submodules A and B and then want to give the updated
superproject that binds the latest in both A and B as an atomic
update, the three lines above would not quite work, no?
When using Gerrit you can submit A,B together bound by a "topic"
and that will trigger a superproject update that contains one
atomic commit updating A and B at the same time.
To fully emulate that Gerrit feature you would need to put
the 3 lines above in an infinite loop that polls the remote
submodules all the time.
If you wanted to do that without that Gerrit feature,
this becomes racy as "submodule update --remote"
fetches the submodules racily (by design) and it may end
up putting A and B in the same commit or not.
quoted
To ease the configuration in Gerrit
a special value of "." has been introduced for the submodule.<name>.branch
to mean the same branch as the superproject[2], such that you can create a
new branch on both superproject and the submodule and this feature
continues to work on that new branch.
Now we have find projects in the wild with such a .gitmodules file.
I'll fix the typo. (delete have or find)
That's annoying.
quoted
To have Git working well with these, we imitate the behavior and
look up the superprojects branch name if the submodules branch is
configured to ".". In projects that do not use Gerrit, this value
whould be never configured as "." is not a valid branch name.
I find that the last sentence is somewhat misleading. I agree it is
justifiable that using "." as the name to trigger a new (to us)
feature is safe, because such a setting wouldn't have meant anything
useful without this change, but I initially misread it and thought
you added "are we using Gerrit? Error out if we are not" logic,
which is not the case here.
Well I wanted to express:
The .gitmodules used in these Gerrit projects do not conform
to Gits understanding of how .gitmodules should look like.
Let's make Git understand this Gerrit corner case (which you
would only need when using Gerrit).
Adding special treatment of the "." value is safe as this is an
invalid branch name in Git.
quoted
diff --git a/git-submodule.sh b/git-submodule.sh
index 4ec7546..1eb33ad 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -590,7 +590,6 @@ cmd_update()
name=$(git submodule--helper name "$sm_path") || exit
url=$(git config submodule."$name".url)
- branch=$(get_submodule_config "$name" branch master)
if ! test -z "$update"
then
update_module=$update@@ -616,6 +615,14 @@ cmd_update()
if test -n "$remote"
then
+ branch=$(get_submodule_config "$name" branch master)
+ if test "$branch" = "."
+ then
+ if ! branch=$(git symbolic-ref --short -q HEAD)
+ then
+ die "$(eval_gettext "submodule branch configured to inherit branch from superproject, but it's not on any branch")"
+ fi
+ fi
I see that you narrowed the scope of "$branch" (which is only used
when $remote exists), but it is a bit annoying to see that change
mixed with "now a dot means something different" change.
I wonder if the above 8-line block wants to be encapsulated to
become a part of "git submodule--helper" interface, though. IOW,
branch=$(git submodule--helper branch "$name") or something?
There is already get_submodule_config that we implement in shell,
which is also used in cmd_summary for reading the .ignore
field.
So having the "." check in that method (whether in shell or in C)
doesn't make sense to me.
We could of course have our own method specific for branches,
that take the "." into account. I think we can go with that.
quoted
+test_expect_success 'submodule update --remote should fetch upstream changes with .' '
+ (cd super &&
+ git config -f .gitmodules submodule."submodule".branch "." &&
+ git add .gitmodules &&
+ git commit -m "submodules: update from the respective superproject branch"
+ ) &&
(cd submodule &&
+ echo line4a >> file &&
+ git add file &&
+ test_tick &&
+ git commit -m "upstream line4a" &&
+ git checkout -b test-branch &&
+ test_commit on-test-branch
+ ) &&
+ (cd super &&
+ git submodule update --remote --force submodule &&
+ (cd submodule &&
+ test "$(git log -1 --oneline)" = "$(GIT_DIR=../../submodule/.git git log -1 --oneline master)"
A few issues:
* A crash in "git log" would not be noticed with this. Perhaps
git log -1 --oneline $one_way_to_invoke >expect &&
git log -1 --oneline $the_other_way >actual &&
test_cmp expect actual
would be better?
Of course. I tried to blend in with the code after looking at the surrounding
code. Maybe I need to modernize that whole test file as a preparatory step.
* What exactly is this testing? The current branch (in submodule)
pointing at the same commit as the tip of 'master'? Or the
current branch _is_ 'master'?
* What exactly is the reason why one has GIT_DIR=... and the other
does not? I do not think this a place to test that "gitdir: "
in .git points at the right place, so it must be testing
something else, but I cannot guess.
It is testing that the local state is at the same commit as the
master branch on the remote side.
(GIT_DIR=../../submodule/.git is saying to be "remote", and "master"
to check that branch. We need to have master here as we configured
"." and have the master branch checked out in the superproject.)
At least that is my understanding from the existing tests for
the "--remote" flag
quoted
+ (cd super &&
+ git checkout --detach &&
+ # update is not confused by branch="." even if the the superproject
+ # is not on any branch currently
+ git submodule update &&
+ git revert HEAD &&
"revert" is rather unusual thing to see in the test.
The tests are so long that I tried to get back in a state that is as least
different from before to not break the following tests.
(And except for the depth this worked well)
Also I am not
sure why cmd_update that now has an explicit check to die when
branch is set to "." and the head is detached is expected "not" to
be confused. Perhaps I misread the main part of the patch?
Well you *only* explicitly die(..) when you ask for --remote.
If the superproject is on a detached HEAD and just wants the
sha1s as recorded (--no-remote), it doesn't care about the recorded
branch value and should *not* die.