[PATCH v5 6/8] better recovery from failure of smudgeToFile filter

[Joey Hess <joeyh@joeyh.name>]

If the smudgeToFile filter fails, it can leave the worktree file with the
wrong content, or even deleted. Recover from this by falling back to
running the smudge filter.

Signed-off-by: Joey Hess <joeyh@joeyh.name>
---
 entry.c               | 66 ++++++++++++++++++++++++++++++++++-----------------
 t/t0021-conversion.sh | 24 +++++++++++++++++++
 2 files changed, 68 insertions(+), 22 deletions(-)

diff --git a/entry.c b/entry.c
index 81d12a1..7811e31 100644
--- a/entry.c
+++ b/entry.c
@@ -182,12 +182,6 @@ static int write_entry(struct cache_entry *ce,
 		regular_file = ce_mode_s_ifmt == S_IFREG;
 		smudge_to_file = regular_file
 			&& can_smudge_to_file(ce->name);
-		if (regular_file && !smudge_to_file &&
-		    convert_to_working_tree(ce->name, new, size, &buf)) {
-			free(new);
-			new = strbuf_detach(&buf, &newsize);
-			size = newsize;
-		}
 
 		fd = open_output_fd(path, ce, to_tempfile);
 		if (fd < 0) {
@@ -195,7 +189,51 @@ static int write_entry(struct cache_entry *ce,
 			return error_errno("unable to create file %s", path);
 		}
 
+		if (smudge_to_file) {
+			close(fd);
+			if (convert_to_working_tree_filter_to_file(ce->name, path, new, size)) {
+				free(new);
+				/*
+				 * The smudgeToFile filter may have replaced
+				 * or deleted the file; reopen it to make
+				 * sure that the file exists.
+				 */
+				fd = open(path, O_RDONLY);
+				if (fd < 0)
+					return error_errno("unable to create file %s", path);
+				if (!to_tempfile)
+					fstat_done = fstat_output(fd, state, &st);
+				close(fd);
+			}
+			else {
+				/*
+				 * The failing smudgeToFile filter may have
+				 * deleted or replaced the file; delete
+				 * the file and re-open for recovery write.
+				 */
+				unlink(path);
+				fd = open_output_fd(path, ce, to_tempfile);
+				if (fd < 0) {
+					free(new);
+					return error_errno("unable to create file %s", path);
+				}
+				/* Fall through to normal write below. */
+				smudge_to_file = 0;
+			}
+		}
+
+		/*
+		 * Not an else of above if (smudge_to_file) because the
+		 * smudgeToFile filter may fail and in that case this is
+		 * run to recover.
+		 */
 		if (!smudge_to_file) {
+			if (regular_file &&
+			    convert_to_working_tree(ce->name, new, size, &buf)) {
+				free(new);
+				new = strbuf_detach(&buf, &newsize);
+				size = newsize;
+			}
 			wrote = write_in_full(fd, new, size);
 			if (!to_tempfile)
 				fstat_done = fstat_output(fd, state, &st);
@@ -204,22 +242,6 @@ static int write_entry(struct cache_entry *ce,
 			if (wrote != size)
 				return error("unable to write file %s", path);
 		}
-		else {
-			close(fd);
-			convert_to_working_tree_filter_to_file(ce->name, path, new, size);
-			free(new);
-			/*
-			 * The smudgeToFile filter may have replaced the
-			 * file; open it to make sure that the file
-			 * exists.
-			 */
-			fd = open(path, O_RDONLY);
-			if (fd < 0)
-				return error_errno("unable to create file %s", path);
-			if (!to_tempfile)
-				fstat_done = fstat_output(fd, state, &st);
-			close(fd);
-		}
 		break;
 	case S_IFGITLINK:
 		if (to_tempfile)
diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index ea18b17..0efad9b 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -28,6 +28,14 @@ touch rot13-to-file.ran
 EOF
 chmod +x rot13-to-file.sh
 
+cat <<EOF >delete-file-and-fail.sh
+#!$SHELL_PATH
+destfile="\$1"
+rm -f "\$destfile"
+exit 1
+EOF
+chmod +x delete-file-and-fail.sh
+
 test_expect_success setup '
 	git config filter.rot13.smudge ./rot13.sh &&
 	git config filter.rot13.clean ./rot13.sh &&
@@ -310,6 +318,22 @@ test_expect_success 'smudgeToFile filter is used when checking out a file' '
 	rm -f rot13-to-file.ran
 '
 
+test_expect_success 'recovery from failure of smudgeToFile filter, using smudge filter' '
+	test_config filter.rot13.smudgeToFile false &&
+
+	rm -f fstest.t &&
+	git checkout -- fstest.t &&
+	test_cmp test fstest.t
+'
+
+test_expect_success 'recovery from failure of smudgeToFile filter that deletes the worktree file' '
+	test_config filter.rot13.smudgeToFile ./delete-file-and-fail.sh &&
+
+	rm -f fstest.t &&
+	git checkout -- fstest.t &&
+	test_cmp test fstest.t
+'
+
 test_expect_success 'cleanFromFile filter is not used when clean filter is not configured' '
 	test_config filter.noclean.smudge ./rot13.sh &&
 	test_config filter.noclean.cleanFromFile ./rot13-from-file.sh &&
-- 
2.8.1