From: Junio C Hamano <hidden> Date: 2016-06-15 22:44:25
Jörg Sommer [off-list ref] writes:
Me too, but I think it's not possible to do what I want with -p. -p
misses a definition of the (new) parent of a commit. It tries to preserve
all commits from all branches. But going through the _list_ of commands
couldn't preserve this structure.
o--A--B
\ \
C--D--M--E
How should the graph look like after these commands:
pick A
pick C
squash E
# pick D
pick B
pick M
I am beginning to suspect that the root cause of this is that the todo
language is not expressive enough to reproduce a merge _and_ allow end
user editing.
Let's step back a bit.
If you have this history:
o---o---o---o---o---Z
/
X---Y---A---B
\ \
C---D---M---E
and you want to transplant the history between X..E on top of Z, from the
command line you would say:
$ git rebase --interactive -p --onto Z X E
First let's think what you would do if you want to do this by hand. The
sequence would be:
$ git checkout Z^0 ;# detach at Z
$ git cherry-pick Y
$ git tag new-Y ;# remember it
$ git cherry-pick A
$ git cherry-pick B
$ git tag new-B ;# remember it
$ git checkout new-Y
$ git cherry-pick C
$ git cherry-pick D
$ git merge new-B ;# this reproduces M
$ git cherry-pick E
$ git branch -f $the_branch && git checkout $the_branch
Now how does the todo file before you edit look like?
pick Y
pick A
pick B
pick C
pick D
pick M
pick E
The todo file expects the initial detaching and the final switching back
outside of its control, so it is Ok that the first "checkout Z^0" and the
last "branch && checkout" do not appear, but it should be able to express
the remainder and let you tweak. Is it expressive enough to do so?
Most of the "pick" from the above list literally translate to the
"cherry-pick", and if you change any of them to "edit", that is
"cherry-pick --no-edit" followed by a return of control to you with insn
to "rebase --continue" to resume. There appears nothing magical.
Not really. There already are two gotchas even without talking about
end-user editing.
First, "pick M" is not "cherry-pick M". You do not want to end up with
merging an old parent before rewriting. It has to be something like
"merge rewritten-Y".
Second, before you start picking C, if you want to preserve merges, you
have to switch to rewritten Y. The original sequence left in todo does
not have that information to begin with. We need, before the "pick C", to
say the equivalent of "git checkout new-Y" in the manual sequence
illustrated above. The lack of "checkout new-Y" is perfectly fine if
rebase is meant to linearlize the history, but if you want to preserve the
shape of the history, you would need to give a clue that the sequence that
begins with the "pick C" starts from somewhere else.
You also need to make sure that "pick M" moved elsewhere still merges the
tips of two forked histories. Moving "pick M" before "pick C" or "pick A"
would not make much sense. So you would need some kind of "barrier" that
says "do not move this 'pick M' beyond this point".
Perhaps we can make it clearer by introducing a few more primitives to the
todo language: mark, reset and merge. The above illustrated history would
then become:
pick Y
mark #0
pick A
pick B
mark #1
reset #0
pick C
pick D
mark #2
merge #1 #2
pick E
You can change any of the "pick" to "edit, or drop it, and you can reorder
"pick" in a sequence of "pick", but you cannot change "mark", "reset",
"merge", or move "pick" across insn that was not originally "pick".
From: Jörg Sommer <hidden> Date: 2016-06-15 22:44:25
Hi Junio,
Junio C Hamano schrieb am Mon 24. Mar, 11:35 (-0700):
If you have this history:
o---o---o---o---o---Z
/
X---Y---A---B
\ \
C---D---M---E
I would like to extend this example:
o---o---o---o---o---Z
/
X---Y---A-------B
\ \
C---D---N---M---E
/
V
and you want to transplant the history between X..E on top of Z, from the
command line you would say:
$ git rebase --interactive -p --onto Z X E
First let's think what you would do if you want to do this by hand. The
sequence would be:
$ git checkout Z^0 ;# detach at Z
$ git cherry-pick Y
$ git tag new-Y ;# remember it
$ git cherry-pick A
$ git cherry-pick B
$ git tag new-B ;# remember it
$ git checkout new-Y
$ git cherry-pick C
$ git cherry-pick D
% git merge V
$ git merge new-B ;# this reproduces M
$ git cherry-pick E
$ git branch -f $the_branch && git checkout $the_branch
Perhaps we can make it clearer by introducing a few more primitives to the
todo language: mark, reset and merge. The above illustrated history would
then become:
pick Y
mark #0
pick A
pick B
mark #1
reset #0
pick C
pick D
merge V
V is not a mark.
mark #2
merge #1 #2
pick E
You can change any of the "pick" to "edit, or drop it, and you can reorder
"pick" in a sequence of "pick", but you cannot change "mark", "reset",
"merge", or move "pick" across insn that was not originally "pick".
This way we can also merge more than two branches. Your idea sounds good.
Bye, Jörg.
--
Nichts ist so langweilig, wie die Wiederholung seinerselbst.
(Marcel Reich‐Ranicki)
From: Junio C Hamano <hidden> Date: 2016-06-15 22:44:25
Jörg Sommer [off-list ref] writes:
I would like to extend this example:
o---o---o---o---o---Z
/
X---Y---A-------B
\ \
C---D---N---M---E
/
V
...
merge V
V is not a mark.
That's essentially the same as what I drew in a follow-up to the message
you are responding to, using --first-parents to mark D as "not subject to
rewrite but still interesting".
As I explained there, a bigger issue is how you would express the set of
commits that you would want to rewrite and use intact. In your
illustration, you would want to rewrite N but you want to reuse V.
"rebase Z E" or "rebase --onto Z X E" would include V (and all of its
ancestors that cannot be reached from Z or X that you did not draw) in the
set to be rewritten. Extending the input to the rebase command to use
revision range syntax and saying it as Z..E (or X..E) would not help
either.
@@ -137,8 +135,6 @@ pick_one () {no_ff=case"$1"in-n)sha1=$2;no_ff=t;;*)sha1=$1;;esacoutputgitrev-parse--verify$sha1||die"Invalid commit name: $sha1"-test-d"$REWRITTEN"&&-pick_one_preserving_merges"$@"&&returnparent_sha1=$(gitrev-parse--verify$sha1^)||die"Could not get the parent of $sha1"current_sha1=$(gitrev-parse--verifyHEAD)
@@ -152,66 +148,6 @@ pick_one () {fi}-pick_one_preserving_merges(){-case"$1"in-n)sha1=$2;;*)sha1=$1;;esac-sha1=$(gitrev-parse$sha1)--iftest-f"$DOTEST"/current-commit-then-current_commit=$(cat"$DOTEST"/current-commit)&&-gitrev-parseHEAD>"$REWRITTEN"/$current_commit&&-rm"$DOTEST"/current-commit||-die"Cannot write current commit's replacement sha1"-fi--# rewrite parents; if none were rewritten, we can fast-forward.-fast_forward=t-preserve=t-new_parents=-forpin$(gitrev-list--parents-1$sha1|cut-d' '-f2-)-do-iftest-f"$REWRITTEN"/$p-then-preserve=f-new_p=$(cat"$REWRITTEN"/$p)-test$p!=$new_p&&fast_forward=f-case"$new_parents"in-*$new_p*)-;;# do nothing; that parent is already there-*)-new_parents="$new_parents$new_p"-;;-esac-fi-done-case$fast_forwardin-t)-outputwarn"Fast forward to $sha1"-test$preserve=f||echo$sha1>"$REWRITTEN"/$sha1-;;-f)-test"a$1"=a-n&&die"Refusing to squash a merge: $sha1"--first_parent=$(expr"$new_parents":' \([^ ]*\)')-# detach HEAD to current parent-outputgitcheckout$first_parent2>/dev/null||-die"Cannot move HEAD to $first_parent"--echo$sha1>"$DOTEST"/current-commit-case"$new_parents"in-' '*' '*)-# No point in merging the first parent, that's HEAD-redo_merge$sha1${new_parents# $first_parent}-;;-*)-outputgitcherry-pick"$@"||-die_with_patch$sha1"Could not pick $sha1"-;;-esac-;;-esac-}- nth_string(){case"$1"in*1[0-9]|*[04-9])echo"$1"th;;
@@ -448,6 +371,138 @@ do_rest () {done}+sha1_to_mark(){+# args: "sha1" " sha1#mark sha1#mark"+localtmp+case"$2"in+*" $1#"*)+tmp="${2#* $1#}"+echo"${tmp%% *}"+;;+*)+return1+;;+esac+}++insert_sha1_with_mark_in_list(){+# args: "sha1" "mark" " sha1#mark sha1#mark"+case"$3"in+*" $1#"*)+echo"$3"+return1+;;+*)+echo"$3$1#$2"+;;+esac+}++create_extended_todo_list(){+(+whileIFS=_readcommitparentssubject+do+iftest"${last_parent:-$commit}"!="$commit"+then+iftestt="${delayed_mark:-f}"+then+case"${marked_commits:-} "in+*" $last_parent ")+;;+*)+marked_commits="${marked_commits:-}$last_parent"+;;+esac+delayed_mark=f+fi+test"$last_parent"=$SHORTUPSTREAM&&\+last_parent=$SHORTONTO+echo"reset $last_parent"+fi+last_parent="${parents%% *}"++case"${marked_commits:-} "in+*" $commit "*)+echomark+;;+esac++case"$parents"in+*' '*)+delayed_mark=t+new_parents=+forpin${parents#* }+do+case"${marked_commits:-} "in+*" $p ")+;;+*)+marked_commits="${marked_commits:-}$p"+;;+esac+iftest"$p"=$SHORTUPSTREAM+then+new_parents="$new_parents$SHORTONTO"+else+new_parents="$new_parents$p"+fi+done+unsetp+echomerge$commit$new_parents+unsetnew_parents+;;+*)+echo"pick $commit$subject"+;;+esac+done+test-n"${last_parent:-}"-a"${last_parent:-}"!=$SHORTUPSTREAM&&\+echoreset$last_parent+)|\+tac|\+whilereadcmdargs+do+:${commit_mark_list:=}${last_commit:=000}+case"$cmd"in+pick)+last_commit="${args%% *}"+;;+mark)+:${next_mark:=0}+ifcommit_mark_list=$(insert_sha1_with_mark_in_list\+$last_commit$next_mark"$commit_mark_list")+then+args="#$next_mark"+next_mark=$(($next_mark+1))+else+die"Internal error: two marks for the same commit"+fi+;;+reset)+iftmp=$(sha1_to_mark$args"$commit_mark_list")+then+args="#$tmp"+fi+;;+merge)+new_args=+foriin${args#* }+do+iftmp=$(sha1_to_mark$i"$commit_mark_list")+then+new_args="$new_args #$tmp"+else+new_args="$new_args$i"+fi+done+last_commit="${args%% *}"+args="$last_commit${new_args# }"+;;+esac+echo"$cmd$args"+done+}+whiletest$#!=0docase"$1"in
@@ -580,33 +635,24 @@ doecho$ONTO>"$DOTEST"/ontotest-z"$STRATEGY"||echo"$STRATEGY">"$DOTEST"/strategytestt="$VERBOSE"&&:>"$DOTEST"/verbose-iftestt="$PRESERVE_MERGES"-then-# $REWRITTEN contains files for each commit that is-# reachable by at least one merge base of $HEAD and-# $UPSTREAM. They are not necessarily rewritten, but-# their children might be.-# This ensures that commits on merged, but otherwise-# unrelated side branches are left alone. (Think "X"-# in the man page's example.)-mkdir"$REWRITTEN"&&-forcin$(gitmerge-base--all$HEAD$UPSTREAM)-do-echo$ONTO>"$REWRITTEN"/$c||-die"Could not init rewritten commits"-done-MERGES_OPTION=-else-MERGES_OPTION=--no-merges-fiSHORTUPSTREAM=$(gitrev-parse--short=7$UPSTREAM)SHORTHEAD=$(gitrev-parse--short=7$HEAD)SHORTONTO=$(gitrev-parse--short=7$ONTO)-gitrev-list$MERGES_OPTION--pretty=oneline--abbrev-commit\---abbrev=7--reverse--left-right--cherry-pick\-$UPSTREAM...$HEAD|\-sed-n"s/^>/pick /p">"$TODO"+common_rev_parse_opts="--abbrev-commit+--abbrev=7--left-right--cherry-pick+$UPSTREAM...$HEAD"+iftestt="$PRESERVE_MERGES"+then+gitrev-list--pretty='format:%h_%p_%s'\+--topo-order$common_rev_parse_opts|\+grep-v^commit|\+create_extended_todo_list+else+gitrev-list--no-merges--reverse--pretty=oneline\+$common_rev_parse_opts|sed-n"s/^>/pick /p"+fi>"$TODO"+cat>>"$TODO"<<EOF# Rebase $SHORTUPSTREAM..$SHORTHEAD onto $SHORTONTO
@@ -181,22 +197,9 @@ pick_one_preserving_merges () {echo$sha1>"$DOTEST"/current-commitcase"$new_parents"in' '*' '*)-# redo merge-author_script=$(get_author_ident_from_commit$sha1)-eval"$author_script"-msg="$(gitcat-filecommit$sha1|sed-e'1,/^$/d')"# No point in merging the first parent, that's HEADnew_parents=${new_parents# $first_parent}-if!GIT_AUTHOR_NAME="$GIT_AUTHOR_NAME"\-GIT_AUTHOR_EMAIL="$GIT_AUTHOR_EMAIL"\-GIT_AUTHOR_DATE="$GIT_AUTHOR_DATE"\-outputgitmerge$STRATEGY-m"$msg"\-$new_parents-then-gitrerere-printf"%s\n""$msg">"$GIT_DIR"/MERGE_MSG-dieErrorredoingmerge$sha1-fi+redo_merge;;*)outputgitcherry-pick"$@"||
From: Jörg Sommer <hidden> Date: 2016-06-15 22:44:28
It's much helpful to see the TODO list generated by rebase in the verbose
output. This makes it easier to check, if the list was not broken by
design.
---
t/t3404-rebase-interactive.sh | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
@@ -352,6 +362,15 @@ do_next () {gitrev-parse--verifyHEAD>"$MARKS"/$mark||\die"HEAD is invalid";;+reset|r)+comment_for_reflogreset++mark_action_done+tmp=$(mark_to_sha1$sha1)||\+tmp=$(gitrev-parse--verify$sha1)||+die"Invalid parent '$sha1' in $command$sha1$rest"+outputgitreset--hard$tmp+;;*)warn"Unknown command: $command$sha1$rest"die_with_patch$sha1"Please fix this in the file $TODO."
@@ -569,6 +588,7 @@ do# edit = use commit, but stop for amending# squash = use commit, but meld into previous commit# mark #NUM = mark the current HEAD for later reference+# reset #NUM|commit = reset HEAD to a previous set mark or a commit## If you remove a line here THAT COMMIT WILL BE LOST.# However, if you remove everything, the rebase will be aborted.
@@ -85,6 +85,9 @@ for line in $FAKE_LINES; domark*)echo"mark ${line#mark}"echo"mark ${line#mark}">>"$1";;+reset*)+echo"reset ${line#reset}"+echo"reset ${line#reset}">>"$1";;*)echosed-n"${line}s/^pick/$action/p"sed-n"${line}p"<"$1".tmp
@@ -210,6 +213,13 @@ test_expect_success 'setting marks works' 'gitrebase--abort'+test_expect_success'reset with nonexistent mark fails''+exportFAKE_LINES="reset#0 1"&&+test_must_failgitrebase-iHEAD~1&&+unsetFAKE_LINES&&+gitrebase--abort+'+ test_expect_success'preserve merges with -p''gitcheckout-bto-be-preservedmaster^&&:>unrelated-file&&
@@ -250,17 +250,19 @@ else# We are invoked directly as the first-class UI.head_arg=HEAD-# All the rest are the commits being merged; prepare-# the standard merge summary message to be appended to-# the given message. If remote is invalid we will die-# later in the common codepath so we discard the error-# in this loop.-merge_name=$(forremote-do-merge_name"$remote"-done|gitfmt-merge-msg-)-merge_msg="${merge_msg:+$merge_msg$LF$LF}$merge_name"+iftest-z"$merge_msg"+then+# All the rest are the commits being merged; prepare+# the standard merge summary message to be appended to+# the given message. If remote is invalid we will die+# later in the common codepath so we discard the error+# in this loop.+merge_msg=$(forremote+do+merge_name"$remote"+done|gitfmt-merge-msg+)+fifihead=$(gitrev-parse--verify"$head_arg"^0)||usage
@@ -198,8 +201,7 @@ pick_one_preserving_merges () {case"$new_parents"in' '*' '*)# No point in merging the first parent, that's HEAD-new_parents=${new_parents# $first_parent}-redo_merge+redo_merge$sha1${new_parents# $first_parent};;*)outputgitcherry-pick"$@"||
From: Jörg Sommer <hidden> Date: 2016-06-15 22:44:28
Junio proposed to add the commands mark, merge and reset to rebase
interactive for a better support of rebase with preserve merges. The
current format can only cope with flat lineare lists of commits and not
with the nonâlinear structure of branches with merges.
This patch series is not intended for inclusion. It's a RFC and meant for
gathering ideas.
Currently it misses:
· commit messages
· documentation of the new commands
· update of the documentation of preserve merges
· tests for the merge command
I've included the patch to change the behaviour of git-merge from Junio,
because I don't know how to use the second syntax of git merge and pass
the merge strategies -s to git merge. Without this patch, a new merge
message gets appended to the commit message of a rebased merge commit.
This patch should not be part of the final commit series.
Open questions:
· Is it possible to get a list of all commits iterating the parents from
the last to the first and going to the child of a node before the
neighbors?
· How to get a symbolic name (branch or tag) for a commit? Not something
like git describe. I want to know if 0123 refers to the tip of a
branch.
Jörg.
From: Jörg Sommer <hidden> Date: 2016-06-15 22:44:28
This command redoes merges. It's useful if you rebase a branch that
contains merges and you want to preserve these merges. You can also use
it to add new merges.
---
git-rebase--interactive.sh | 25 +++++++++++++++++++++++++
t/t3404-rebase-interactive.sh | 3 +++
2 files changed, 28 insertions(+), 0 deletions(-)
@@ -362,6 +362,29 @@ do_next () {gitrev-parse--verifyHEAD>"$MARKS"/$mark||\die"HEAD is invalid";;+merge|m)+comment_for_reflogmerge++if!gitrev-parse--verify$sha1+then+die"Invalid reference merge '$sha1' in $command$sha1$rest"+fi++new_parents=+forpin$rest+do+tmp=$(mark_to_sha1$p)||\+tmp=$(gitrev-parse--verify$p)||+die"Invalid parent '$sha1' in $command$sha1$rest"+new_parents="$new_parents$tmp"+done+new_parents="${new_parents# }"+test-n"$new_parents"||\+die"You forgot to give the parents for the merge"++mark_action_done+redo_merge$sha1$new_parents+;;reset|r)comment_for_reflogreset
@@ -589,6 +612,8 @@ do# squash = use commit, but meld into previous commit# mark #NUM = mark the current HEAD for later reference# reset #NUM|commit = reset HEAD to a previous set mark or a commit+# merge commit-M #NUM|commit-P ... = redo merge commit-M with the+# current HEAD and the parents marked with #NUM or the commit-P## If you remove a line here THAT COMMIT WILL BE LOST.# However, if you remove everything, the rebase will be aborted.
@@ -88,6 +88,9 @@ for line in $FAKE_LINES; doreset*)echo"reset ${line#reset}"echo"reset ${line#reset}">>"$1";;+merge*)+echo"merge ${line#merge}"|tr/' '+echo"merge ${line#merge}"|tr/' '>>"$1";;*)echosed-n"${line}s/^pick/$action/p"sed-n"${line}p"<"$1".tmp
From: Jörg Sommer <hidden> Date: 2016-06-15 22:44:28
This new command can be used to set symbolic marks for an commit while
doing a rebase. This symbolic name can later be used for merges or
resets.
---
git-rebase--interactive.sh | 36 ++++++++++++++++++++++++++++++++++++
t/t3404-rebase-interactive.sh | 21 +++++++++++++++++++++
2 files changed, 57 insertions(+), 0 deletions(-)
@@ -317,6 +335,23 @@ do_next () {die_with_patch$sha1""fi;;+mark|a)+if!mark=$(parse_mark$sha1)+then+warn"Invalid mark given: $command$sha1$rest"+die_with_patch$sha1\+"Please fix this in the file $TODO."+fi+mark_action_done++test-d"$MARKS"||mkdir"$MARKS"++test-e"$MARKS"/$mark&&\+warn"mark $mark already exist; overwriting it"++gitrev-parse--verifyHEAD>"$MARKS"/$mark||\+die"HEAD is invalid"+;;*)warn"Unknown command: $command$sha1$rest"die_with_patch$sha1"Please fix this in the file $TODO."
@@ -533,6 +568,7 @@ do# pick = use commit# edit = use commit, but stop for amending# squash = use commit, but meld into previous commit+# mark #NUM = mark the current HEAD for later reference## If you remove a line here THAT COMMIT WILL BE LOST.# However, if you remove everything, the rebase will be aborted.
@@ -82,6 +82,9 @@ for line in $FAKE_LINES; docase$lineinsquash|edit)action="$line";;+mark*)+echo"mark ${line#mark}"+echo"mark ${line#mark}">>"$1";;*)echosed-n"${line}s/^pick/$action/p"sed-n"${line}p"<"$1".tmp
@@ -189,6 +192,24 @@ test_expect_success '-p handles "no changes" gracefully' 'test$HEAD=$(gitrev-parseHEAD)'+test_expect_success'setting an invalid mark fails''+exportFAKE_LINES="mark12 1"&&\+test_must_failgitrebase-iHEAD~1&&+unsetFAKE_LINES&&+gitrebase--abort+'++test_expect_success'setting marks works''+gitcheckoutmaster&&+FAKE_LINES="mark#0 2 1 mark#42 3 edit 4"gitrebase-iHEAD~4&&+marks_dir=.git/.dotest-merge/marks+test-d$marks_dir&&+test$(ls$marks_dir|wc-l)-eq2&&+test"$(gitrev-parseHEAD~4)"="$(cat$marks_dir/0)"&&+test"$(gitrev-parseHEAD~2)"="$(cat$marks_dir/42)"&&+gitrebase--abort+'+ test_expect_success'preserve merges with -p''gitcheckout-bto-be-preservedmaster^&&:>unrelated-file&&
From: Jörg Sommer <hidden> Date: 2016-06-15 22:44:28
The old fake-editor selects only lines they start with pick, if you give
the number of a line. With the new commands mark, merge and reset it was
not possible to select such lines for the new TODO list. The new
fake-editor selects all kinds of lines, but replaces only the command
“pick” with a different action.
---
t/t3404-rebase-interactive.sh | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
@@ -92,9 +92,8 @@ for line in $FAKE_LINES; doecho"merge ${line#merge}"|tr/' 'echo"merge ${line#merge}"|tr/' '>>"$1";;*)-echosed-n"${line}s/^pick/$action/p"-sed-n"${line}p"<"$1".tmp-sed-n"${line}s/^pick/$action/p"<"$1".tmp>>"$1"+sed-n"${line}{s/^pick/$action/; p;}"<"$1".tmp+sed-n"${line}{s/^pick/$action/; p;}"<"$1".tmp>>"$1"action=pick;;esacdone
From: Jörg Sommer <hidden> Date: 2016-06-15 22:44:28
Hallo Mike,
Mike Ralphson schrieb am Thu 10. Apr, 10:33 (+0100):
On 10/04/2008, Jörg Sommer [off-list ref] wrote:
quoted
This new command can be used to set symbolic marks for an commit while
doing a rebase. This symbolic name can later be used for merges or
resets.
What would be wrong with using the existing tag machinery for this instead?
You may have to deal with conflicts if users named tags 03 or 10. But
Junio suggested to use a ref, too. I think refs/rebase-marks/ is a good
prefix.
Bye, Jörg.
--
Der Hase läuft schneller als der Fuchs,
denn der Hase läuft um sein Leben.
From: Jörg Sommer <hidden> Date: 2016-06-15 22:44:28
With this new option it's possible to narrow the list of commits in the
TODO list to only those commits you get following the first parent of
each merge, i.e. not those from the merged branches.
Signed-off-by: Jörg Sommer <redacted>
---
Documentation/git-rebase.txt | 7 ++++++-
git-rebase--interactive.sh | 15 +++++++++++----
t/t3404-rebase-interactive.sh | 12 ++++++++++++
3 files changed, 29 insertions(+), 5 deletions(-)
@@ -247,6 +247,11 @@ OPTIONS Instead of ignoring merges, try to recreate them. This option only works in interactive mode.+-f, \--first-parent::+ This option implies the option --preserve-merges, but instead of+ showing all commits from the merged branches show only the+ commits and merges following the first parent of each commit.+ include::merge-strategies.txt[] NOTES
@@ -10,8 +10,8 @@# The original idea comes from Eric W. Biederman, in# http://article.gmane.org/gmane.comp.version-control.git/22407-USAGE='(--continue|--abort|--skip|[--preserve-merges][--verbose]-[--onto<branch>]<upstream>[<branch>])'+USAGE='(--continue|--abort|--skip|[--preserve-merges][--first-parent]+[--verbose][--onto<branch>]<upstream>[<branch>])'OPTIONS_SPEC= .git-sh-setup
@@ -565,6 +565,10 @@ do-p|--preserve-merges)PRESERVE_MERGES=t;;+-f|--first-parent)+FIRST_PARENT=t+PRESERVE_MERGES=t+;;-i|--interactive)# yeah, we know;;
@@ -294,6 +294,18 @@ test_expect_success 'rebase with preserve merge forth and back is a noop' 'test"$head"="$(gitrev-parseHEAD)"'+test_expect_success'interactive --first-parent gives a linear list''+head=$(gitrev-parseHEAD)&&+EXPECT_COUNT=6FAKE_LINES="2 1 4 3 6 5"\+gitrebase-i-f--ontodead-endmaster&&+test"$head"!="$(gitrev-parseHEAD)"&&+gitrev-parseHEAD^^2&&+test"$(gitrev-parseHEAD~6)"="$(gitrev-parsedead-end)"&&+EXPECT_COUNT=6FAKE_LINES="2 1 4 3 6 5"\+gitrebase-i-f--ontomasterdead-end&&+test"$head"="$(gitrev-parseHEAD)"+'+ test_expect_success'--continue tries to commit''gitcheckoutto-be-rebased&&test_tick&&
From: Jörg Sommer <hidden> Date: 2016-06-15 22:44:28
With this new option tags set on commits, they are part of a rebase, are
reset to the rebased commits. This way the tags on a branch are kept
across rebases.
Signed-off-by: Jörg Sommer <redacted>
---
Documentation/git-rebase.txt | 6 ++++-
git-rebase--interactive.sh | 42 ++++++++++++++++++++++++++++++++++++++--
t/t3404-rebase-interactive.sh | 10 +++++++++
3 files changed, 54 insertions(+), 4 deletions(-)
@@ -252,6 +253,9 @@ OPTIONS showing all commits from the merged branches show only the commits and merges following the first parent of each commit.+-t, \--preserve-tags::+ If one of the commits has a tag, reset it to the new commit object.+ include::merge-strategies.txt[] NOTES
From: Jörg Sommer <hidden> Date: 2016-06-15 22:44:28
This makes it easier to test for equality of a commit in the TODO list
and one of SHORTUPSTREAM, SHORTHEAD or SHORTONTO.
Signed-off-by: Jörg Sommer <redacted>
---
git-rebase--interactive.sh | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
From: Jörg Sommer <hidden> Date: 2016-06-15 22:44:28
It's much helpful to see the TODO list generated by rebase in the verbose
output of the test. This makes it easier to check, if the list was not
broken from the beginning.
Signed-off-by: Jörg Sommer <redacted>
---
t/t3404-rebase-interactive.sh | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
@@ -250,17 +250,19 @@ else# We are invoked directly as the first-class UI.head_arg=HEAD-# All the rest are the commits being merged; prepare-# the standard merge summary message to be appended to-# the given message. If remote is invalid we will die-# later in the common codepath so we discard the error-# in this loop.-merge_name=$(forremote-do-merge_name"$remote"-done|gitfmt-merge-msg-)-merge_msg="${merge_msg:+$merge_msg$LF$LF}$merge_name"+iftest-z"$merge_msg"+then+# All the rest are the commits being merged; prepare+# the standard merge summary message to be appended to+# the given message. If remote is invalid we will die+# later in the common codepath so we discard the error+# in this loop.+merge_msg=$(forremote+do+merge_name"$remote"+done|gitfmt-merge-msg+)+fifihead=$(gitrev-parse--verify"$head_arg"^0)||usage
From: Jörg Sommer <hidden> Date: 2016-06-15 22:44:28
This command redoes merges. It's useful if you rebase a branch that
contains merges and you want to preserve these merges. You can also use
it to add new merges.
Signed-off-by: Jörg Sommer <redacted>
---
git-rebase--interactive.sh | 24 ++++++++++++++++++++++++
t/t3404-rebase-interactive.sh | 13 +++++++++++++
2 files changed, 37 insertions(+), 0 deletions(-)
@@ -356,6 +356,28 @@ do_next () {gitupdate-ref"$mark"HEAD||die"update-ref failed";;+merge|m)+comment_for_reflogmerge++if!gitrev-parse--verify$sha1>/dev/null+then+die"Invalid reference merge '$sha1' in"+"$command$sha1$rest"+fi++new_parents=+forpin$rest+do+new_parents="$new_parents$(mark_to_ref$p)"+done+new_parents="${new_parents# }"+test-n"$new_parents"||\+die"You forgot to give the parents for the"\+"merge $sha1. Please fix it in $TODO"++mark_action_done+redo_merge$sha1$new_parents+;;reset|r)comment_for_reflogreset
@@ -587,6 +609,8 @@ do# squash = use commit, but meld into previous commit# mark :mark = mark the current HEAD for later reference# reset commit = reset HEAD to the commit+# merge commit-M commit-P ... = redo merge commit-M with the+# current HEAD and the parents commit-P## If you remove a line here THAT COMMIT WILL BE LOST.# However, if you remove everything, the rebase will be aborted.
@@ -88,6 +88,9 @@ for line in $FAKE_LINES; doreset*)echo"reset ${line#reset}"echo"reset ${line#reset}">>"$1";;+merge*)+echo"merge ${line#merge}"|tr/' '+echo"merge ${line#merge}"|tr/' '>>"$1";;*)echosed-n"${line}s/^pick/$action/p"sed-n"${line}p"<"$1".tmp
@@ -223,6 +226,16 @@ test_expect_success 'reset to HEAD is a nop' 'test"$(gitrev-parse--shortHEAD)"="$head"'+test_expect_success'merge redoes merges''+test_tick&&+gitmergedead-end&&+merge=$(gitrev-parseHEAD)&&+gitreset--hardHEAD~1&&+FAKE_LINES="1 merge$merge/dead-end"gitrebase-iHEAD~1&&+test$merge="$(gitrev-parseHEAD)"&&+gitreset--hardHEAD~1+'+ test_expect_success'preserve merges with -p''gitcheckout-bto-be-preservedmaster^&&:>unrelated-file&&
From: Jörg Sommer <hidden> Date: 2016-06-15 22:44:28
The intent of the tag command is to (re)set tags for commits in the TODO
list. This way it's possible to rebase a commit together with its tag.
Signed-off-by: Jörg Sommer <redacted>
---
git-rebase--interactive.sh | 7 +++++++
t/t3404-rebase-interactive.sh | 13 +++++++++++++
2 files changed, 20 insertions(+), 0 deletions(-)
@@ -323,6 +323,12 @@ do_next () {mark_action_doneoutputgitreset--hard$tmp;;+tag|t)+comment_for_reflogtag++mark_action_done+outputgittag-f"$sha1"+;;*)warn"Unknown command: $command$sha1$rest"die_with_patch$sha1"Please fix this in the file $TODO."
@@ -655,6 +661,7 @@ do# reset commit = reset HEAD to the commit# merge commit-M commit-P ... = redo merge commit-M with the# current HEAD and the parents commit-P+# tag = reset tag to the current HEAD## If you remove a line here THAT COMMIT WILL BE LOST.# However, if you remove everything, the rebase will be aborted.
From: Jörg Sommer <hidden> Date: 2016-06-15 22:44:28
This new command can be used to set symbolic marks for an commit while
doing a rebase. This symbolic name can later be used for merges or
resets.
The decision to use references for the marks and not files like done with
the rewritten commits for preserve merges was made to ensure no commit
objects get lost if prune is started while (a long term) rebase is
running. This also unifies the checking of the validity of marks and
references by using rev-parse for it.
The usage of : as the sign for marks conforms with the tag sign of
fast-export and fast-import.
Signed-off-by: Jörg Sommer <redacted>
---
git-rebase--interactive.sh | 37 ++++++++++++++++++++++++++++++++++++-
t/t3404-rebase-interactive.sh | 17 +++++++++++++++++
2 files changed, 53 insertions(+), 1 deletions(-)
@@ -35,6 +35,8 @@ mark the corrected paths with 'git add <paths>', and run'git rebase --continue'"exportGIT_CHERRY_PICK_HELP+mark_prefix=refs/rebase-marks/+ warn(){echo"$*">&2}
@@ -244,6 +252,19 @@ peek_next_command () {sed-n"1s/ .*$//p"<"$TODO"}+mark_to_ref(){+case"$1"in+:[!/]*)+# :/SOMETHING is a reference for the last commit whose+# message starts with SOMETHING+echo"$mark_prefix${1#:}"+;;+*)+echo"$1"+;;+esac+}+ do_next(){rm-f"$DOTEST"/message"$DOTEST"/author-script\"$DOTEST"/amend||exit
@@ -321,6 +342,15 @@ do_next () {die_with_patch$sha1""fi;;+mark)+mark_action_done++mark=$(mark_to_ref:${sha1#:})+gitrev-parse--verify"$mark">/dev/null2>&1&&\+warn"mark $sha1 already exist; overwriting it"++gitupdate-ref"$mark"HEAD||die"update-ref failed"+;;*)warn"Unknown command: $command$sha1$rest"die_with_patch$sha1"Please fix this in the file $TODO."
@@ -533,10 +563,15 @@ do# Rebase $SHORTUPSTREAM..$SHORTHEAD onto $SHORTONTO#+# In the todo insn whenever you need to refer to a commit, in addition+# to the usual commit object name, you can use ':mark' syntax to refer+# to a commit previously marked with the 'mark' insn.+## Commands:# pick = use commit# edit = use commit, but stop for amending# squash = use commit, but meld into previous commit+# mark :mark = mark the current HEAD for later reference## If you remove a line here THAT COMMIT WILL BE LOST.# However, if you remove everything, the rebase will be aborted.
@@ -82,6 +82,9 @@ for line in $FAKE_LINES; docase$lineinsquash|edit)action="$line";;+mark*)+echo"mark ${line#mark}"+echo"mark ${line#mark}">>"$1";;*)echosed-n"${line}s/^pick/$action/p"sed-n"${line}p"<"$1".tmp
@@ -189,6 +192,20 @@ test_expect_success '-p handles "no changes" gracefully' 'test$HEAD=$(gitrev-parseHEAD)'+test_expect_success'setting marks works''+gitcheckoutmaster&&+FAKE_LINES="mark:0 2 1 mark:42 3 edit 4"gitrebase-iHEAD~4&&+marks_dir=.git/refs/rebase-marks&&+test-d$marks_dir&&+test$(ls$marks_dir|wc-l)-eq2&&+test"$(gitrev-parseHEAD~4)"=\+"$(gitrev-parserefs/rebase-marks/0)"&&+test"$(gitrev-parseHEAD~2)"=\+"$(gitrev-parserefs/rebase-marks/42)"&&+gitrebase--abort&&+ls$marks_dir|wc-l|grep-Fx0+'+ test_expect_success'preserve merges with -p''gitcheckout-bto-be-preservedmaster^&&:>unrelated-file&&
From: Jörg Sommer <hidden> Date: 2016-06-15 22:44:28
The old fake-editor selects only lines they start with pick, if you give
the number of a line. With the new commands mark, merge and reset it was
not possible to select such lines for the new TODO list. The new
fake-editor selects all kinds of lines, but replaces only the command
“pick” with a different action.
Signed-off-by: Jörg Sommer <redacted>
---
t/t3404-rebase-interactive.sh | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
@@ -92,9 +92,8 @@ for line in $FAKE_LINES; doecho"merge ${line#merge}"|tr/' 'echo"merge ${line#merge}"|tr/' '>>"$1";;*)-echosed-n"${line}s/^pick/$action/p"-sed-n"${line}p"<"$1".tmp-sed-n"${line}s/^pick/$action/p"<"$1".tmp>>"$1"+sed-n"${line}{s/^pick/$action/; p;}"<"$1".tmp+sed-n"${line}{s/^pick/$action/; p;}"<"$1".tmp>>"$1"action=pick;;esacdone
From: Jörg Sommer <hidden> Date: 2016-06-15 22:44:28
This command does a hard reset of the HEAD, i.e. the next operation used
this commit as parent. This is necessary to redo the commits of different
branches they become merged later.
Signed-off-by: Jörg Sommer <redacted>
---
git-rebase--interactive.sh | 10 ++++++++++
t/t3404-rebase-interactive.sh | 17 +++++++++++++++++
2 files changed, 27 insertions(+), 0 deletions(-)
@@ -351,6 +351,15 @@ do_next () {gitupdate-ref"$mark"HEAD||die"update-ref failed";;+reset|r)+comment_for_reflogreset++tmp=$(gitrev-parse--verify"$(mark_to_ref$sha1)")||+die"Invalid parent '$sha1' in $command$sha1$rest"++mark_action_done+outputgitreset--hard$tmp+;;*)warn"Unknown command: $command$sha1$rest"die_with_patch$sha1"Please fix this in the file $TODO."
@@ -572,6 +581,7 @@ do# edit = use commit, but stop for amending# squash = use commit, but meld into previous commit# mark :mark = mark the current HEAD for later reference+# reset commit = reset HEAD to the commit## If you remove a line here THAT COMMIT WILL BE LOST.# However, if you remove everything, the rebase will be aborted.
@@ -85,6 +85,9 @@ for line in $FAKE_LINES; domark*)echo"mark ${line#mark}"echo"mark ${line#mark}">>"$1";;+reset*)+echo"reset ${line#reset}"+echo"reset ${line#reset}">>"$1";;*)echosed-n"${line}s/^pick/$action/p"sed-n"${line}p"<"$1".tmp
@@ -206,6 +209,20 @@ test_expect_success 'setting marks works' 'ls$marks_dir|wc-l|grep-Fx0'+test_expect_success'reset with nonexistent mark fails''+exportFAKE_LINES="reset:0 1"&&+test_must_failgitrebase-iHEAD~1&&+unsetFAKE_LINES&&+gitrebase--abort+'++test_expect_success'reset to HEAD is a nop''+test_tick&&+head=$(gitrev-parse--shortHEAD)&&+FAKE_LINES="reset$head"gitrebase-iHEAD~4&&+test"$(gitrev-parse--shortHEAD)"="$head"+'+ test_expect_success'preserve merges with -p''gitcheckout-bto-be-preservedmaster^&&:>unrelated-file&&
@@ -192,22 +211,8 @@ pick_one_preserving_merges () {echo$sha1>"$DOTEST"/current-commitcase"$new_parents"in' '*' '*)-# redo merge-author_script=$(get_author_ident_from_commit$sha1)-eval"$author_script"-msg="$(gitcat-filecommit$sha1|sed-e'1,/^$/d')"# No point in merging the first parent, that's HEAD-new_parents=${new_parents# $first_parent}-if!GIT_AUTHOR_NAME="$GIT_AUTHOR_NAME"\-GIT_AUTHOR_EMAIL="$GIT_AUTHOR_EMAIL"\-GIT_AUTHOR_DATE="$GIT_AUTHOR_DATE"\-outputgitmerge$STRATEGY-m"$msg"\-$new_parents-then-gitrerere-printf"%s\n""$msg">"$GIT_DIR"/MERGE_MSG-dieErrorredoingmerge$sha1-fi+redo_merge$sha1${new_parents# $first_parent};;*)outputgitcherry-pick"$@"||
From: Jörg Sommer <hidden> Date: 2016-06-15 22:44:28
The current algorithmus used to rebase a branch with merges on top of
another has some drawbacks: it's not possible to squash commits, it's not
possible to change the order of commits, particularly the tip of the
branch can't change.
This new algorithmus uses the idea from Junio to create a TODO list with
the commands mark, merge and reset to represent the nonlinear structure
of merges.
Signed-off-by: Jörg Sommer <redacted>
---
git-rebase--interactive.sh | 239 ++++++++++++++++++++++++-----------------
t/t3404-rebase-interactive.sh | 37 +++++++
2 files changed, 175 insertions(+), 101 deletions(-)
@@ -148,8 +146,6 @@ pick_one () {no_ff=case"$1"in-n)sha1=$2;no_ff=t;;*)sha1=$1;;esacoutputgitrev-parse--verify$sha1||die"Invalid commit name: $sha1"-test-d"$REWRITTEN"&&-pick_one_preserving_merges"$@"&&returnparent_sha1=$(gitrev-parse--verify$sha1^)||die"Could not get the parent of $sha1"current_sha1=$(gitrev-parse--verifyHEAD)
@@ -163,66 +159,6 @@ pick_one () {fi}-pick_one_preserving_merges(){-case"$1"in-n)sha1=$2;;*)sha1=$1;;esac-sha1=$(gitrev-parse$sha1)--iftest-f"$DOTEST"/current-commit-then-current_commit=$(cat"$DOTEST"/current-commit)&&-gitrev-parseHEAD>"$REWRITTEN"/$current_commit&&-rm"$DOTEST"/current-commit||-die"Cannot write current commit's replacement sha1"-fi--# rewrite parents; if none were rewritten, we can fast-forward.-fast_forward=t-preserve=t-new_parents=-forpin$(gitrev-list--parents-1$sha1|cut-d' '-f2-)-do-iftest-f"$REWRITTEN"/$p-then-preserve=f-new_p=$(cat"$REWRITTEN"/$p)-test$p!=$new_p&&fast_forward=f-case"$new_parents"in-*$new_p*)-;;# do nothing; that parent is already there-*)-new_parents="$new_parents$new_p"-;;-esac-fi-done-case$fast_forwardin-t)-outputwarn"Fast forward to $sha1"-test$preserve=f||echo$sha1>"$REWRITTEN"/$sha1-;;-f)-test"a$1"=a-n&&die"Refusing to squash a merge: $sha1"--first_parent=$(expr"$new_parents":' \([^ ]*\)')-# detach HEAD to current parent-outputgitcheckout$first_parent2>/dev/null||-die"Cannot move HEAD to $first_parent"--echo$sha1>"$DOTEST"/current-commit-case"$new_parents"in-' '*' '*)-# No point in merging the first parent, that's HEAD-redo_merge$sha1${new_parents# $first_parent}-;;-*)-outputgitcherry-pick"$@"||-die_with_patch$sha1"Could not pick $sha1"-;;-esac-;;-esac-}- nth_string(){case"$1"in*1[0-9]|*[04-9])echo"$1"th;;
@@ -568,33 +615,23 @@ doecho$ONTO>"$DOTEST"/ontotest-z"$STRATEGY"||echo"$STRATEGY">"$DOTEST"/strategytestt="$VERBOSE"&&:>"$DOTEST"/verbose-iftestt="$PRESERVE_MERGES"-then-# $REWRITTEN contains files for each commit that is-# reachable by at least one merge base of $HEAD and-# $UPSTREAM. They are not necessarily rewritten, but-# their children might be.-# This ensures that commits on merged, but otherwise-# unrelated side branches are left alone. (Think "X"-# in the man page's example.)-mkdir"$REWRITTEN"&&-forcin$(gitmerge-base--all$HEAD$UPSTREAM)-do-echo$ONTO>"$REWRITTEN"/$c||-die"Could not init rewritten commits"-done-MERGES_OPTION=-else-MERGES_OPTION=--no-merges-fiSHORTUPSTREAM=$(gitrev-parse--short=7$UPSTREAM)SHORTHEAD=$(gitrev-parse--short=7$HEAD)SHORTONTO=$(gitrev-parse--short=7$ONTO)-gitrev-list$MERGES_OPTION--pretty=oneline--abbrev-commit\---abbrev=7--reverse--left-right--cherry-pick\-$UPSTREAM...$HEAD|\-sed-n"s/^>/pick /p">"$TODO"+common_rev_list_opts="--abbrev-commit --abbrev=7+--left-right--cherry-pick$UPSTREAM...$HEAD"+iftestt="$PRESERVE_MERGES"+then+gitrev-list--pretty='format:%h_%p_%s'--topo-order\+$common_rev_list_opts|\+grep-v^commit|\+create_extended_todo_list+else+gitrev-list--no-merges--reverse--pretty=oneline\+$common_rev_list_opts|sed-n"s/^>/pick /p"+fi>"$TODO"+cat>>"$TODO"<<EOF# Rebase $SHORTUPSTREAM..$SHORTHEAD onto $SHORTONTO
From: Jörg Sommer <hidden> Date: 2016-06-15 22:44:29
This new command can be used to set symbolic marks for an commit while
doing a rebase. This symbolic name can later be used for merges or
resets.
The decision to use references for the marks and not files like done with
the rewritten commits for preserve merges was made to ensure no commit
objects get lost if prune is started while (a long term) rebase is
running. This also unifies the checking of the validity of marks and
references by using rev-parse for it.
The format of the marks is as close as possible to the format of the
marks used by fast-export and fast-import, i.e. :001 == :1 and
“:12a” == :12. It differs from the format of fast-import in that point
that it requires a digit after the colon, i.e. “:abc” != :0 and “:-12”
and “:+12” aren't allowed.
Signed-off-by: Jörg Sommer <redacted>
---
git-rebase--interactive.sh | 35 ++++++++++++++++++++++++++++++++++-
t/t3404-rebase-interactive.sh | 17 +++++++++++++++++
2 files changed, 51 insertions(+), 1 deletions(-)
The difference to the v2 patch is the definition of the mark as
discussed with "Shawn O. Pearce"
mark_to_ref () {
case "$1" in
- :[!/]*)
- # :/SOMETHING is a reference for the last commit whose
- # message starts with SOMETHING
- echo "$mark_prefix${1#:}"
+ :[0-9]*)
+ echo "$mark_prefix$(printf %d ${1#:} 2>/dev/null)"
;;
*)
echo "$1"
@@ -35,6 +35,8 @@ mark the corrected paths with 'git add <paths>', and run'git rebase --continue'"exportGIT_CHERRY_PICK_HELP+mark_prefix=refs/rebase-marks/+ warn(){echo"$*">&2}
@@ -321,6 +340,15 @@ do_next () {die_with_patch$sha1""fi;;+mark)+mark_action_done++mark=$(mark_to_ref:${sha1#:})+gitrev-parse--verify"$mark">/dev/null2>&1&&\+warn"mark $sha1 already exist; overwriting it"++gitupdate-ref"$mark"HEAD||die"update-ref failed"+;;*)warn"Unknown command: $command$sha1$rest"die_with_patch$sha1"Please fix this in the file $TODO."
@@ -533,10 +561,15 @@ do# Rebase $SHORTUPSTREAM..$SHORTHEAD onto $SHORTONTO#+# In the todo insn whenever you need to refer to a commit, in addition+# to the usual commit object name, you can use ':mark' syntax to refer+# to a commit previously marked with the 'mark' insn.+## Commands:# pick = use commit# edit = use commit, but stop for amending# squash = use commit, but meld into previous commit+# mark :mark = mark the current HEAD for later reference## If you remove a line here THAT COMMIT WILL BE LOST.# However, if you remove everything, the rebase will be aborted.
@@ -82,6 +82,9 @@ for line in $FAKE_LINES; docase$lineinsquash|edit)action="$line";;+mark*)+echo"mark ${line#mark}"+echo"mark ${line#mark}">>"$1";;*)echosed-n"${line}s/^pick/$action/p"sed-n"${line}p"<"$1".tmp
@@ -189,6 +192,20 @@ test_expect_success '-p handles "no changes" gracefully' 'test$HEAD=$(gitrev-parseHEAD)'+test_expect_success'setting marks works''+gitcheckoutmaster&&+FAKE_LINES="mark:0 2 1 mark:42 3 edit 4"gitrebase-iHEAD~4&&+marks_dir=.git/refs/rebase-marks&&+test-d$marks_dir&&+test$(ls$marks_dir|wc-l)-eq2&&+test"$(gitrev-parseHEAD~4)"=\+"$(gitrev-parserefs/rebase-marks/0)"&&+test"$(gitrev-parseHEAD~2)"=\+"$(gitrev-parserefs/rebase-marks/42)"&&+gitrebase--abort&&+ls$marks_dir|wc-l|grep-Fx0+'+ test_expect_success'preserve merges with -p''gitcheckout-bto-be-preservedmaster^&&:>unrelated-file&&
From: Shawn O. Pearce <hidden> Date: 2016-06-15 22:44:29
Jrg Sommer [off-list ref] wrote:
The format of the marks is as close as possible to the format of the
marks used by fast-export and fast-import,
Yay.
i.e. :001 == :1 and
“:12a” == :12. It differs from the format of fast-import in that point
that it requires a digit after the colon, i.e. “:abc” != :0 and “:-12”
and “:+12” aren't allowed.
Uh, that's a bug in fast-import. ":4abc" is _not_ a mark if you
read the language specification. Only ":4" is a mark. So we are
accepting crap and reading it in odd ways. Not good.
--
Shawn.
From: Jörg Sommer <hidden> Date: 2016-06-15 22:44:31
Hallo Shawn,
Shawn O. Pearce schrieb am Mon 14. Apr, 19:29 (-0400):
Jrg Sommer [off-list ref] wrote:
quoted
The format of the marks is as close as possible to the format of the
marks used by fast-export and fast-import,
Yay.
quoted
i.e. :001 == :1 and
“:12a” == :12. It differs from the format of fast-import in that point
that it requires a digit after the colon, i.e. “:abc” != :0 and “:-12”
and “:+12” aren't allowed.
Uh, that's a bug in fast-import. ":4abc" is _not_ a mark if you
read the language specification. Only ":4" is a mark. So we are
accepting crap and reading it in odd ways. Not good.
@@ -2045,7 +2069,11 @@ static int cmd_from(struct branch *b)hashcpy(b->branch_tree.versions[0].sha1,t);hashcpy(b->branch_tree.versions[1].sha1,t);}elseif(*from==':'){-uintmax_tidnum=strtoumax(from+1,NULL,10);+char*after_mark;+uintmax_tidnum;+if(parse_mark(from,&idnum,&after_mark)||+*after_mark!='\0')+die("Not a valid mark: %s",from);structobject_entry*oe=find_mark(idnum);if(oe->type!=OBJ_COMMIT)die("Mark :%"PRIuMAX" not a commit",idnum);
@@ -2080,7 +2108,11 @@ static struct hash_list *cmd_merge(unsigned int *count)if(s)hashcpy(n->sha1,s->sha1);elseif(*from==':'){-uintmax_tidnum=strtoumax(from+1,NULL,10);+char*after_mark;+uintmax_tidnum;+if(parse_mark(from,&idnum,&after_mark)||+*after_mark!='\0')+die("Not a valid mark: %s",from);structobject_entry*oe=find_mark(idnum);if(oe->type!=OBJ_COMMIT)die("Mark :%"PRIuMAX" not a commit",idnum);
@@ -2228,7 +2260,10 @@ static void cmd_new_tag(void)hashcpy(sha1,s->sha1);}elseif(*from==':'){structobject_entry*oe;-from_mark=strtoumax(from+1,NULL,10);+char*after_mark;+if(parse_mark(from,&from_mark,&after_mark)||+*after_mark!='\0')+die("Not a valid mark: %s",from);oe=find_mark(from_mark);if(oe->type!=OBJ_COMMIT)die("Mark :%"PRIuMAX" not a commit",from_mark);
@@ -2333,9 +2368,8 @@ static void import_marks(const char *input_file)if(line[0]!=':'||!end)die("corrupt mark line: %s",line);*end=0;-mark=strtoumax(line+1,&end,10);-if(!mark||end==line+1-||*end!=' '||get_sha1(end+1,sha1))+if(parse_mark(line,&mark,&end)||!mark||+*end!=' '||get_sha1(end+1,sha1))die("corrupt mark line: %s",line);e=find_object(sha1);if(!e){
Bye, Jörg.
--
Wer eher stirbt ist länger tot.
(Un B. Kant)
From: Shawn O. Pearce <hidden> Date: 2016-06-15 22:44:31
Jrg Sommer [off-list ref] wrote:
quoted hunk
quoted
Uh, that's a bug in fast-import. ":4abc" is _not_ a mark if you
read the language specification. Only ":4" is a mark. So we are
accepting crap and reading it in odd ways. Not good.
Hmm. Shouldn't this be ! parse_mark given that it returns 0
on success and 1 on failure?
Yes, you're right. I've checked some other functions and found this
behaviour. Can I use a different behabiour, i.e. return 0 on failure and
!0 on success?
Bye, Jörg.
--
„Wer im Usenet gelesen werden will, sollte leserorientiert schreiben. Wer nur
für sich schreiben will, dem ist mit einem Tagebuch vielleicht besser
geholfen. Gelesen zu werden ist kein Recht, sondern ein Privileg.“
Thore Tams in [off-list ref]
Hmm. Shouldn't this be ! parse_mark given that it returns 0
on success and 1 on failure?
Yes, you're right. I've checked some other functions and found this
behaviour. Can I use a different behabiour, i.e. return 0 on failure and
!0 on success?
I wasn't objected to the return values as written, but more to the
fact that it seemed like a logic error to me. We use both patterns
in Git. Perhaps the best example to follow is get_sha1_hex();
it returns -1 on error and 0 on success. So a common pattern is
"!get_sha1_hex()" to ensure a successful conversion of a hex string
to an unsigned char array.
--
Shawn.
From: Jörg Sommer <hidden> Date: 2016-06-15 22:44:31
Hallo Shawn,
Shawn O. Pearce schrieb am Mon 21. Apr, 19:59 (-0400):
Jrg Sommer [off-list ref] wrote:
quoted
Yes, you're right. I've checked some other functions and found this
behaviour. Can I use a different behabiour, i.e. return 0 on failure and
!0 on success?
I wasn't objected to the return values as written, but more to the
fact that it seemed like a logic error to me. We use both patterns
in Git. Perhaps the best example to follow is get_sha1_hex();
it returns -1 on error and 0 on success. So a common pattern is
"!get_sha1_hex()" to ensure a successful conversion of a hex string
to an unsigned char array.
Thanks for this explanation. This was what I was looking for.
Another question: Is :0 a valid mark? In import_marks() is a check for
!mark, but I haven't seen it anywhere else.
Bye, Jörg.
--
Du hast keine Chance – also nutze sie.
From: Shawn O. Pearce <hidden> Date: 2016-06-15 22:44:31
Jrg Sommer [off-list ref] wrote:
Another question: Is :0 a valid mark? In import_marks() is a check for
!mark, but I haven't seen it anywhere else.
No, in fast-import ":0" is _not_ a valid mark. We burn the first
entry in the marks table (always leaving it empty) as then we can
use the idiom "!mark" to say "no mark was requested/given" and
"mark" to say "mark was requested/given". Hence we do not need an
extra flag to tell us either way.
Given that a mark is just a pointer, and that extra flag would
likely have been a global "static int have_mark" or some such it
works out to be about the same amount of memory - 4 or 8 bytes.
No big deal, and the code is probably easier to follow as a result.
--
Shawn.
From: Jörg Sommer <hidden> Date: 2016-06-15 22:44:31
The current implementation of mark parsing doesn't care for trailing
garbage like in :12a and doesn't check for unsigned numbers, i.e. it
accepts :-12 as a valid mark.
This patch enforces a number follows the colon and there comes nothing
after the bignum.
Signed-off-by: Jörg Sommer <redacted>
---
fast-import.c | 49 ++++++++++++++++++++++++++++++++++++++-----------
1 files changed, 38 insertions(+), 11 deletions(-)
"Shawn O. Pearce" [off-list ref] wrote:
Jrg Sommer [off-list ref] wrote:
quoted
Another question: Is :0 a valid mark? In import_marks() is a check for
!mark, but I haven't seen it anywhere else.
@@ -2045,7 +2064,10 @@ static int cmd_from(struct branch *b)hashcpy(b->branch_tree.versions[0].sha1,t);hashcpy(b->branch_tree.versions[1].sha1,t);}elseif(*from==':'){-uintmax_tidnum=strtoumax(from+1,NULL,10);+char*after_mark;+uintmax_tidnum=parse_mark(from,&after_mark);+if(!idnum||*after_mark!='\0')+die("Not a valid mark: %s",from);structobject_entry*oe=find_mark(idnum);if(oe->type!=OBJ_COMMIT)die("Mark :%"PRIuMAX" not a commit",idnum);
@@ -2080,7 +2102,10 @@ static struct hash_list *cmd_merge(unsigned int *count)if(s)hashcpy(n->sha1,s->sha1);elseif(*from==':'){-uintmax_tidnum=strtoumax(from+1,NULL,10);+char*after_mark;+uintmax_tidnum=parse_mark(from,&after_mark);+if(!idnum||*after_mark!='\0')+die("Not a valid mark: %s",from);structobject_entry*oe=find_mark(idnum);if(oe->type!=OBJ_COMMIT)die("Mark :%"PRIuMAX" not a commit",idnum);
@@ -2228,7 +2253,10 @@ static void cmd_new_tag(void)hashcpy(sha1,s->sha1);}elseif(*from==':'){structobject_entry*oe;-from_mark=strtoumax(from+1,NULL,10);+char*after_mark;+from_mark=parse_mark(from,&after_mark);+if(!from_mark||*after_mark!='\0')+die("Not a valid mark: %s",from);oe=find_mark(from_mark);if(oe->type!=OBJ_COMMIT)die("Mark :%"PRIuMAX" not a commit",from_mark);
@@ -2333,9 +2361,8 @@ static void import_marks(const char *input_file)if(line[0]!=':'||!end)die("corrupt mark line: %s",line);*end=0;-mark=strtoumax(line+1,&end,10);-if(!mark||end==line+1-||*end!=' '||get_sha1(end+1,sha1))+mark=parse_mark(line,&end);+if(!mark||*end!=' '||get_sha1(end+1,sha1))die("corrupt mark line: %s",line);e=find_object(sha1);if(!e){