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

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

From
Joey Hess <joeyh@joeyh.name>
Date
Jun 22, 2016, 20:21 UTC
Message-ID
<20160622202133.23565-7-joeyh@joeyh.name>
In-Reply-To
<20160622202133.23565-1-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               | 55 ++++++++++++++++++++++++++++++++-------------------
 t/t0021-conversion.sh | 24 ++++++++++++++++++++++
 2 files changed, 59 insertions(+), 20 deletions(-)
diff --git a/entry.c b/entry.c
index 97975e5..8322127 100644
--- a/entry.c
+++ b/entry.c
@@ -181,12 +181,6 @@ static int write_entry(struct cache_entry *ce,
 		int regular_file = ce_mode_s_ifmt == S_IFREG;
 		int 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) {
@@ -194,7 +188,42 @@ 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)) {
+				smudge_to_file = 0;
+				/* 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);
+				}
+			}
+			else {
+				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);
+			}
+		}
+
 		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);
@@ -203,20 +232,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 cba03fd..c0b4709 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 &&
+	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 &&
+	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.9.0.8.g973eabb.dirty
Previous: Joey HessNext: Joey Hess
Message 2 of 9 in “extend smudge/clean filters with direct file access”
  1. 0/8 extend smudge/clean filters with direct file accessJoey Hess, Jun 22, 2016
  2. 6/8 better recovery from failure of smudgeToFile filterJoey Hess, Jun 22, 2016
  3. 1/8 clarify %f documentationJoey Hess, Jun 22, 2016
  4. 2/8 add smudgeToFile and cleanFromFile filter configsJoey Hess, Jun 22, 2016
  5. 3/8 use cleanFromFile in git addJoey Hess, Jun 22, 2016
  6. 8/8 use smudgeToFile filter in recursive mergeJoey Hess, Jun 22, 2016
  7. 5/8 warn on unusable smudgeToFile/cleanFromFile configJoey Hess, Jun 22, 2016
  8. 4/8 use smudgeToFile in git checkout etcJoey Hess, Jun 22, 2016
  9. 7/8 use smudgeToFile filter in git amJoey Hess, Jun 22, 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.