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

Re: [PATCH 03/11] merge-ort: add a function for initializing our special attr_index

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Mar 8, 2021, 12:46 UTC
Message-ID
<87czw9ke2m.fsf@evledraar.gmail.com>
In-Reply-To
<815af5d30ebd5e7f80aa42e4a54808af2e3781e0.1614905738.git.gitgitgadget@gmail.com>
On Fri, Mar 05 2021, Elijah Newren via GitGitGadget wrote:
Show 48 quoted lines
> From: Elijah Newren <newren@gmail.com>
>
> Add a function which can be called to populate the attr_index with the
> appropriate .gitattributes contents when necessary.  Make it return
> early if the attr_index is already initialized or if we are not
> renormalizing files.
>
> NOTE 1: Even if the user has a working copy or a real index (which is
> not a given as merge-ort can be used in bare repositories), we
> explicitly ignore any .gitattributes file from either of these
> locations.  merge-ort can be used to merge two branches that are
> unrelated to HEAD, so .gitattributes from the working copy and current
> index should not be considered relevant.
>
> NOTE 2: Since we are in the middle of merging, there is a risk that
> .gitattributes itself is conflicted...leaving us with an ill-defined
> situation about how to perform the rest of the merge.  It could be that
> the .gitattributes file does not even exist on one of the sides of the
> merge, or that it has been modified on both sides.  If it's been
> modified on both sides, it's possible that it could itself be merged
> cleanly, though it's also possible that it only merges cleanly if you
> use the right version of the .gitattributes file to drive the merge.  It
> gets kind of complicated.  The only test we ever had that attempted to
> test behavior in this area was seemingly unaware of the undefined
> behavior, but knew the test wouldn't work for lack of attribute handling
> support, marked it as test_expect_failure from the beginning, but
> managed to fail for several reasons unrelated to attribute handling.
> See commit 6f6e7cfb52 ("t6038: remove problematic test", 2020-08-03) for
> details.  So there are probably various ways to improve what
> initialize_attr_index() picks in the case of a conflicted .gitattributes
> but for now I just implemented something simple -- look for whatever
> .gitattributes file we can find in any of the higher order stages and
> use it.
>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  merge-ort.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 61 insertions(+)
>
> diff --git a/merge-ort.c b/merge-ort.c
> index d91b66a052b6..028d1adcd2c9 100644
> --- a/merge-ort.c
> +++ b/merge-ort.c
> @@ -988,6 +988,67 @@ static int merge_submodule(struct merge_options *opt,
>  	return 0;
>  }
>  
> +MAYBE_UNUSED

As with the lst series you had I also think this is better squashed with 04/11.

Show 17 quoted lines
> +static void initialize_attr_index(struct merge_options *opt)
> +{
> +	/*
> +	 * The renormalize_buffer() functions require attributes, and
> +	 * annoyingly those can only be read from the working tree or from
> +	 * an index_state.  merge-ort doesn't have an index_state, so we
> +	 * generate a fake one containing only attribute information.
> +	 */
> +	struct merged_info *mi;
> +	struct index_state *attr_index = &opt->priv->attr_index;
> +	struct cache_entry *ce;
> +
> +	if (!opt->renormalize)
> +		return;
> +
> +	if (attr_index->initialized)
> +		return;
Will comment on this in 04/11.
Show 19 quoted lines
> +	attr_index->initialized = 1;
> +
> +	mi = strmap_get(&opt->priv->paths, GITATTRIBUTES_FILE);
> +	if (!mi)
> +		return;
> +
> +	if (mi->clean) {
> +		int len = strlen(GITATTRIBUTES_FILE);
> +		ce = make_empty_cache_entry(attr_index, len);
> +		ce->ce_mode = create_ce_mode(mi->result.mode);
> +		ce->ce_flags = create_ce_flags(0);
> +		ce->ce_namelen = len;
> +		oidcpy(&ce->oid, &mi->result.oid);
> +		memcpy(ce->name, GITATTRIBUTES_FILE, len);
> +		add_index_entry(attr_index, ce,
> +				ADD_CACHE_OK_TO_ADD | ADD_CACHE_OK_TO_REPLACE);
> +		get_stream_filter(attr_index, GITATTRIBUTES_FILE, &ce->oid);
> +	}
> +	else {
Style nit: } else {
Show 5 quoted lines
> +		int stage, len;
> +		struct conflict_info *ci;
> +
> +		ASSIGN_AND_VERIFY_CI(ci, mi);
> +		for (stage=0; stage<3; ++stage) {
Style nit: stage < 3

Style nit: I find just stage++ to be more readable in for-loops, makes no difference to the compiler, just more idiomatic.

Show 19 quoted lines
> +			unsigned stage_mask = (1 << stage);
> +
> +			if (!(ci->filemask & stage_mask))
> +				continue;
> +			len = strlen(GITATTRIBUTES_FILE);
> +			ce = make_empty_cache_entry(attr_index, len);
> +			ce->ce_mode = create_ce_mode(ci->stages[stage].mode);
> +			ce->ce_flags = create_ce_flags(stage);
> +			ce->ce_namelen = len;
> +			oidcpy(&ce->oid, &ci->stages[stage].oid);
> +			memcpy(ce->name, GITATTRIBUTES_FILE, len);
> +			add_index_entry(attr_index, ce,
> +					ADD_CACHE_OK_TO_ADD | ADD_CACHE_OK_TO_REPLACE);
> +			get_stream_filter(attr_index, GITATTRIBUTES_FILE,
> +					  &ce->oid);
> +		}
> +	}
> +}
> +
Previous: Elijah Newren via GitGitGadgetNext: Elijah Newren via GitGitGadget
Message 5 of 44 in “Complete merge-ort implementation...almost”
  1. 00/11 Complete merge-ort implementation...almostElijah Newren via GitGitGadget, Mar 5, 2021
  2. 01/11 merge-ort: use STABLE_QSORT instead of QSORT where requiredElijah Newren via GitGitGadget, Mar 5, 2021
  3. 02/11 merge-ort: add a special minimal index just for renormalizationElijah Newren via GitGitGadget, Mar 5, 2021
  4. 03/11 merge-ort: add a function for initializing our special attr_indexElijah Newren via GitGitGadget, Mar 5, 2021
  5. Ævar Arnfjörð BjarmasonMar 8, 2021
  6. 04/11 merge-ort: have ll_merge() calls use the attr_index for renormalizationElijah Newren via GitGitGadget, Mar 5, 2021
  7. Ævar Arnfjörð BjarmasonMar 8, 2021
  8. Elijah NewrenMar 9, 2021
  9. 05/11 merge-ort: let renormalization change modify/delete into clean deleteElijah Newren via GitGitGadget, Mar 5, 2021
  10. Ævar Arnfjörð BjarmasonMar 8, 2021
  11. 06/11 merge-ort: support subtree shiftingElijah Newren via GitGitGadget, Mar 5, 2021
  12. 07/11 t6428: new test for SKIP_WORKTREE handling and conflictsElijah Newren via GitGitGadget, Mar 5, 2021
  13. Ævar Arnfjörð BjarmasonMar 8, 2021
  14. Elijah NewrenMar 8, 2021
  15. 08/11 merge-ort: implement CE_SKIP_WORKTREE handling with conflicted entriesElijah Newren via GitGitGadget, Mar 5, 2021
  16. Ævar Arnfjörð BjarmasonMar 8, 2021
  17. Elijah NewrenMar 8, 2021
  18. 09/11 t: mark several submodule merging tests as fixed under merge-ortElijah Newren via GitGitGadget, Mar 5, 2021
  19. Ævar Arnfjörð BjarmasonMar 8, 2021
  20. 10/11 merge-ort: write $GIT_DIR/AUTO_MERGE whenever we hit a conflictElijah Newren via GitGitGadget, Mar 5, 2021
  21. Ævar Arnfjörð BjarmasonMar 8, 2021
  22. Elijah NewrenMar 8, 2021
  23. 11/11 merge-recursive: add a bunch of FIXME comments documenting known bugsElijah Newren via GitGitGadget, Mar 5, 2021
  24. Ævar Arnfjörð BjarmasonMar 8, 2021
  25. Ævar Arnfjörð BjarmasonMar 8, 2021
  26. Elijah NewrenMar 8, 2021
  27. 00/10 Complete merge-ort implementation...almostElijah Newren via GitGitGadget, Mar 9, 2021
  28. 01/10 merge-ort: use STABLE_QSORT instead of QSORT where requiredElijah Newren via GitGitGadget, Mar 9, 2021
  29. 02/10 merge-ort: add a special minimal index just for renormalizationElijah Newren via GitGitGadget, Mar 9, 2021
  30. Derrick StoleeMar 11, 2021
  31. 05/10 merge-ort: support subtree shiftingElijah Newren via GitGitGadget, Mar 9, 2021
  32. 04/10 merge-ort: let renormalization change modify/delete into clean deleteElijah Newren via GitGitGadget, Mar 9, 2021
  33. 06/10 t6428: new test for SKIP_WORKTREE handling and conflictsElijah Newren via GitGitGadget, Mar 9, 2021
  34. Derrick StoleeMar 11, 2021
  35. 03/10 merge-ort: have ll_merge() use a special attr_index for renormalizationElijah Newren via GitGitGadget, Mar 9, 2021
  36. Derrick StoleeMar 11, 2021
  37. 07/10 merge-ort: implement CE_SKIP_WORKTREE handling with conflicted entriesElijah Newren via GitGitGadget, Mar 9, 2021
  38. 09/10 merge-ort: write $GIT_DIR/AUTO_MERGE whenever we hit a conflictElijah Newren via GitGitGadget, Mar 9, 2021
  39. 08/10 t: mark several submodule merging tests as fixed under merge-ortElijah Newren via GitGitGadget, Mar 9, 2021
  40. Derrick StoleeMar 11, 2021
  41. 10/10 merge-recursive: add a bunch of FIXME comments documenting known bugsElijah Newren via GitGitGadget, Mar 9, 2021
  42. Derrick StoleeMar 11, 2021
  43. Elijah NewrenMar 11, 2021
  44. Elijah NewrenMar 17, 2021

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.