* On merging strategies, fast forward and index merge
@ 2006-03-18 10:17 Mark Wooding
2006-03-18 10:19 ` [PATCH] git-merge: New options `--no-fast-forward' and `--direct' Mark Wooding
0 siblings, 1 reply; 4+ messages in thread
From: Mark Wooding @ 2006-03-18 10:17 UTC (permalink / raw
To: git
I recently read Junio's description of how to dig oneself out of a hole
using `git merge -s ours' (I'm learning to use the space...), and I've
realised there's a problem here.
The `ours' merge strategy is meant to create a merge commit whose tree
is in every way identical to that of the starting commit. But `git
merge' won't always do this, because it doesn't always invoke the
strategy program.
Consider the command `git merge -s ours MESSAGE MASTER FAILED'.
* If we've not actually messed with our MASTER since the FAILED branch
departed, then MASTER is actually an ancestor of FAILED, and `git
merge' will unhelpfully fast-forward us to the tip of the FAILED
branch. Instead of leaving the merge result like MASTER, it's made
it entirely the wrong thing!
* If both MASTER and FAILED have made changes, but to different files,
then `git merge' will try an index-level merge, find that it
succeeds, and leave us with a mixture of MASTER and FAILED files.
Which is (in this case) entirely what we didn't want.
Additionally, it occurs to me that the fast-forwarding behaviour isn't
always what I want anyway. Consider a merge of a topic branch:
`git merge MESSAGE MASTER TOPIC'
If I allow fast-forward, I lose information about where the topic
started and ended. This is a shame, particularly if I find other places
I want to apply those changes (either as a string of similar commits, or
squidged into a single one) onto other branches.
Because code speaks louder, I'll follow-up this article with a suggested
patch.
-- [mdw]
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] git-merge: New options `--no-fast-forward' and `--direct'.
2006-03-18 10:17 On merging strategies, fast forward and index merge Mark Wooding
@ 2006-03-18 10:19 ` Mark Wooding
2006-03-18 21:59 ` Junio C Hamano
0 siblings, 1 reply; 4+ messages in thread
From: Mark Wooding @ 2006-03-18 10:19 UTC (permalink / raw
To: git
From: Mark Wooding <mdw@distorted.org.uk>
These options disable some of git-merge's optimizations.
--no-fast-forward
Does what it says on the tin: git-merge will always make a
commit as a result of this merge (or leave one in the pipeline,
if --no-commit was given).
--direct
Don't do anything clever: go directly to the merge strategy
programs. In particular, this forbids an attempt at in-index
merging.
We also force direct merging with the `ours' strategy, since this is
obviously what was wanted.
Signed-off-by: Mark Wooding <mdw@distorted.org.uk>
---
Documentation/merge-options.txt | 9 ++++++++-
git-merge.sh | 28 ++++++++++++++++++++++------
git-pull.sh | 13 +++++++++++--
3 files changed, 41 insertions(+), 9 deletions(-)
diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
index 53cc355..5b145a1 100644
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -6,7 +6,6 @@
not autocommit, to give the user a chance to inspect and
further tweak the merge result before committing.
-
-s <strategy>, \--strategy=<strategy>::
Use the given merge strategy; can be supplied more than
once to specify them in the order they should be tried.
@@ -14,3 +13,11 @@
is used instead (`git-merge-recursive` when merging a single
head, `git-merge-octopus` otherwise).
+--no-ff, \--no-fast-forward::
+ Don't fast-forward, even when it looks possible. There will
+ always be a commit to do at the end of the merge.
+
+--direct::
+ Don't do anything clever: go directly to the merge strategy
+ programs. In particular, this forbids an attempt at in-index
+ merging.
diff --git a/git-merge.sh b/git-merge.sh
index cc0952a..d6a579f 100755
--- a/git-merge.sh
+++ b/git-merge.sh
@@ -13,6 +13,8 @@ LF='
all_strategies='recursive octopus resolve stupid ours'
default_strategies='recursive'
use_strategies=
+ff=t
+index_merge=t
if test "@@NO_PYTHON@@"; then
all_strategies='resolve octopus stupid ours'
default_strategies='resolve'
@@ -65,6 +67,12 @@ do
no_summary=t ;;
--no-c|--no-co|--no-com|--no-comm|--no-commi|--no-commit)
no_commit=t ;;
+ --no-f|--no-ff|--no-fa|--no-fas|--no-fast|--no-fast-|--no-fast-f|\
+ --no-fast-fo|--no-fast-for|--no-fast-forw|--no-fast-forwa|\
+ --no-fast-forwar|--no-fast-forward)
+ ff=f ;;
+ --d|--di|--dir|--dire|--direc|--direct)
+ ff=f index_merge=f ;;
-s=*|--s=*|--st=*|--str=*|--stra=*|--strat=*|--strate=*|\
--strateg=*|--strategy=*|\
-s|--s|--st|--str|--stra|--strat|--strate|--strateg|--strategy)
@@ -90,6 +98,10 @@ do
shift
done
+# `ours' is a funny strategy and clever merging optimizations here make
+# it not work.
+case " $use_strategies " in *" ours "*) ff=f index_merge=f ;; esac
+
test "$#" -le 2 && usage ;# we need at least two heads.
merge_msg="$1"
@@ -118,18 +130,18 @@ case "$#" in
esac
echo "$head" >"$GIT_DIR/ORIG_HEAD"
-case "$#,$common,$no_commit" in
-*,'',*)
+case "$#,$ff,$index_merge,$common,$no_commit" in
+*,*,*,'',*)
# No common ancestors found. We need a real merge.
;;
-1,"$1",*)
+1,*,*,"$1",*)
# If head can reach all the merge then we are up to date.
# but first the most common case of merging one remote
echo "Already up-to-date."
dropsave
exit 0
;;
-1,"$head",*)
+1,t,*,"$head",*)
# Again the most common case of merging one remote.
echo "Updating from $head to $1"
git-update-index --refresh 2>/dev/null
@@ -139,11 +151,11 @@ case "$#,$common,$no_commit" in
dropsave
exit 0
;;
-1,?*"$LF"?*,*)
+1,*,*,?*"$LF"?*,*)
# We are not doing octopus and not fast forward. Need a
# real merge.
;;
-1,*,)
+1,*,t,*,)
# We are not doing octopus, not fast forward, and have only
# one common. See if it is really trivial.
git var GIT_COMMITTER_IDENT >/dev/null || exit
@@ -164,6 +176,10 @@ case "$#,$common,$no_commit" in
fi
echo "Nope."
;;
+1,*,*,*,)
+ # Only a single remote, but we've been told not to try anything
+ # clever. Skip to real merge.
+ ;;
*)
# An octopus. If we can reach all the remote we are up to date.
up_to_date=t
diff --git a/git-pull.sh b/git-pull.sh
index 17fda26..229cec7 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -8,7 +8,7 @@ USAGE='[-n | --no-summary] [--no-commit]
LONG_USAGE='Fetch one or more remote refs and merge it/them into the current HEAD.'
. git-sh-setup
-strategy_args= no_summary= no_commit=
+strategy_args= no_summary= no_commit= noff= direct=
while case "$#,$1" in 0) break ;; *,-*) ;; *) break ;; esac
do
case "$1" in
@@ -17,6 +17,12 @@ do
no_summary=-n ;;
--no-c|--no-co|--no-com|--no-comm|--no-commi|--no-commit)
no_commit=--no-commit ;;
+ --no-f|--no-ff|--no-fa|--no-fas|--no-fast|--no-fast-|--no-fast-f|\
+ --no-fast-fo|--no-fast-for|--no-fast-forw|--no-fast-forwa|\
+ --no-fast-forwar|--no-fast-forward)
+ noff=--no-fast-forward ;;
+ --d|--di|--dir|--dire|--direc|--direct)
+ direct=--direct ;;
-s=*|--s=*|--st=*|--str=*|--stra=*|--strat=*|--strate=*|\
--strateg=*|--strategy=*|\
-s|--s|--st|--str|--stra|--strat|--strate|--strateg|--strategy)
@@ -92,4 +98,7 @@ case "$strategy_args" in
esac
merge_name=$(git-fmt-merge-msg <"$GIT_DIR/FETCH_HEAD")
-git-merge $no_summary $no_commit $strategy_args "$merge_name" HEAD $merge_head
+git-merge \
+ $no_summary $no_commit $noff $direct \
+ $strategy_args \
+ "$merge_name" HEAD $merge_head
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] git-merge: New options `--no-fast-forward' and `--direct'.
2006-03-18 10:19 ` [PATCH] git-merge: New options `--no-fast-forward' and `--direct' Mark Wooding
@ 2006-03-18 21:59 ` Junio C Hamano
2006-03-18 22:53 ` Junio C Hamano
0 siblings, 1 reply; 4+ messages in thread
From: Junio C Hamano @ 2006-03-18 21:59 UTC (permalink / raw
To: Mark Wooding; +Cc: git
Mark Wooding <mdw@distorted.org.uk> writes:
> These options disable some of git-merge's optimizations.
While there is no question about the part of the proposed change
to bypass "trivial merge", I do not necessarily agree with
"skipping fast forward" part.
It is a problem that "ours" strategy cannot be used as a way to
recover from the "accidentally rewound head" situation, because
it is prevented from running under certain conditions as you
described. But that does not necessarily mean we should make
"ours" to work in these situations.
You accidentally discarded B while somebody picked it up:
o---o
/ \
---o---o---A---o---o---B
^your head
Now you would want to recover. With your patch, it would create
this:
o---o
/ \
---o---o---A---o---o---B
\ \
------------M
^your updated head,
having the same tree as A
But recording A as a parent of M is not necessary. I think what
we want to have as the result is this instead:
o---o
/ \
---o---o---A---o---o---B---M
^your updated head,
having the same tree as A
This is something you cannot do within the current git-merge
framework; it is set up to either just fast forward or make a
multi-parent commit. You would want have a "revert to this
state" [*1*], something like this (assuming you have rewound to
A and currently your index matches A):
$ git reset --soft B
$ git commit -m 'Discard A..B and revert to A'
I did "ours" primarily as a demonstration of a funky thing
people could do with the consolidated driver "git-merge". I did
not have a useful use-case in mind back then, but it turned out
to be the ideal way to recover from "accidentally rewound head"
situation, except that making a merge commit between A and B is
not always the way to recover from it. If we wanted to, we
could have a special purpose command that does "git merge -s
ours" if there will be a new commit, otherwise the above two
commands sequence if it will be a fast forward, but the
"recovering from accidentally rewound head" _is_ really a
special purpose, so I do not know if it is worth it.
In your cover letter, you talked about using --no-fast-forward
to collapse your sole topic branch into your master branch. I
do not think smudging the development history with extra merge
commits for that is justfied either. There is no reason for you
to discard your topic branch heads after you merged them into
master. If they get in the way of your normal workflow, you can
stash them away as tags that you do not usually see in "git
branch" output [*2*].
Also I am already unhappy that git-merge knows about the
specifics of strategies [*3*], e.g. it knows octopus is
currently the only strategy that can do more than two heads.
Your patch gives more strategy specific knowledge to it, but I
do not know how to avoid it.
[Footnotes]
*1* As opposed to "git revert X" which means "revert the effect
of commit X", you would want "revert to the state X".
*2* I keep some of my old topic branch heads under
.git/tags/attic/.
*3* Another thing I am unhappy about is the list of available
strategies. I initially wanted to allow users to write their
own merge strategies and have them on their PATH (not even
necessarily in GIT_EXEC_PATH directory), so that you can do a
git-merge-mdw secretly, keep it in ~mdw/bin and cook it for a
while using yourself as a guinea pig, and then share that with
the community later, _without_ touching git-merge.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] git-merge: New options `--no-fast-forward' and `--direct'.
2006-03-18 21:59 ` Junio C Hamano
@ 2006-03-18 22:53 ` Junio C Hamano
0 siblings, 0 replies; 4+ messages in thread
From: Junio C Hamano @ 2006-03-18 22:53 UTC (permalink / raw
To: Mark Wooding; +Cc: git
>From nobody Mon Sep 17 00:00:00 2001
From: Junio C Hamano <junkio@cox.net>
Date: Sat Mar 18 14:50:53 2006 -0800
Subject: [PATCH] git-merge knows some strategies want to skip trivial merges
Most notably "ours". Also this makes sure we do not record
duplicated parents on the parent list of the resulting commit.
This is based on Mark Wooding's work, but does not change the UI
nor introduce new flags.
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
* How about this instead? It looks larger than it really is
because strategy-defaulting code needed to get moved around.
git-merge.sh | 67 +++++++++++++++++++++++++++++++++++-----------------------
1 files changed, 40 insertions(+), 27 deletions(-)
313093ea6d29bbce5977556645eb5946dbfb211e
diff --git a/git-merge.sh b/git-merge.sh
index cc0952a..78ab422 100755
--- a/git-merge.sh
+++ b/git-merge.sh
@@ -11,11 +11,15 @@ LF='
'
all_strategies='recursive octopus resolve stupid ours'
-default_strategies='recursive'
+default_twohead_strategies='recursive'
+default_octopus_strategies='octopus'
+no_trivial_merge_strategies='ours'
use_strategies=
+
+index_merge=t
if test "@@NO_PYTHON@@"; then
all_strategies='resolve octopus stupid ours'
- default_strategies='resolve'
+ default_twohead_strategies='resolve'
fi
dropsave() {
@@ -90,8 +94,6 @@ do
shift
done
-test "$#" -le 2 && usage ;# we need at least two heads.
-
merge_msg="$1"
shift
head_arg="$1"
@@ -99,6 +101,8 @@ head=$(git-rev-parse --verify "$1"^0) ||
shift
# All the rest are remote heads
+test "$#" = 0 && usage ;# we need at least one remote head.
+
remoteheads=
for remote
do
@@ -108,6 +112,27 @@ do
done
set x $remoteheads ; shift
+case "$use_strategies" in
+'')
+ case "$#" in
+ 1)
+ use_strategies="$default_twohead_strategies" ;;
+ *)
+ use_strategies="$default_octopus_strategies" ;;
+ esac
+ ;;
+esac
+
+for s in $use_strategies
+do
+ case " $s " in
+ *" $no_trivial_merge_strategies "*)
+ index_merge=f
+ break
+ ;;
+ esac
+done
+
case "$#" in
1)
common=$(git-merge-base --all $head "$@")
@@ -118,18 +143,21 @@ case "$#" in
esac
echo "$head" >"$GIT_DIR/ORIG_HEAD"
-case "$#,$common,$no_commit" in
-*,'',*)
+case "$index_merge,$#,$common,$no_commit" in
+f,*)
+ # We've been told not to try anything clever. Skip to real merge.
+ ;;
+?,*,'',*)
# No common ancestors found. We need a real merge.
;;
-1,"$1",*)
+?,1,"$1",*)
# If head can reach all the merge then we are up to date.
- # but first the most common case of merging one remote
+ # but first the most common case of merging one remote.
echo "Already up-to-date."
dropsave
exit 0
;;
-1,"$head",*)
+?,1,"$head",*)
# Again the most common case of merging one remote.
echo "Updating from $head to $1"
git-update-index --refresh 2>/dev/null
@@ -139,11 +167,11 @@ case "$#,$common,$no_commit" in
dropsave
exit 0
;;
-1,?*"$LF"?*,*)
+?,1,?*"$LF"?*,*)
# We are not doing octopus and not fast forward. Need a
# real merge.
;;
-1,*,)
+?,1,*,)
# We are not doing octopus, not fast forward, and have only
# one common. See if it is really trivial.
git var GIT_COMMITTER_IDENT >/dev/null || exit
@@ -188,17 +216,6 @@ esac
# We are going to make a new commit.
git var GIT_COMMITTER_IDENT >/dev/null || exit
-case "$use_strategies" in
-'')
- case "$#" in
- 1)
- use_strategies="$default_strategies" ;;
- *)
- use_strategies=octopus ;;
- esac
- ;;
-esac
-
# At this point, we need a real merge. No matter what strategy
# we use, it would operate on the index, possibly affecting the
# working tree, and when resolved cleanly, have the desired tree
@@ -270,11 +287,7 @@ done
# auto resolved the merge cleanly.
if test '' != "$result_tree"
then
- parents="-p $head"
- for remote
- do
- parents="$parents -p $remote"
- done
+ parents=$(git-show-branch --independent "$head" "$@" | sed -e 's/^/-p /')
result_commit=$(echo "$merge_msg" | git-commit-tree $result_tree $parents) || exit
finish "$result_commit" "Merge $result_commit, made by $wt_strategy."
dropsave
--
1.2.4.g2fc2
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2006-03-18 22:54 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-03-18 10:17 On merging strategies, fast forward and index merge Mark Wooding
2006-03-18 10:19 ` [PATCH] git-merge: New options `--no-fast-forward' and `--direct' Mark Wooding
2006-03-18 21:59 ` Junio C Hamano
2006-03-18 22:53 ` Junio C Hamano
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).