threads / patch / 2542

patch, 4 partsreworking git-rebase

Subject: [PATCH 0/4] reworking git-rebase

## tl;dr

8 messages between Nov 15, 2005 and Nov 17, 2005. Diffs are folded; open one to read it.

replies: 7people: 2as markdown or json

Junio C Hamano· Nov 15, 2005, 23:32 UTC · lore

The current rebase implementation finds commits in our tree but not in the upstream tree using git-cherry, and tries to apply them using git-cherry-pick (i.e. always use 3-way) one by one.

Which is fine, but when some of the changes do not apply cleanly, it punts, and punts badly.

Suppose you have commits A-B-C-D-E since you forked from the upstream and submitted the changes for inclusion. You fetch from upstream head U and find that B has been picked up. You run git-rebase to update your branch, which tries to apply changes contained in A-C-D-E, in this order, but replaying of C fails, because the upstream got changes that touch the same area from elsewhere.

Now what?

It notes that fact, and goes ahead to apply D and E, and at the very end tells you to deal with C by hand. Even if you somehow managed to replay C on top of the result, you would now end up with ...-B-...-U-A-D-E-C.

Breaking the order between B and others was the conscious decision made by the upstream, so we would not worry about it, and even if it were worrisome, it is too late for us to fix now. What D and E do may well depend on having C applied before them, which is a problem for us.

Here is a series requesting comments and testing. I think this change makes rebase easier to use when some of the changes do not replay cleanly. What it does is:

 - Instead of calling git-cherry-pick, it runs git-format-patch
   on the unmerged commits git-cherry found, and feeds the
   result to "git-am --3way".
 - To keep rebase working on a repository that manages binary
   files, it adds a very limited support to handle "binary
   diffs".

In the "unapplicable patch in the middle" case, this "rebase" works like this:

 - A series of patches in e-mail form is created that records
   what A-C-D-E do, and is fed to git-am.  This is stored in
   .dotest/ directory, just like the case you tried to apply
   them from your mailbox.  Your branch is rewound to the tip of
   upstream U, and the original head is kept in .git/ORIG_HEAD,
   so you could "git reset --hard ORIG_HEAD" in case the end
   result is really messy.
 - Patch A applies cleanly.  This could either be a clean patch
   application on top of rewound head (i.e. same as upstream
   head), or git-am might have internally fell back on 3-way
   (i.e.  it would have done the same thing as git-cherry-pick).
   In either case, a rebased commit A is made on top of U.
 - Patch C does not apply.  git-am stops here, with conflicts to
   be resolved in the working tree.  Yet-to-be-applied D and E
   are still kept in .dotest/ directory at this point.  What the
   user does is exactly the same as fixing up unapplicable patch
   when running git-am:
   - Resolve conflict just like any merge conflicts. 
   - "git diff -p --full-index HEAD >.dotest/patch" to pretend
     as if you received a perfect, applicable patch.
   - "git reset --hard", to pretend you have not tried to apply
     that patch yet.
   [Side note] I think the latter two steps can and should be
   made into a short-hand to tell "git-am" that the conflicting
   patch is resolved.  "git-am --resolved", perhaps?
 - Continue with "git am --3way".  This applies the fixed-up
   patch so by definition it had better apply.  "git am" knows
   the patch after the fixed-up one is D and then E; it applies
   them, and you will get the changes from A-C-D-E commits on
   top of U, in this order.

I've been using this without noticing any problem, and as people may know I do a lot of rebases.

Junio C Hamano· Nov 15, 2005, 23:33 UTC · re: Junio C Hamano · lore

[PATCH 1/4] Rewrite rebase to use git-format-patch piped to git-am.

This does not handle binary files yet, but a patch or two to git-apply should solve that problem.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 git-rebase.sh |   71 ++++++++++++---------------------------------------------
 1 files changed, 15 insertions(+), 56 deletions(-)

applies-to: efad0bc70473bdc02b8ac6a5e5f001519f1d5dfa d64a7b926edfcebf9f5b116fedebd365e5c206f1

Show changes to git-rebase.sh +15 −56
diff --git a/git-rebase.sh b/git-rebase.sh
index fa95009..56196e7 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -5,65 +5,24 @@
 
 . git-sh-setup || die "Not a git archive."
 
-usage="usage: $0 "'<upstream> [<head>]
-
-Uses output from git-cherry to rebase local commits to the new head of
-upstream tree.'
-
-case "$#,$1" in
-1,*..*)
-    upstream=$(expr "$1" : '\(.*\)\.\.') ours=$(expr "$1" : '.*\.\.\(.*\)$')
-    set x "$upstream" "$ours"
-    shift ;;
-esac
+# The other head is given
+other=$(git-rev-parse --verify "$1^0") || exit
 
+# The tree must be really really clean.
 git-update-index --refresh || exit
+diff=$(git-diff-index --cached --name-status -r HEAD)
+case "$different" in
+?*)	echo "$diff"
+	exit 1
+	;;
+esac
 
+# If the branch to rebase is given, first switch to it.
 case "$#" in
-1) ours_symbolic=HEAD ;;
-2) ours_symbolic="$2" ;;
-*) die "$usage" ;;
+2)
+	git-checkout "$2" || exit
 esac
 
-upstream=`git-rev-parse --verify "$1"` &&
-ours=`git-rev-parse --verify "$ours_symbolic"` || exit
-different1=$(git-diff-index --name-only --cached "$ours") &&
-different2=$(git-diff-index --name-only "$ours") &&
-test "$different1$different2" = "" ||
-die "Your working tree does not match $ours_symbolic."
-
-git-read-tree -m -u $ours $upstream &&
-new_head=$(git-rev-parse --verify "$upstream^0") &&
-git-update-ref HEAD "$new_head" || exit
-
-tmp=.rebase-tmp$$
-fail=$tmp-fail
-trap "rm -rf $tmp-*" 1 2 3 15
-
->$fail
-
-git-cherry -v $upstream $ours |
-while read sign commit msg
-do
-	case "$sign" in
-	-)
-		echo >&2 "* Already applied: $msg"
-		continue ;;
-	esac
-	echo >&2 "* Applying: $msg"
-	S=$(git-rev-parse --verify HEAD) &&
-	git-cherry-pick --replay $commit || {
-		echo >&2 "* Not applying the patch and continuing."
-		echo $commit >>$fail
-		git-reset --hard $S
-	}
-done
-if test -s $fail
-then
-	echo >&2 Some commits could not be rebased, check by hand:
-	cat >&2 $fail
-	echo >&2 "(the same list of commits are found in $tmp)"
-	exit 1
-else
-	rm -f $fail
-fi
+# Rewind the head to "$other"
+git-reset --hard "$other"
+git-format-patch -k --stdout "$other" ORIG_HEAD | git am -3 -k
---
0.99.9.GIT
Junio C Hamano· Nov 15, 2005, 23:33 UTC · re: Junio C Hamano · lore

[PATCH 2/4] apply: allow-binary-replacement.

A new option, --allow-binary-replacement, is introduced.

When you feed a diff that records full SHA1 name of pre- and post-image blob on its index line to git-apply with this option, the post-image blob replaces the path if what you have in the working tree matches the pre-image _and_ post-image blob is already available in the object directory.

Later we _might_ want to enhance the diff output to also include the full binary data of the post-image, to make this more useful, but this is good enough for local rebasing application.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 Documentation/git-apply.txt |   13 +++++++-
 apply.c                     |   72 +++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 81 insertions(+), 4 deletions(-)

applies-to: 4567363448244862ec2c6c8bd74fc09cd322f153 0d534967c6850288e1a36eae364da700348a77fe

Show changes to 2 files +81 −4

Documentation/git-apply.txt, apply.c

diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt
index 6702a18..626e281 100644
--- a/Documentation/git-apply.txt
+++ b/Documentation/git-apply.txt
@@ -8,7 +8,7 @@ git-apply - Apply patch on a git index f
 
 SYNOPSIS
 --------
-'git-apply' [--stat] [--numstat] [--summary] [--check] [--index] [--apply] [--no-add] [--index-info] [-z] [<patch>...]
+'git-apply' [--stat] [--numstat] [--summary] [--check] [--index] [--apply] [--no-add] [--index-info] [--allow-binary-replacement] [-z] [<patch>...]
 
 DESCRIPTION
 -----------
@@ -79,6 +79,17 @@ OPTIONS
 	the result with this option, which would apply the
 	deletion part but not addition part.
 
+--allow-binary-replacement::
+	When applying a patch, which is a git-enhanced patch
+	that was prepared to record the pre- and post-image object
+	name in full, and the path being patched exactly matches
+	the object the patch applies to (i.e. "index" line's
+	pre-image object name is what is in the working tree),
+	and the post-image object is available in the object
+	database, use the post-image object as the patch
+	result.  This allows binary files to be patched in a
+	very limited way.
+
 Author
 ------
 Written by Linus Torvalds <torvalds@osdl.org>
diff --git a/apply.c b/apply.c
index 590adc6..cb9a6c0 100644
--- a/apply.c
+++ b/apply.c
@@ -16,6 +16,7 @@
 //  --numstat does numeric diffstat, and doesn't actually apply
 //  --index-info shows the old and new index info for paths if available.
 //
+static int allow_binary_replacement = 0;
 static int check_index = 0;
 static int write_index = 0;
 static int diffstat = 0;
@@ -27,7 +28,7 @@ static int no_add = 0;
 static int show_index_info = 0;
 static int line_termination = '\n';
 static const char apply_usage[] =
-"git-apply [--stat] [--numstat] [--summary] [--check] [--index] [--apply] [--no-add] [--index-info] [-z] <patch>...";
+"git-apply [--stat] [--numstat] [--summary] [--check] [--index] [--apply] [--no-add] [--index-info] [--allow-binary-replacement] [-z] <patch>...";
 
 /*
  * For "diff-stat" like behaviour, we keep track of the biggest change
@@ -899,8 +900,12 @@ static int parse_chunk(char *buffer, uns
 			    sizeof(binhdr)-1))
 			patch->is_binary = 1;
 
-		if (patch->is_binary && !apply && !check)
-			;
+		if (patch->is_binary) {
+			if ((!apply && !check) || allow_binary_replacement)
+				;
+			else
+				die("binary patch at line %d; you might want to try --allow-binary-replacement?", linenr);
+		}
 		else
 			die("patch with only garbage at line %d", linenr);
 	}
@@ -1156,6 +1161,63 @@ static int apply_fragments(struct buffer
 {
 	struct fragment *frag = patch->fragments;
 
+	if (patch->is_binary) {
+		unsigned char sha1[20];
+		const char *name = patch->old_name ? patch->old_name : patch->new_name;
+
+		if (!allow_binary_replacement)
+			return error("cannot apply binary patch to '%s' without --allow-binary-replacement", name);
+
+		/* For safety, we require patch index line to contain
+		 * full 40-byte textual SHA1 for old and new, at least for now.
+		 */
+		if (strlen(patch->old_sha1_prefix) != 40 ||
+		    strlen(patch->new_sha1_prefix) != 40 ||
+		    get_sha1_hex(patch->old_sha1_prefix, sha1) ||
+		    get_sha1_hex(patch->new_sha1_prefix, sha1))
+			return error("cannot apply binary patch to '%s' without full index line", name);
+
+		if (patch->old_name) {
+			unsigned char hdr[50];
+			int hdrlen;
+
+			/* See if the old one matches what the patch
+			 * applies to.
+			 */
+			write_sha1_file_prepare(desc->buffer, desc->size,
+						"blob", sha1, hdr, &hdrlen);
+			if (strcmp(sha1_to_hex(sha1), patch->old_sha1_prefix))
+				return error("the patch applies to '%s' (%s), which does not match the current contents.", name, sha1_to_hex(sha1));
+		}
+		else {
+			/* Otherwise, the old one must be empty. */
+			if (desc->size)
+				return error("the patch applies to an empty '%s' but it is not empty", name);
+		}
+
+		/* For now, we do not record post-image data in the patch,
+		 * and require the object already present in the recipient's
+		 * object database.
+		 */
+		if (desc->buffer) {
+			free(desc->buffer);
+			desc->alloc = desc->size = 0;
+		}
+		get_sha1_hex(patch->new_sha1_prefix, sha1);
+
+		if (memcmp(sha1, null_sha1, 20)) {
+			char type[10];
+			unsigned long size;
+
+			desc->buffer = read_sha1_file(sha1, type, &size);
+			if (!desc->buffer)
+				return error("the necessary postimage %s for '%s' does not exist", patch->new_sha1_prefix, name);
+			desc->alloc = desc->size = size;
+		}
+
+		return 0;
+	}
+
 	while (frag) {
 		if (apply_one_fragment(desc, frag) < 0)
 			return error("patch failed: %s:%ld", patch->old_name, frag->oldpos);
@@ -1723,6 +1785,10 @@ int main(int argc, char **argv)
 			diffstat = 1;
 			continue;
 		}
+		if (!strcmp(arg, "--allow-binary-replacement")) {
+			allow_binary_replacement = 1;
+			continue;
+		}
 		if (!strcmp(arg, "--numstat")) {
 			apply = 0;
 			numstat = 1;
---
0.99.9.GIT
Junio C Hamano· Nov 15, 2005, 23:34 UTC · re: Junio C Hamano · lore

[PATCH 3/4] diff: --full-index

A new option, --full-index, is introduced to diff family. This causes the full object name of pre- and post-images to appear on the index line of patch formatted output, to be used in conjunction with --allow-binary-replacement option of git-apply.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 Documentation/diff-options.txt |    5 +++++
 diff.c                         |   14 ++++++++------
 diff.h                         |    4 +++-
 3 files changed, 16 insertions(+), 7 deletions(-)

applies-to: 87c089ccd1a4e2ea6284c1ac5e944efd5299f60e 3c38f257b87e61f02e8bf49195eb5084652ff0f9

Show changes to 3 files +16 −7

Documentation/diff-options.txt, diff.c, diff.h

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 8eef86e..6b496ed 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -13,6 +13,11 @@
 --name-status::
 	Show only names and status of changed files.
 
+--full-index::
+	Instead of the first handful characters, show full
+	object name of pre- and post-image blob on the "index"
+	line when generating a patch format output.	
+
 -B::
 	Break complete rewrite changes into pairs of delete and create.
 
diff --git a/diff.c b/diff.c
index ec94a96..8b7bb67 100644
--- a/diff.c
+++ b/diff.c
@@ -648,7 +648,7 @@ static void diff_fill_sha1_info(struct d
 		memset(one->sha1, 0, 20);
 }
 
-static void run_diff(struct diff_filepair *p)
+static void run_diff(struct diff_filepair *p, struct diff_options *o)
 {
 	const char *pgm = external_diff();
 	char msg[PATH_MAX*2+300], *xfrm_msg;
@@ -711,11 +711,11 @@ static void run_diff(struct diff_filepai
 
 	if (memcmp(one->sha1, two->sha1, 20)) {
 		char one_sha1[41];
+		const char *index_fmt = o->full_index ? "index %s..%s" : "index %.7s..%.7s";
 		memcpy(one_sha1, sha1_to_hex(one->sha1), 41);
 
 		len += snprintf(msg + len, sizeof(msg) - len,
-				"index %.7s..%.7s", one_sha1,
-				sha1_to_hex(two->sha1));
+				index_fmt, one_sha1, sha1_to_hex(two->sha1));
 		if (one->mode == two->mode)
 			len += snprintf(msg + len, sizeof(msg) - len,
 					" %06o", one->mode);
@@ -789,6 +789,8 @@ int diff_opt_parse(struct diff_options *
 		options->line_termination = 0;
 	else if (!strncmp(arg, "-l", 2))
 		options->rename_limit = strtoul(arg+2, NULL, 10);
+	else if (!strcmp(arg, "--full-index"))
+		options->full_index = 1;
 	else if (!strcmp(arg, "--name-only"))
 		options->output_format = DIFF_FORMAT_NAME;
 	else if (!strcmp(arg, "--name-status"))
@@ -1017,7 +1019,7 @@ int diff_unmodified_pair(struct diff_fil
 	return 0;
 }
 
-static void diff_flush_patch(struct diff_filepair *p)
+static void diff_flush_patch(struct diff_filepair *p, struct diff_options *o)
 {
 	if (diff_unmodified_pair(p))
 		return;
@@ -1026,7 +1028,7 @@ static void diff_flush_patch(struct diff
 	    (DIFF_FILE_VALID(p->two) && S_ISDIR(p->two->mode)))
 		return; /* no tree diffs in patch format */ 
 
-	run_diff(p);
+	run_diff(p, o);
 }
 
 int diff_queue_is_empty(void)
@@ -1158,7 +1160,7 @@ void diff_flush(struct diff_options *opt
 			die("internal error in diff-resolve-rename-copy");
 		switch (diff_output_format) {
 		case DIFF_FORMAT_PATCH:
-			diff_flush_patch(p);
+			diff_flush_patch(p, options);
 			break;
 		case DIFF_FORMAT_RAW:
 		case DIFF_FORMAT_NAME_STATUS:
diff --git a/diff.h b/diff.h
index 1259079..9b2e1e6 100644
--- a/diff.h
+++ b/diff.h
@@ -32,7 +32,8 @@ struct diff_options {
 	const char *orderfile;
 	const char *pickaxe;
 	unsigned recursive:1,
-		 tree_in_recursive:1;
+		 tree_in_recursive:1,
+		 full_index:1;
 	int break_opt;
 	int detect_rename;
 	int find_copies_harder;
@@ -96,6 +97,7 @@ extern void diffcore_std_no_resolve(stru
 "  -u            synonym for -p.\n" \
 "  --name-only   show only names of changed files.\n" \
 "  --name-status show names and status of changed files.\n" \
+"  --full-index  show full object name on index ines.\n" \
 "  -R            swap input file pairs.\n" \
 "  -B            detect complete rewrites.\n" \
 "  -M            detect renames.\n" \
---
0.99.9.GIT
Junio C Hamano· Nov 15, 2005, 23:34 UTC · re: Junio C Hamano · lore

[PATCH 4/4] rebase: make it usable for binary files as well.

Use git-format-patch --full-index to generate a patch, and feed that to git-am that uses git-apply --allow-binary-replacement. This way, rebase can be used to move development that contains binary files as long as there is no need for file-level merges.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 git-am.sh     |    7 ++++---
 git-rebase.sh |    2 +-
 2 files changed, 5 insertions(+), 4 deletions(-)

applies-to: 349326f3647597dabf38d82e1c2806cef45060f7 7f5bff7c3e5b1245e479fc13665641424dee9a41

Show changes to 2 files +5 −4

git-am.sh, git-rebase.sh

diff --git a/git-am.sh b/git-am.sh
index 38841d9..8307d77 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -46,7 +46,7 @@ fall_back_3way () {
 	    cd "$dotest/patch-merge-tmp-dir" &&
 	    GIT_INDEX_FILE="../patch-merge-tmp-index" \
 	    GIT_OBJECT_DIRECTORY="$O_OBJECT" \
-	    git-apply --index <../patch
+	    git-apply --allow-binary-replacement --index <../patch
         )
     then
 	echo Using index info to reconstruct a base tree...
@@ -77,7 +77,7 @@ fall_back_3way () {
 		GIT_OBJECT_DIRECTORY="$O_OBJECT" &&
 		export GIT_INDEX_FILE GIT_OBJECT_DIRECTORY &&
 		git-read-tree "$base" &&
-		git-apply --index &&
+		git-apply --index --allow-binary-replacement &&
 		mv ../patch-merge-tmp-index ../patch-merge-index &&
 		echo "$base" >../patch-merge-base
 	    ) <"$dotest/patch"  2>/dev/null && break
@@ -310,7 +310,8 @@ do
 	echo "Applying '$SUBJECT'"
 	echo
 
-	git-apply --index "$dotest/patch"; apply_status=$?
+	git-apply --allow-binary-replacement --index "$dotest/patch"
+	apply_status=$?
 	if test $apply_status = 1 && test "$threeway" = t
 	then
 		if (fall_back_3way)
diff --git a/git-rebase.sh b/git-rebase.sh
index 56196e7..b8abf33 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -25,4 +25,4 @@ esac
 
 # Rewind the head to "$other"
 git-reset --hard "$other"
-git-format-patch -k --stdout "$other" ORIG_HEAD | git am -3 -k
+git-format-patch -k --stdout --full-index "$other" ORIG_HEAD | git am -3 -k
---
0.99.9.GIT
Junio C Hamano· Nov 16, 2005, 09:26 UTC · re: Junio C Hamano · lore

[PATCH] git-am: --resolved.

Junio C Hamano <junkio@cox.net> writes:
Show 17 quoted lines
>  - Patch C does not apply.  git-am stops here, with conflicts to
>    be resolved in the working tree.  Yet-to-be-applied D and E
>    are still kept in .dotest/ directory at this point.  What the
>    user does is exactly the same as fixing up unapplicable patch
>    when running git-am:
>
>    - Resolve conflict just like any merge conflicts. 
>
>    - "git diff -p --full-index HEAD >.dotest/patch" to pretend
>      as if you received a perfect, applicable patch.
>
>    - "git reset --hard", to pretend you have not tried to apply
>      that patch yet.
>
>    [Side note] I think the latter two steps can and should be
>    made into a short-hand to tell "git-am" that the conflicting
>    patch is resolved.  "git-am --resolved", perhaps?

It turns out that the above "diff >.dotest/patch && reset --hard" was unnecessary. Instead, the above workflow would read:

    - Resolve conflict just like any merge conflicts.
    - Do necessary 'git-update-index' to prepare your index to
      record the result of the patch application.
    - "git am --resolved".

What it does is just record the current index as tree, along with the commit log message and authorship information taken from the e-mail. No diff & patch is involved. Oh, and you can do this in a dirty working tree as long as your index is clean and dirty paths do not interfere with the patch application.

The patch I am posting here is against the tip of the master, if somebody wants to try it out. Proposed updates branch has an equivalent patch on top of the reworked rebase series.

-- >8 -- After failed patch application, you can manually apply the patch (this includes resolving the conflicted merge after git-am falls back to 3-way merge) and run git-update-index on necessary paths to prepare the index file in a shape a successful patch application should have produced. Then re-running git-am --resolved would record the resulting index file along with the commit log information taken from the patch e-mail.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 git-am.sh |   65 ++++++++++++++++++++++++++++++++++++++++++++++++++-----------
 1 files changed, 53 insertions(+), 12 deletions(-)

applies-to: 1e3fcf60526c196a46433e6947c9104ca236f230 3efc3e216c8a64fae53a15767064162ee431f57e

Show changes to git-am.sh +53 −12
diff --git a/git-am.sh b/git-am.sh
index 38841d9..98a390a 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -3,16 +3,10 @@
 #
 . git-sh-setup || die "Not a git archive"
 
-files=$(git-diff-index --cached --name-only HEAD) || exit
-if [ "$files" ]; then
-   echo "Dirty index: cannot apply patches (dirty: $files)" >&2
-   exit 1
-fi
-
 usage () {
     echo >&2 "usage: $0 [--signoff] [--dotest=<dir>] [--utf8] [--3way] <mbox>"
     echo >&2 "	or, when resuming"
-    echo >&2 "	$0 [--skip]"
+    echo >&2 "	$0 [--skip | --resolved]"
     exit 1;
 }
 
@@ -104,7 +98,7 @@ fall_back_3way () {
 }
 
 prec=4
-dotest=.dotest sign= utf8= keep= skip= interactive=
+dotest=.dotest sign= utf8= keep= skip= interactive= resolved=
 
 while case "$#" in 0) break;; esac
 do
@@ -128,6 +122,9 @@ do
 	-k|--k|--ke|--kee|--keep)
 	keep=t; shift ;;
 
+	-r|--r|--re|--res|--reso|--resol|--resolv|--resolve|--resolved)
+	resolved=t; shift ;;
+
 	--sk|--ski|--skip)
 	skip=t; shift ;;
 
@@ -140,6 +137,8 @@ do
 	esac
 done
 
+# If the dotest directory exists, but we have finished applying all the
+# patches in them, clear it out.
 if test -d "$dotest" &&
    last=$(cat "$dotest/last") &&
    next=$(cat "$dotest/next") &&
@@ -155,9 +154,9 @@ then
 	die "previous dotest directory $dotest still exists but mbox given."
 	resume=yes
 else
-	# Make sure we are not given --skip
-	test ",$skip," = ,, ||
-	die "we are not resuming."
+	# Make sure we are not given --skip nor --resolved
+	test ",$skip,$resolved," = ,,, ||
+		die "we are not resuming."
 
 	# Start afresh.
 	mkdir -p "$dotest" || exit
@@ -170,12 +169,24 @@ else
 		exit 1
 	}
 
+	# -s, -u and -k flags are kept for the resuming session after
+	# a patch failure.
+	# -3 and -i can and must be given when resuming.
 	echo "$sign" >"$dotest/sign"
 	echo "$utf8" >"$dotest/utf8"
 	echo "$keep" >"$dotest/keep"
 	echo 1 >"$dotest/next"
 fi
 
+case "$resolved" in
+'')
+	files=$(git-diff-index --cached --name-only HEAD) || exit
+	if [ "$files" ]; then
+	   echo "Dirty index: cannot apply patches (dirty: $files)" >&2
+	   exit 1
+	fi
+esac
+
 if test "$(cat "$dotest/utf8")" = t
 then
 	utf8=-u
@@ -216,6 +227,15 @@ do
 		go_next
 		continue
 	}
+
+	# If we are not resuming, parse and extract the patch information
+	# into separate files:
+	#  - info records the authorship and title
+	#  - msg is the rest of commit log message
+	#  - patch is the patch body.
+	#
+	# When we are resuming, these files are either already prepared
+	# by the user, or the user can tell us to do so by --resolved flag.
 	case "$resume" in
 	'')
 		git-mailinfo $keep $utf8 "$dotest/msg" "$dotest/patch" \
@@ -263,6 +283,13 @@ do
 		fi
 	    } >"$dotest/final-commit"
 	    ;;
+	*)
+		case "$resolved,$interactive" in
+		tt)
+			# This is used only for interactive view option.
+			git-diff-index -p --cached HEAD >"$dotest/patch"
+			;;
+		esac
 	esac
 
 	resume=
@@ -310,7 +337,21 @@ do
 	echo "Applying '$SUBJECT'"
 	echo
 
-	git-apply --index "$dotest/patch"; apply_status=$?
+	case "$resolved" in
+	'')
+		git-apply --index "$dotest/patch"
+		apply_status=$?
+		;;
+	t)
+		# Resolved means the user did all the hard work, and
+		# we do not have to do any patch application.  Just
+		# trust what the user has in the index file and the
+		# working tree.
+		resolved=
+		apply_status=0
+		;;
+	esac
+
 	if test $apply_status = 1 && test "$threeway" = t
 	then
 		if (fall_back_3way)
---
0.99.9.GIT
Sven Verdoolaege· Nov 17, 2005, 10:30 UTC · re: Junio C Hamano · lore

Re: [PATCH 0/4] reworking git-rebase

On Tue, Nov 15, 2005 at 03:32:05PM -0800, Junio C Hamano wrote:
> It notes that fact, and goes ahead to apply D and E, and at the
> very end tells you to deal with C by hand.  Even if you somehow
> managed to replay C on top of the result, you would now end up
> with ...-B-...-U-A-D-E-C.

Actually this is sometimes useful when rewriting history. Suppose you cherry-pick two changes that touch the same piece of code and combine them into a single change and then rebase all your changes on top of this combined change. The two cherry-picked changes will not apply and end up and at the end of the chain where you can simply discard them.

Will the reworked rebase still support this use or is there a better way to do this ?

skimo
Junio C Hamano· Nov 17, 2005, 10:57 UTC · re: Sven Verdoolaege · lore

Re: [PATCH 0/4] reworking git-rebase

Sven Verdoolaege <skimo@kotnet.org> writes:
> Will the reworked rebase still support this use or is there
> a better way to do this ?

I think it is reasonable to assume that you would not usually want to muck with commits that cleanly replay. Further, you would do that kind of irregular replaying (e.g. not replaying all of them, or skipping some of them) only after one of the commit fails to cleanly replay, and you would not mind (rather, you would be happy) to keep the commits that have been already cleanly replayed, and would start playing games like dropping patches after the one that fails.

If the above is a reasonable assumption, then, the answer is yes. And if not, you can always resort to manual "git-format-patch -k", followed by file editing, followed by "git am -k -3", to reorder patches. Although at that point you should either be looking at StGIT or doing a just plain merge.

When "git am" first fails, all the rest of patches are kept in .dotest/. And after fixing up the failed one in your working tree, you can restart it with interactive flag, like this: "git am -i -3 --resolved". It lets you tell it skip one patch, or not apply any of the rest. [Side note: it might be a good idea to make --resolved imply --interactive].

Quite often, the reason why a rebased patch does not apply is because an equivalent patch (or a set of patches whose cumulative effect is the same as one of your patches) has been thrown in to the new "master". "git cherry" is too dumb (IOW, git-patch-id is too strict) to notice such cases. "git am -3" first tries to do git-apply, and when it fails (and it will fail if your working tree already has what the patch would do), it falls back to 3-way merge by picking up the pre-image blob ids recorded in the patch. When seeing an already applied patch, this 3-way merge often results in the same tree as you started. When "git am" notices the situation, it throws away that patch (it says "already applied") automatically.

← back to recent threads