Re: [PATCH] git-submodule: Remove duplicate entries during merge with conflict
Junio C Hamano <gitster@pobox.com> writes:
Show 11 quoted lines
> Nicolas Morey-Chaisemartin <devel-git@morey-chaisemartin.com> writes:
>
>> During a merge with conflict on a submodule, the submodule appears 3
>> times in git ls-files (stage 1,2,3) which causes the submodule to be
>> used 3 times in git submodule update or status command. This patch
>> filters the results of git ls-files and only shows submodule in stage 0
>> or 1 thus removing the duplicates.
>
> That is a wrong thing to do. What if both sides added a submodule at the
> same directory after forked from an ancestor that did not have it? You
> will have only stage #2 and stage #3, no?
It is not very friendly to say a solution is wrong without hinting what the right thing to do, so let's try.
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 a bigger question is, shouldn't some callers _care_ when the superproject is still unmerged? What does it mean to say "submodule update" when your superproject is unmerged, you haven't resolved it so you don't know if you are going to take the commit your branch points at, or the commit their branch points at, or a commit completely different from either? If submodule.$name.update is set to detach and checkout the commit registred at the superproject, which stage do you want to get the commit to update the submodule with? If it is set to "rebase" or "merge", which stage do you take the commit to rebase/merge your history onto?
I think "update" should not touch the submodule repository in such a case. "status" might want to recurse into the submodule repository, but there is no information at the superproject level to compare the submodule status against, so we should at least skip that part for submodule entries that are unmerged in the superproject.
Perhaps something along the lines of (as usual, totally untested, just to illustrate the concept) the attached patch?
The idea is to 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 $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.
Fine points:
- The command called by foreach may want to do whatever it wants to do by
noticing the merged status in the superproject itself, so I made no
change to the function;
- Init is an unlikely thing to do, 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;
- Update should refrain from touching the submodule itself. It may want
to recurse into the submodule of the unmerged one, but people involved
in submodule work should think things through;
- Status does not have anything to report for an unmerged submodule. It
may want to recurse into the submodule of the unmerged one, but people
involved in submodule work should think things through; and
- Sync should be Ok for the same reason as Init.
git-submodule.sh | 29 ++++++++++++++++++++++++++++-
1 files changed, 28 insertions(+), 1 deletions(-)
Show changes to git-submodule.sh +28 −1
diff --git a/git-submodule.sh b/git-submodule.sh
index 3a13397..8ecd311 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -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";
+ }
+ '
}
#
@@ -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)
@@ -767,6 +789,11 @@ cmd_status()
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)
displaypath="$prefix$path"
Nicolas Morey-Chaisemartin
