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

Re: [PATCH 28/48] merge-recursive: Split update_stages_and_entry; only update stages at end

From
Junio C Hamano <gitster@pobox.com>
Date
Jul 18, 2011, 23:39 UTC
Message-ID
<7vvcuzb1kp.fsf@alter.siamese.dyndns.org>
In-Reply-To
<1307518278-23814-29-git-send-email-newren@gmail.com>
Elijah Newren <newren@gmail.com> writes:
Show 55 quoted lines
> Instead of having the process_renames logic update the stages in the index
> for the rename destination, have the index updated after process_entry or
> process_df_entry.  This will also allow us to have process_entry determine
> whether a file was tracked and existed in the working copy before the
> merge started.
>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  merge-recursive.c |   35 +++++++++++++++++------------------
>  1 files changed, 17 insertions(+), 18 deletions(-)
>
> diff --git a/merge-recursive.c b/merge-recursive.c
> index b4baa35..7878b30 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -90,6 +90,7 @@ struct stage_data {
>  	} stages[4];
>  	struct rename_df_conflict_info *rename_df_conflict_info;
>  	unsigned processed:1;
> +	unsigned involved_in_rename:1;
>  };
>  
>  static inline void setup_rename_df_conflict_info(enum rename_type rename_type,
> @@ -516,15 +517,11 @@ static int update_stages(const char *path, const struct diff_filespec *o,
>  	return 0;
>  }
>  
> -static int update_stages_and_entry(const char *path,
> -				   struct stage_data *entry,
> -				   struct diff_filespec *o,
> -				   struct diff_filespec *a,
> -				   struct diff_filespec *b,
> -				   int clear)
> +static void update_entry(struct stage_data *entry,
> +			 struct diff_filespec *o,
> +			 struct diff_filespec *a,
> +			 struct diff_filespec *b)
>  {
> -	int options;
> -
>  	entry->processed = 0;
>  	entry->stages[1].mode = o->mode;
>  	entry->stages[2].mode = a->mode;
> @@ -532,7 +529,6 @@ static int update_stages_and_entry(const char *path,
>  	hashcpy(entry->stages[1].sha, o->sha1);
>  	hashcpy(entry->stages[2].sha, a->sha1);
>  	hashcpy(entry->stages[3].sha, b->sha1);
> -	return update_stages(path, o, a, b);
>  }
>  
>  static int remove_file(struct merge_options *o, int clean,
> @@ -1084,12 +1080,11 @@ static int process_renames(struct merge_options *o,
>  							      ren2->dst_entry);
>  			} else {
>  				remove_file(o, 1, ren1_src, 1);

Hopefully this unconditional removal from the working tree will be fixed by the end of the series, at least for the virtual ancestor merges.

Show 11 quoted lines
> -				update_stages_and_entry(ren1_dst,
> -							ren1->dst_entry,
> -							ren1->pair->one,
> -							ren1->pair->two,
> -							ren2->pair->two,
> -							1 /* clear */);
> +				update_entry(ren1->dst_entry,
> +					     ren1->pair->one,
> +					     ren1->pair->two,
> +					     ren2->pair->two);
> +				ren1->dst_entry->involved_in_rename = 1;

So the idea is to just record what the stage data should be, and delay calling update_stages() until we run the content level merge?

Show 35 quoted lines
> @@ -1291,6 +1287,7 @@ static void handle_delete_modify(struct merge_options *o,
>  }
>  
>  static int merge_content(struct merge_options *o,
> +			 unsigned involved_in_rename,
>  			 const char *path,
>  			 unsigned char *o_sha, int o_mode,
>  			 unsigned char *a_sha, int a_mode,
> @@ -1331,6 +1328,8 @@ static int merge_content(struct merge_options *o,
>  			reason = "submodule";
>  		output(o, 1, "CONFLICT (%s): Merge conflict in %s",
>  				reason, path);
> +		if (involved_in_rename)
> +			update_stages(path, &one, &a, &b);
>  	}
>  
>  	if (df_conflict_remains) {
> @@ -1415,7 +1414,7 @@ static int process_entry(struct merge_options *o,
>  	} else if (a_sha && b_sha) {
>  		/* Case C: Added in both (check for same permissions) and */
>  		/* case D: Modified in both, but differently. */
> -		clean_merge = merge_content(o, path,
> +		clean_merge = merge_content(o, entry->involved_in_rename, path,
>  					    o_sha, o_mode, a_sha, a_mode, b_sha, b_mode,
>  					    NULL);
>  	} else if (!o_sha && !a_sha && !b_sha) {
> @@ -1459,7 +1458,7 @@ static int process_df_entry(struct merge_options *o,
>  		char *src;
>  		switch (conflict_info->rename_type) {
>  		case RENAME_NORMAL:
> -			clean_merge = merge_content(o, path,
> +			clean_merge = merge_content(o, entry->involved_in_rename, path,
>  						    o_sha, o_mode, a_sha, a_mode, b_sha, b_mode,
>  						    conflict_info->branch1);
>  			break;

Feels sane from a cursory look, but are there cases where we do update_entry() above and end up not calling merge_content() at all?

Previous: Elijah NewrenNext: Elijah Newren
Message 58 of 97 in “Handling more corner cases in merge-recursive.c”
  1. 00/48 Handling more corner cases in merge-recursive.cElijah Newren, Jun 8, 2011
  2. 01/48 t6039: Add a testcase where git deletes an untracked fileElijah Newren, Jun 8, 2011
  3. 02/48 t6039: Add failing testcase for rename/modify/add-source conflictElijah Newren, Jun 8, 2011
  4. 03/48 t6039: Add a pair of cases where undetected renames cause issuesElijah Newren, Jun 8, 2011
  5. 04/48 t6039: Add a testcase where undetected rename causes silent file deletionElijah Newren, Jun 8, 2011
  6. 05/48 t6039: Add tests for content issues with modify/rename/directory conflictsElijah Newren, Jun 8, 2011
  7. Junio C HamanoJul 18, 2011
  8. Elijah NewrenAug 8, 2011
  9. 06/48 t6039: Add failing testcases for rename/rename/add-{source,dest} conflictsElijah Newren, Jun 8, 2011
  10. Junio C HamanoJul 18, 2011
  11. 07/48 t6039: Ensure rename/rename conflicts leave index and workdir in sane stateElijah Newren, Jun 8, 2011
  12. Junio C HamanoJul 18, 2011
  13. Elijah NewrenAug 8, 2011
  14. 08/48 t6036: Add differently resolved modify/delete conflict in criss-cross testElijah Newren, Jun 8, 2011
  15. Junio C HamanoJul 18, 2011
  16. 09/48 t6036: criss-cross with weird content can fool git into clean mergeElijah Newren, Jun 8, 2011
  17. Junio C HamanoJul 18, 2011
  18. Elijah NewrenAug 8, 2011
  19. 10/48 t6036: tests for criss-cross merges with various directory/file conflictsElijah Newren, Jun 8, 2011
  20. Junio C HamanoJul 18, 2011
  21. Elijah NewrenAug 8, 2011
  22. 11/48 t6036: criss-cross w/ rename/rename(1to2)/modify+rename/rename(2to1)/modifyElijah Newren, Jun 8, 2011
  23. Junio C HamanoJul 18, 2011
  24. 12/48 t6036: criss-cross + rename/rename(1to2)/add-source + modify/modifyElijah Newren, Jun 8, 2011
  25. Junio C HamanoJul 18, 2011
  26. Phil HordJul 20, 2011
  27. 13/48 t6022: Remove unnecessary untracked files to make test cleanerElijah Newren, Jun 8, 2011
  28. 14/48 t6022: New tests checking for unnecessary updates of filesElijah Newren, Jun 8, 2011
  29. 15/48 t6022: Add testcase for merging a renamed file with a simple changeElijah Newren, Jun 8, 2011
  30. 16/48 merge-recursive: Make BUG message more legible by adding a newlineElijah Newren, Jun 8, 2011
  31. 17/48 merge-recursive: Correct a commentElijah Newren, Jun 8, 2011
  32. 18/48 merge-recursive: Mark some diff_filespec struct arguments constElijah Newren, Jun 8, 2011
  33. Junio C HamanoJul 18, 2011
  34. 19/48 merge-recursive: Remember to free generated unique path namesElijah Newren, Jun 8, 2011
  35. Junio C HamanoJul 18, 2011
  36. 20/48 merge-recursive: Avoid working directory changes during recursive caseElijah Newren, Jun 8, 2011
  37. 21/48 merge-recursive: Fix recursive case with D/F conflict via add/add conflictElijah Newren, Jun 8, 2011
  38. Junio C HamanoJul 18, 2011
  39. 22/48 merge-recursive: Fix sorting order and directory change assumptionsElijah Newren, Jun 8, 2011
  40. Johannes SixtJul 11, 2011
  41. Johannes SixtJul 12, 2011
  42. Johannes SixtJul 13, 2011
  43. Junio C HamanoJul 13, 2011
  44. Junio C HamanoJul 18, 2011
  45. Elijah NewrenAug 8, 2011
  46. 23/48 merge-recursive: Fix code checking for D/F conflicts still being presentElijah Newren, Jun 8, 2011
  47. 24/48 merge-recursive: Save D/F conflict filenames instead of unlinking themElijah Newren, Jun 8, 2011
  48. 25/48 merge-recursive: Split was_tracked() out of would_lose_untracked()Elijah Newren, Jun 8, 2011
  49. 26/48 merge-recursive: Allow make_room_for_path() to remove D/F entriesElijah Newren, Jun 8, 2011
  50. Johannes SixtJul 11, 2011
  51. Johannes SixtJul 13, 2011
  52. Elijah NewrenAug 8, 2011
  53. Johannes SixtAug 9, 2011
  54. Junio C HamanoJul 18, 2011
  55. 27/48 merge-recursive: Consolidate different update_stages functionsElijah Newren, Jun 8, 2011
  56. Junio C HamanoJul 18, 2011
  57. 28/48 merge-recursive: Split update_stages_and_entry; only update stages at endElijah Newren, Jun 8, 2011
  58. Junio C HamanoJul 18, 2011
  59. 29/48 merge-recursive: When we detect we can skip an update, actually skip itElijah Newren, Jun 8, 2011
  60. Junio C HamanoJul 18, 2011
  61. 30/48 merge-recursive: Fix deletion of untracked file in rename/delete conflictsElijah Newren, Jun 8, 2011
  62. Junio C HamanoJul 21, 2011
  63. 31/48 merge-recursive: Make dead code for rename/rename(2to1) conflicts undeadElijah Newren, Jun 8, 2011
  64. 32/48 merge-recursive: Add comments about handling rename/add-source casesElijah Newren, Jun 8, 2011
  65. 33/48 merge-recursive: Improve handling of rename target vs. directory additionElijah Newren, Jun 8, 2011
  66. 34/48 merge-recursive: Consolidate process_entry() and process_df_entry()Elijah Newren, Jun 8, 2011
  67. Junio C HamanoJul 21, 2011
  68. 35/48 merge-recursive: Cleanup and consolidation of rename_conflict_infoElijah Newren, Jun 8, 2011
  69. 36/48 merge-recursive: Provide more info in conflict markers with file renamesElijah Newren, Jun 8, 2011
  70. Junio C HamanoJul 21, 2011
  71. 37/48 merge-recursive: Fix modify/delete resolution in the recursive caseElijah Newren, Jun 8, 2011
  72. Junio C HamanoJul 21, 2011
  73. Elijah NewrenAug 8, 2011
  74. 38/48 merge-recursive: Introduce a merge_file convenience functionElijah Newren, Jun 8, 2011
  75. 39/48 merge-recursive: Fix rename/rename(1to2) resolution for virtual merge baseElijah Newren, Jun 8, 2011
  76. Junio C HamanoJul 25, 2011
  77. Elijah NewrenAug 8, 2011
  78. 40/48 merge-recursive: Small cleanups for conflict_rename_rename_1to2Elijah Newren, Jun 8, 2011
  79. 41/48 merge-recursive: Defer rename/rename(2to1) handling until process_entryElijah Newren, Jun 8, 2011
  80. 42/48 merge-recursive: Record more data needed for merging with dual renamesElijah Newren, Jun 8, 2011
  81. 43/48 merge-recursive: Create function for merging with branchname:file markersElijah Newren, Jun 8, 2011
  82. 44/48 merge-recursive: Consider modifications in rename/rename(2to1) conflictsElijah Newren, Jun 8, 2011
  83. 45/48 merge-recursive: Make modify/delete handling code reusableElijah Newren, Jun 8, 2011
  84. 46/48 merge-recursive: Have conflict_rename_delete reuse modify/delete codeElijah Newren, Jun 8, 2011
  85. 47/48 merge-recursive: add handling for rename/rename/add-dest/add-destElijah Newren, Jun 8, 2011
  86. 48/48 merge-recursive: Fix working copy handling for rename/rename/add/addElijah Newren, Jun 8, 2011
  87. Junio C HamanoJun 11, 2011
  88. Junio C HamanoAug 4, 2011
  89. Junio C HamanoAug 4, 2011
  90. Elijah NewrenAug 4, 2011
  91. Elijah NewrenAug 4, 2011
  92. Junio C HamanoAug 4, 2011
  93. Elijah NewrenAug 4, 2011
  94. Junio C HamanoAug 6, 2011
  95. Elijah NewrenAug 6, 2011
  96. Junio C HamanoJun 12, 2011
  97. Junio C HamanoJun 12, 2011

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.