git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: git-apply fails on creating a new file, with both -p and --directory specified

From
Jeff King <peff@peff.net>
Date
Dec 8, 2009, 07:49 UTC
Message-ID
<20091208074935.GB12049@coredump.intra.peff.net>
In-Reply-To
<7v3a3lorge.fsf@alter.siamese.dyndns.org>
On Mon, Dec 07, 2009 at 11:28:01PM -0800, Junio C Hamano wrote:
Show 10 quoted lines
> That might be something we may want to fix someday, when we find ourselves
> needing to add a feature to turn deletion into non-deletion or vice versa
> during "add -p" [e]dit, as I suspect that the "hunk editing" codepath does
> not keep track of what the user's patch is doing, to the point that it
> does not even know how many lines there are supposed to be in the
> resulting hunk that it asks "git apply" to recount.  There is no way to
> add/delete "deleted file" line if the logic does not know what the patch
> is doing.
> 
> But someday is not today.  I think this six-liner is preferable.

OK, here it is with the test and an amended commit message. You could almost do an [e]dit on this and delete the "deleted" line, but you have no way of fixing up the "+++ /dev/null" line. For now, we have disabled [e]dit entirely for non-content hunks, so at least you cannot get yourself into trouble creating a broken patch. :)

-- >8 --
Subject: [PATCH] add-interactive: fix deletion of non-empty files

Commit 24ab81a fixed the deletion of empty files, but broke deletion of non-empty files. The approach it took was to factor out the "deleted" line from the patch header into its own hunk, the same way we do for mode changes. However, unlike mode changes, we only showed the special "delete this file" hunk if there were no other hunks. Otherwise, the user would annoyingly be presented with _two_ hunks: one for deleting the file and one for deleting the content.

This meant that in the non-empty case, we forgot about the deleted line entirely, and we submitted a bogus patch to git-apply (with "/dev/null" as the destination file, but not marked as a deletion).

Instead, this patch combines the file deletion hunk and the content deletion hunk (if there is one) into a single deletion hunk which is either staged or not.

Signed-off-by: Jeff King <peff@peff.net>
---
 git-add--interactive.perl  |    6 +++++-
 t/t3701-add-interactive.sh |   20 ++++++++++++++++++++
 2 files changed, 25 insertions(+), 1 deletions(-)
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 35f4ef1..02e97b9 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -1217,7 +1217,11 @@ sub patch_update_file {
 	if (@{$mode->{TEXT}}) {
 		unshift @hunk, $mode;
 	}
-	if (@{$deletion->{TEXT}} && !@hunk) {
+	if (@{$deletion->{TEXT}}) {
+		foreach my $hunk (@hunk) {
+			push @{$deletion->{TEXT}}, @{$hunk->{TEXT}};
+			push @{$deletion->{DISPLAY}}, @{$hunk->{DISPLAY}};
+		}
 		@hunk = ($deletion);
 	}
 
diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index aa5909b..0926b91 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -215,6 +215,26 @@ test_expect_success 'add first line works' '
 '
 
 cat >expected <<EOF
+diff --git a/non-empty b/non-empty
+deleted file mode 100644
+index d95f3ad..0000000
+--- a/non-empty
++++ /dev/null
+@@ -1 +0,0 @@
+-content
+EOF
+test_expect_success 'deleting a non-empty file' '
+	git reset --hard &&
+	echo content >non-empty &&
+	git add non-empty &&
+	git commit -m non-empty &&
+	rm non-empty &&
+	echo y | git add -p non-empty &&
+	git diff --cached >diff &&
+	test_cmp expected diff
+'
+
+cat >expected <<EOF
 diff --git a/empty b/empty
 deleted file mode 100644
 index e69de29..0000000
-- 
1.6.5.1.g24ab.dirty
Previous: Junio C HamanoNext: Junio C Hamano
Message 10 of 14 in “git-apply fails on creating a new file, with both -p and --directory specified”
  1. Steven J. MurdochNov 23, 2009
  2. Junio C HamanoNov 25, 2009
  3. James VegaDec 7, 2009
  4. Junio C HamanoDec 8, 2009
  5. Junio C HamanoDec 8, 2009
  6. Jeff KingDec 8, 2009
  7. Jeff KingDec 8, 2009
  8. James VegaDec 8, 2009
  9. Junio C HamanoDec 8, 2009
  10. Jeff KingDec 8, 2009
  11. Junio C HamanoDec 8, 2009
  12. Junio C HamanoDec 8, 2009
  13. Jeff KingDec 8, 2009
  14. James VegaDec 8, 2009

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.