[PATCH 0/3] Return of "add -p [E]dit" fix

[PATCH 0/3] Return of "add -p [E]dit" fix

[Junio C Hamano]

I kept this in the stalled state for too long.  Resending with tests
to move the topic forward.

Junio C Hamano (3):
  t3701: Editing a split hunk in an "add -p" session
  add--interactive.perl: factor out repeated --recount option
  "add -p": work-around an old laziness that does not coalesce hunks

 builtin/apply.c            |    9 ++++++---
 git-add--interactive.perl  |   16 ++++++++--------
 t/t3701-add-interactive.sh |   36 ++++++++++++++++++++++++++++++++++++
 3 files changed, 50 insertions(+), 11 deletions(-)

-- 
1.7.5.252.g565191

[PATCH 1/3] t3701: Editing a split hunk in an "add -p" session

[Junio C Hamano]

Arnaud Lacombe reported that with the recent change to reject overlapping
hunks fed to "git apply", the edit mode of an "add -p" session that lazily
feeds overlapping hunks without coalescing adjacent ones claim that the
patch does not apply.  Expose the problem to be fixed.

Cf. http://thread.gmane.org/gmane.comp.version-control.git/170685/focus=171000

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t3701-add-interactive.sh |   36 ++++++++++++++++++++++++++++++++++++
 1 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index d6327e7..72559e1 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -295,4 +295,40 @@ test_expect_success PERL 'deleting an empty file' '
 	test_cmp expected diff
 '
 
+test_expect_success PERL 'split hunk setup' '
+	git reset --hard &&
+	for i in 10 20 30 40 50 60
+	do
+		echo $i
+	done >test &&
+	git add test &&
+	test_tick &&
+	git commit -m test &&
+
+	for i in 10 15 20 21 22 23 24 30 40 50 60
+	do
+		echo $i
+	done >test
+'
+
+test_expect_failure PERL 'split hunk "add -p (edit)"' '
+	# Split, say Edit and do nothing.  Then:
+	#
+	# 1. Broken version results in a patch that does not apply and
+	# only takes [y/n] (edit again) so the first q is discarded
+	# and then n attempts to discard the edit. Repeat q enough
+	# times to get out.
+	#
+	# 2. Correct version applies the (not)edited version, and asks
+	#    about the next hunk, against wich we say q and program
+	#    exits.
+	for a in s e     q n q q
+	do
+		echo $a
+	done |
+	EDITOR=: git add -p &&
+	git diff >actual &&
+	! grep "^+15" actual
+'
+
 test_done
-- 
1.7.5.252.g565191

[PATCH 2/3] add--interactive.perl: factor out repeated --recount option

[Junio C Hamano]

Depending on the direction and the target of patch application, we would
need to pass --cached and --reverse to underlying "git apply".  Also we
only pass --check when we are not applying but just checking.

But we always pass --recount since 8cbd431 (git-add--interactive: replace
hunk recounting with apply --recount, 2008-07-02).  Instead of repeating
the same --recount over and over again, move it to a single place that
actually runs the command, namely, "run_git_apply" subroutine.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 git-add--interactive.perl |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 4fb8cd0..fced0ce 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -705,7 +705,7 @@ sub add_untracked_cmd {
 sub run_git_apply {
 	my $cmd = shift;
 	my $fh;
-	open $fh, '| git ' . $cmd;
+	open $fh, '| git ' . $cmd . " --recount";
 	print $fh @_;
 	return close $fh;
 }
@@ -1050,7 +1050,7 @@ sub edit_hunk_manually {
 
 sub diff_applies {
 	my $fh;
-	return run_git_apply($patch_mode_flavour{APPLY_CHECK} . ' --recount --check',
+	return run_git_apply($patch_mode_flavour{APPLY_CHECK} . ' --check',
 			     map { @{$_->{TEXT}} } @_);
 }
 
@@ -1139,7 +1139,7 @@ sub help_patch_cmd {
 
 sub apply_patch {
 	my $cmd = shift;
-	my $ret = run_git_apply $cmd . ' --recount', @_;
+	my $ret = run_git_apply $cmd, @_;
 	if (!$ret) {
 		print STDERR @_;
 	}
@@ -1148,17 +1148,17 @@ sub apply_patch {
 
 sub apply_patch_for_checkout_commit {
 	my $reverse = shift;
-	my $applies_index = run_git_apply 'apply '.$reverse.' --cached --recount --check', @_;
-	my $applies_worktree = run_git_apply 'apply '.$reverse.' --recount --check', @_;
+	my $applies_index = run_git_apply 'apply '.$reverse.' --cached --check', @_;
+	my $applies_worktree = run_git_apply 'apply '.$reverse.' --check', @_;
 
 	if ($applies_worktree && $applies_index) {
-		run_git_apply 'apply '.$reverse.' --cached --recount', @_;
-		run_git_apply 'apply '.$reverse.' --recount', @_;
+		run_git_apply 'apply '.$reverse.' --cached', @_;
+		run_git_apply 'apply '.$reverse, @_;
 		return 1;
 	} elsif (!$applies_index) {
 		print colored $error_color, "The selected hunks do not apply to the index!\n";
 		if (prompt_yesno "Apply them to the worktree anyway? ") {
-			return run_git_apply 'apply '.$reverse.' --recount', @_;
+			return run_git_apply 'apply '.$reverse, @_;
 		} else {
 			print colored $error_color, "Nothing was applied.\n";
 			return 0;
-- 
1.7.5.252.g565191

[PATCH 3/3] "add -p": work-around an old laziness that does not coalesce hunks

[Junio C Hamano]

Since 0beee4c (git-add--interactive: remove hunk coalescing, 2008-07-02),
"git add--interactive" behaves lazily and passes overlapping hunks to the
underlying "git apply" without coalescing.  This was partially corrected
by 7a26e65 (its partial revert, 2009-05-16), but overlapping hunks are
still passed when the patch is edited.

Teach "git apply" the --allow-overlap option that disables a safety
feature that avoids misapplication of patches by not applying patches
to overlapping hunks, and pass this option form "add -p" codepath.

Do not even advertise the option, as this is merely a workaround, and the
correct fix is to make "add -p" correctly coalesce adjacent patch hunks.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/apply.c           |    9 ++++++---
 git-add--interactive.perl |    2 +-
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 04f56f8..8be1ce5 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -43,6 +43,7 @@ static int apply = 1;
 static int apply_in_reverse;
 static int apply_with_reject;
 static int apply_verbosely;
+static int allow_overlap;
 static int no_add;
 static const char *fake_ancestor;
 static int line_termination = '\n';
@@ -2430,9 +2431,9 @@ static void update_image(struct image *img,
 	memcpy(img->line + applied_pos,
 	       postimage->line,
 	       postimage->nr * sizeof(*img->line));
-	for (i = 0; i < postimage->nr; i++)
-		img->line[applied_pos + i].flag |= LINE_PATCHED;
-
+	if (!allow_overlap)
+		for (i = 0; i < postimage->nr; i++)
+			img->line[applied_pos + i].flag |= LINE_PATCHED;
 	img->nr = nr;
 }
 
@@ -3877,6 +3878,8 @@ int cmd_apply(int argc, const char **argv, const char *prefix_)
 			"don't expect at least one line of context"),
 		OPT_BOOLEAN(0, "reject", &apply_with_reject,
 			"leave the rejected hunks in corresponding *.rej files"),
+		OPT_BOOLEAN(0, "allow-overlap", &allow_overlap,
+			"allow overlapping hunks"),
 		OPT__VERBOSE(&apply_verbosely, "be verbose"),
 		OPT_BIT(0, "inaccurate-eof", &options,
 			"tolerate incorrectly detected missing new-line at the end of file",
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index fced0ce..4f08fe7 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -705,7 +705,7 @@ sub add_untracked_cmd {
 sub run_git_apply {
 	my $cmd = shift;
 	my $fh;
-	open $fh, '| git ' . $cmd . " --recount";
+	open $fh, '| git ' . $cmd . " --recount --allow-overlap";
 	print $fh @_;
 	return close $fh;
 }
-- 
1.7.5.252.g565191