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

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

From
Joey Hess <joeyh@joeyh.name>
Date
Jul 11, 2016, 22:45 UTC
Message-ID
<1468277112-9909-7-git-send-email-joeyh@joeyh.name>
In-Reply-To
<1468277112-9909-1-git-send-email-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
Previous: Joey HessNext: Junio C Hamano
Message 10 of 11 in “extend smudge/clean filters with direct file access (for pu)”
  1. 0/8 extend smudge/clean filters with direct file access (for pu)Joey Hess, Jul 11, 2016
  2. 1/8 clarify %f documentationJoey Hess, Jul 11, 2016
  3. 3/8 use cleanFromFile in git addJoey Hess, Jul 11, 2016
  4. 2/8 add smudgeToFile and cleanFromFile filter configsJoey Hess, Jul 11, 2016
  5. Lars SchneiderJul 13, 2016
  6. 5/8 warn on unusable smudgeToFile/cleanFromFile configJoey Hess, Jul 11, 2016
  7. 4/8 use smudgeToFile in git checkout etcJoey Hess, Jul 11, 2016
  8. 8/8 use smudgeToFile filter in recursive mergeJoey Hess, Jul 11, 2016
  9. 7/8 use smudgeToFile filter in git amJoey Hess, Jul 11, 2016
  10. 6/8 better recovery from failure of smudgeToFile filterJoey Hess, Jul 11, 2016
  11. Junio C HamanoJul 12, 2016

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.