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

[PATCH/FIXUP 5/2] apply: localize the CRLF business to read_old_data()

From
Junio C Hamano <gitster@pobox.com>
Date
Aug 16, 2017, 18:30 UTC
Message-ID
<xmqqinhnl61g.fsf@gitster.mtv.corp.google.com>
In-Reply-To
<20170813085106.1731-1-tboegi@web.de>

Previous changes passed a new APPLY_FLAGS_CR_AT_EOL option down from load_preimage() to read_old_data(), because the last function in that callchain needs to decide how its call to convert_to_git() function is made on the data read from the working tree.

The load_preimage() function and its direct callees, however, are not limited to the case where the patch is applied to the data in the working tree (i.e. "git apply" that is working as a better "patch -p1"), unlike read_old_data(), which deals only with the patch target in the working tree. They are also responsible for driving "git apply --cached" and "git apply --index", both of which take the current index contents into account and do not need the new special-casing of CRLF. Exposing APPLY_FLAGS_CR_AT_EOL bit to them is misleading.

Instead, just pass the "struct patch" down the same callchain, and have read_old_data() look at its crlf_in_old member to make the necessary decision.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 This is what I care about the most in these fix-ups.
 apply.c | 30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)
diff --git a/apply.c b/apply.c
index 995973da3d..c06f7014a2 100644
--- a/apply.c
+++ b/apply.c
@@ -2287,12 +2287,12 @@ static void show_stats(struct apply_state *state, struct patch *patch)
 		add, pluses, del, minuses);
 }
 
-#define APPLY_FLAGS_CR_AT_EOL   (1<<0)
-
-static int read_old_data(struct stat *st, const char *path, struct strbuf *buf, int flags)
+static int read_old_data(struct stat *st, const char *path, struct strbuf *buf,
+			 struct patch *patch)
 {
-	enum safe_crlf safe_crlf = flags & APPLY_FLAGS_CR_AT_EOL ?
-		SAFE_CRLF_KEEP_CRLF : SAFE_CRLF_FALSE;
+	enum safe_crlf safe_crlf = (patch->crlf_in_old
+				    ? SAFE_CRLF_KEEP_CRLF : SAFE_CRLF_FALSE);
+
 	switch (st->st_mode & S_IFMT) {
 	case S_IFLNK:
 		if (strbuf_readlink(buf, path, st->st_size) < 0)
@@ -3407,9 +3407,9 @@ static int load_patch_target(struct apply_state *state,
 			     struct strbuf *buf,
 			     const struct cache_entry *ce,
 			     struct stat *st,
+			     struct patch *patch,
 			     const char *name,
-			     unsigned expected_mode,
-			     int flags)
+			     unsigned expected_mode)
 {
 	if (state->cached || state->check_index) {
 		if (read_file_or_gitlink(ce, buf))
@@ -3423,7 +3423,7 @@ static int load_patch_target(struct apply_state *state,
 		} else if (has_symlink_leading_path(name, strlen(name))) {
 			return error(_("reading from '%s' beyond a symbolic link"), name);
 		} else {
-			if (read_old_data(st, name, buf, flags))
+			if (read_old_data(st, name, buf, patch))
 				return error(_("failed to read %s"), name);
 		}
 	}
@@ -3447,7 +3447,6 @@ static int load_preimage(struct apply_state *state,
 	char *img;
 	struct patch *previous;
 	int status;
-	int flags = patch->crlf_in_old ? APPLY_FLAGS_CR_AT_EOL : 0;
 
 	previous = previous_patch(state, patch, &status);
 	if (status)
@@ -3457,8 +3456,8 @@ static int load_preimage(struct apply_state *state,
 		/* We have a patched copy in memory; use that. */
 		strbuf_add(&buf, previous->result, previous->resultsize);
 	} else {
-		status = load_patch_target(state, &buf, ce, st,
-					   patch->old_name, patch->old_mode, flags);
+		status = load_patch_target(state, &buf, ce, st, patch,
+					   patch->old_name, patch->old_mode);
 		if (status < 0)
 			return status;
 		else if (status == SUBMODULE_PATCH_WITHOUT_INDEX) {
@@ -3518,8 +3517,7 @@ static int three_way_merge(struct image *image,
  */
 static int load_current(struct apply_state *state,
 			struct image *image,
-			struct patch *patch,
-			int flags)
+			struct patch *patch)
 {
 	struct strbuf buf = STRBUF_INIT;
 	int status, pos;
@@ -3546,7 +3544,7 @@ static int load_current(struct apply_state *state,
 	if (verify_index_match(ce, &st))
 		return error(_("%s: does not match index"), name);
 
-	status = load_patch_target(state, &buf, ce, &st, name, mode, flags);
+	status = load_patch_target(state, &buf, ce, &st, patch, name, mode);
 	if (status < 0)
 		return status;
 	else if (status)
@@ -3597,8 +3595,7 @@ static int try_threeway(struct apply_state *state,
 
 	/* our_oid is ours */
 	if (patch->is_new) {
-		int flags = 0;
-		if (load_current(state, &tmp_image, patch, flags))
+		if (load_current(state, &tmp_image, patch))
 			return error(_("cannot read the current contents of '%s'"),
 				     patch->new_name);
 	} else {
@@ -3644,6 +3641,7 @@ static int apply_data(struct apply_state *state, struct patch *patch,
 		      struct stat *st, const struct cache_entry *ce)
 {
 	struct image image;
+
 	if (load_preimage(state, &image, patch, st, ce) < 0)
 		return -1;
 

-- 
2.14.1-331-g7631d96230
Previous: Junio C HamanoNext: Junio C Hamano
Message 30 of 36 in “core.autocrlf=true causes `git apply` to fail on patch generated with `git diff-index HEAD --patch`”
  1. Anthony SottileAug 1, 2017
  2. Torsten BögershausenAug 1, 2017
  3. Anthony SottileAug 1, 2017
  4. Torsten BögershausenAug 2, 2017
  5. 1/1 correct apply for files commited with CRLFtboegi@web.de, Aug 2, 2017
  6. Junio C HamanoAug 2, 2017
  7. Torsten BögershausenAug 4, 2017
  8. Junio C HamanoAug 4, 2017
  9. Junio C HamanoAug 4, 2017
  10. Junio C HamanoAug 2, 2017
  11. Torsten BögershausenAug 12, 2017
  12. Torsten BögershausenAug 12, 2017
  13. convert: Add SAFE_CRLF_KEEP_CRLFtboegi@web.de, Aug 12, 2017
  14. File commited with CRLF should roundtrip diff and applytboegi@web.de, Aug 12, 2017
  15. Junio C HamanoAug 14, 2017
  16. 1/2 convert: Add SAFE_CRLF_KEEP_CRLFtboegi@web.de, Aug 17, 2017
  17. 2/2 File commited with CRLF should roundtrip diff and applytboegi@web.de, Aug 17, 2017
  18. Junio C HamanoAug 17, 2017
  19. 1/2 convert: Add SAFE_CRLF_KEEP_CRLFtboegi@web.de, Aug 17, 2017
  20. 2/2 File commited with CRLF should roundtrip diff and applytboegi@web.de, Aug 17, 2017
  21. Junio C HamanoAug 17, 2017
  22. Junio C HamanoAug 17, 2017
  23. 1/2 convert: Add SAFE_CRLF_KEEP_CRLFtboegi@web.de, Aug 19, 2017
  24. 2/2 File commited with CRLF should roundtrip diff and applytboegi@web.de, Aug 19, 2017
  25. 1/2 convert: Add SAFE_CRLF_KEEP_CRLFtboegi@web.de, Aug 13, 2017
  26. 2/2 File commited with CRLF should roundtrip diff and applytboegi@web.de, Aug 13, 2017
  27. 3/2 apply: remove unused member apply_state.flagsJunio C Hamano, Aug 16, 2017
  28. Junio C HamanoAug 16, 2017
  29. 4/2 apply: only pay attention to CRLF in the preimageJunio C Hamano, Aug 16, 2017
  30. 5/2 apply: localize the CRLF business to read_old_data()Junio C Hamano, Aug 16, 2017
  31. 6/2 apply: clarify read_old_data() is about no-index caseJunio C Hamano, Aug 16, 2017
  32. Torsten BögershausenAug 17, 2017
  33. Junio C HamanoAug 17, 2017
  34. Junio C HamanoAug 17, 2017
  35. Torsten BögershausenAug 17, 2017
  36. Junio C HamanoAug 17, 2017

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.