Skip to content

Commit

Permalink
submodule: process conflicting submodules only once
Browse files Browse the repository at this point in the history
During a merge module_list returns conflicting submodules several times
(stage 1,2,3) which caused the submodules to be used multiple times in
git submodule init, sync, update and status command.

There are 5 callers of module_list; they all read (mode, sha1, stage,
path) tuple, and most of them care only about path.  As a first level
approximation, it should be Ok (in the sense that it does not make things
worse than it currently is) to filter the duplicate paths from module_list
output, but some callers should change their behaviour when the merge in
the superproject still has conflicts.

Notice the higher-stage entries, and emit only one record from
module_list, but while doing so, mark the entry with "U" (not [0-3]) in
the $stage field and null out the SHA-1 part, as the object name for the
lowest stage does not give any useful information to the caller, and this
way any caller that uses the object name would hopefully barf.  Then
update the codepaths for each subcommands this way:

 - "update" should not touch the submodule repository, because we do not
   know what commit should be checked out yet.

 - "status" reports the conflicting submodules as 'U000...000' and does
   not recurse into them (we might later want to make it recurse).

 - The command called by "foreach" may want to do whatever it wants to do
   by noticing the merged status in the superproject itself, so feed the
   path to it from module_list as before, but only once per submodule.

 - "init" and "sync" are unlikely things to do while the superproject is
   still not merged, but as long as a submodule is there in $path, there
   is no point skipping it. It might however want to take the merged
   status of .gitmodules into account, but that is outside of the scope of
   this topic.

Acked-by: Jens Lehmann <Jens.Lehmann@web.de>
Thanks-to: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Nicolas Morey-Chaisemartin <nicolas@morey-chaisemartin.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
Nicolas Morey-Chaisemartin authored and Junio C Hamano committed Mar 31, 2011
1 parent 806e0ab commit 313ee0d
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 9 deletions.
5 changes: 3 additions & 2 deletions Documentation/git-submodule.txt
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,10 @@ status::
currently checked out commit for each submodule, along with the
submodule path and the output of 'git describe' for the
SHA-1. Each SHA-1 will be prefixed with `-` if the submodule is not
initialized and `+` if the currently checked out submodule commit
initialized, `+` if the currently checked out submodule commit
does not match the SHA-1 found in the index of the containing
repository. This command is the default command for 'git submodule'.
repository and `U` if the submodule has merge conflicts.
This command is the default command for 'git submodule'.
+
If '--recursive' is specified, this command will recurse into nested
submodules, and show their status as well.
Expand Down
29 changes: 28 additions & 1 deletion git-submodule.sh
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,24 @@ resolve_relative_url ()
#
module_list()
{
git ls-files --error-unmatch --stage -- "$@" | sane_grep '^160000 '
git ls-files --error-unmatch --stage -- "$@" |
perl -e '
my %unmerged = ();
my ($null_sha1) = ("0" x 40);
while (<STDIN>) {
chomp;
my ($mode, $sha1, $stage, $path) =
/^([0-7]+) ([0-9a-f]{40}) ([0-3])\t(.*)$/;
next unless $mode eq "160000";
if ($stage ne "0") {
if (!$unmerged{$path}++) {
print "$mode $null_sha1 U\t$path\n";
}
next;
}
print "$_\n";
}
'
}

#
Expand Down Expand Up @@ -427,6 +444,11 @@ cmd_update()
module_list "$@" |
while read mode sha1 stage path
do
if test "$stage" = U
then
echo >&2 "Skipping unmerged submodule $path"
continue
fi
name=$(module_name "$path") || exit
url=$(git config submodule."$name".url)
update_module=$(git config submodule."$name".update)
Expand Down Expand Up @@ -770,6 +792,11 @@ cmd_status()
name=$(module_name "$path") || exit
url=$(git config submodule."$name".url)
displaypath="$prefix$path"
if test "$stage" = U
then
say "U$sha1 $displaypath"
continue
fi
if test -z "$url" || ! test -d "$path"/.git -o -f "$path"/.git
then
say "-$sha1 $displaypath"
Expand Down
58 changes: 52 additions & 6 deletions t/t7405-submodule-merge.sh
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,11 @@ test_expect_success setup '

# History setup
#
# b
# / \
# a d
# \ /
# c
# b
# / \
# init -- a d
# \ \ /
# g c
#
# a in the main repository records to sub-a in the submodule and
# analogous b and c. d should be automatically found by merging c into
Expand All @@ -76,6 +76,8 @@ test_expect_success 'setup for merge search' '
git add file-a &&
git commit -m "sub-a" &&
git branch sub-a) &&
git commit --allow-empty -m init &&
git branch init &&
git add sub &&
git commit -m "a" &&
git branch a &&
Expand All @@ -101,7 +103,13 @@ test_expect_success 'setup for merge search' '
git checkout -b sub-d sub-b &&
git merge sub-c) &&
git commit -a -m "d" &&
git branch test b)
git branch test b &&
git checkout -b g init &&
(cd sub &&
git checkout -b sub-g sub-c) &&
git add sub &&
git commit -a -m "g")
'

test_expect_success 'merge with one side as a fast-forward of the other' '
Expand Down Expand Up @@ -176,6 +184,44 @@ test_expect_success 'merging should fail for changes that are backwards' '
test_must_fail git merge f)
'


# Check that the conflicting submodule is detected when it is
# in the common ancestor. status should be 'U00...00"
test_expect_success 'git submodule status should display the merge conflict properly with merge base' '
(cd merge-search &&
cat >.gitmodules <<EOF &&
[submodule "sub"]
path = sub
url = $TRASH_DIRECTORY/sub
EOF
cat >expect <<EOF &&
U0000000000000000000000000000000000000000 sub
EOF
git submodule status > actual &&
test_cmp expect actual &&
git reset --hard)
'

# Check that the conflicting submodule is detected when it is
# not in the common ancestor. status should be 'U00...00"
test_expect_success 'git submodule status should display the merge conflict properly without merge-base' '
(cd merge-search &&
git checkout -b test-no-merge-base g &&
test_must_fail git merge b &&
cat >.gitmodules <<EOF &&
[submodule "sub"]
path = sub
url = $TRASH_DIRECTORY/sub
EOF
cat >expect <<EOF &&
U0000000000000000000000000000000000000000 sub
EOF
git submodule status > actual &&
test_cmp expect actual &&
git reset --hard)
'


test_expect_success 'merging with a modify/modify conflict between merge bases' '
git reset --hard HEAD &&
git checkout -b test2 c &&
Expand Down

0 comments on commit 313ee0d

Please sign in to comment.