Fabian Ruch [off-list ref] writes:
quoted hunk
diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
index d3fb67d..3f754ae 100644
--- a/git-rebase--merge.sh
+++ b/git-rebase--merge.sh
@@ -67,7 +67,13 @@ call_merge () {
GIT_MERGE_VERBOSITY=1 && export GIT_MERGE_VERBOSITY
fi
test -z "$strategy" && strategy=recursive
- eval 'git-merge-$strategy' $strategy_opts '"$cmt^" -- "$hd" "$cmt"'
+ base=$(git rev-list --parents -1 $cmt | cut -d ' ' -s -f 2 -)
+ if test -z "$base"
+ then
+ # the empty tree sha1
+ base=4b825dc642cb6eb9a060e54bf8d69288fbee4904
+ fi
+ eval 'git-merge-$strategy' $strategy_opts '"$base" -- "$hd" "$cmt"'
This looks wrong.
The interface to "git-merge-$strategy" is designed in such a way
that each strategy should be capable of taking _no_ base at all.
See how unquoted $common is given to git-merge-$strategy in
contrib/examples/git-merge.sh, i.e.
eval 'git-merge-$strategy '"$xopt"' $common -- "$head_arg" "$@"'
where common comes from
common=$(git merge-base ...)
which would be empty when you are looking at disjoint histories.
Also rev-list piped to cut is too ugly to live in our codebase X-<.
Wouldn't it be sufficient to do something like this instead?
eval 'git-merge-$strategy' $strategy_opts \
$(git rev-parse --quiet --verify "$cmt^") -- "$hd" "$cmt"
Hi Junio,
On 10/09/2014 09:05 PM, Junio C Hamano wrote:
Fabian Ruch [off-list ref] writes:
quoted
diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
index d3fb67d..3f754ae 100644
--- a/git-rebase--merge.sh
+++ b/git-rebase--merge.sh
@@ -67,7 +67,13 @@ call_merge () {
GIT_MERGE_VERBOSITY=1 && export GIT_MERGE_VERBOSITY
fi
test -z "$strategy" && strategy=recursive
- eval 'git-merge-$strategy' $strategy_opts '"$cmt^" -- "$hd" "$cmt"'
+ base=$(git rev-list --parents -1 $cmt | cut -d ' ' -s -f 2 -)
+ if test -z "$base"
+ then
+ # the empty tree sha1
+ base=4b825dc642cb6eb9a060e54bf8d69288fbee4904
+ fi
+ eval 'git-merge-$strategy' $strategy_opts '"$base" -- "$hd" "$cmt"'
This looks wrong.
Ok.
The interface to "git-merge-$strategy" is designed in such a way
that each strategy should be capable of taking _no_ base at all.
The merge strategies "resolve" and "octopus" seem to refuse to run if no
base is specified. The former silently exits if no bases are given and
the latter dies saying "Unable to find common commit".
See how unquoted $common is given to git-merge-$strategy in
contrib/examples/git-merge.sh, i.e.
eval 'git-merge-$strategy '"$xopt"' $common -- "$head_arg" "$@"'
where common comes from
common=$(git merge-base ...)
which would be empty when you are looking at disjoint histories.
Also rev-list piped to cut is too ugly to live in our codebase X-<.
Is there a better way to get the parents list from a shell script then?
I stole the construct from git-rebase--interactive.sh which uses it to
check for rewritten parents when preserving merges.
Wouldn't it be sufficient to do something like this instead?
eval 'git-merge-$strategy' $strategy_opts \
$(git rev-parse --quiet --verify "$cmt^") -- "$hd" "$cmt"
Yes, for the "recursive" strategies this seems to have the exact same
behaviour as it inserts the empty tree in case git-merge-base returns an
empty list. Nice, we would get rid of both the magic number and the cut.
Regards,
Fabian
Hi,
Junio C Hamano writes:
Fabian Ruch [off-list ref] writes:
quoted
diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
index d3fb67d..3f754ae 100644
--- a/git-rebase--merge.sh
+++ b/git-rebase--merge.sh
@@ -67,7 +67,13 @@ call_merge () {
GIT_MERGE_VERBOSITY=1 && export GIT_MERGE_VERBOSITY
fi
test -z "$strategy" && strategy=recursive
- eval 'git-merge-$strategy' $strategy_opts '"$cmt^" -- "$hd" "$cmt"'
+ base=$(git rev-list --parents -1 $cmt | cut -d ' ' -s -f 2 -)
+ if test -z "$base"
+ then
+ # the empty tree sha1
+ base=4b825dc642cb6eb9a060e54bf8d69288fbee4904
+ fi
+ eval 'git-merge-$strategy' $strategy_opts '"$base" -- "$hd" "$cmt"'
This looks wrong.
The interface to "git-merge-$strategy" is designed in such a way
that each strategy should be capable of taking _no_ base at all.
Ok, but doesn't this use of the git-merge-$strategy interface (as shown
in the example below) apply only to the case where one wants to merge
two histories by creating a merge commit? When a merge commit is being
created, the documentation states that git-merge abstracts from the
commit history considering the _total change_ since a merge base on each
branch.
In contrast, here (i.e., in the case of git-rebase--merge) we care about
how the changes introduced by the _individual commits_ are applied.
Therefore, don't we want to be explicit about the "base" and tell
git-merge-$strategy exactly which changes it should merge into the
current head?
The codebase has always been doing this both for git-rebase--merge and
git-cherry-pick. What leads to the reported bug is that the latter
covers the case where the commit object has no parents but the former
doesn't. Root commits are handled by git-cherry-pick (and should be by
git-rebase--merge) using an explicit "base" for the same reason why
$cmt^ is given.
See how unquoted $common is given to git-merge-$strategy in
contrib/examples/git-merge.sh, i.e.
eval 'git-merge-$strategy '"$xopt"' $common -- "$head_arg" "$@"'
where common comes from
common=$(git merge-base ...)
which would be empty when you are looking at disjoint histories.
If there are still objections to the patch because of the magic number
and the cut, it might be worth considering an implementation of
git-rebase--merge using git-cherry-pick's merge strategy option.
Fabian