diff mbox series

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

Message ID 815af5d30ebd5e7f80aa42e4a54808af2e3781e0.1614905738.git.gitgitgadget@gmail.com (mailing list archive)
State New
Headers show
Series Complete merge-ort implementation...almost | expand

Commit Message

Elijah Newren March 5, 2021, 12:55 a.m. UTC
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(+)

Comments

Ævar Arnfjörð Bjarmason March 8, 2021, 12:46 p.m. UTC | #1
On Fri, Mar 05 2021, Elijah Newren via GitGitGadget wrote:

> 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.

> +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.

> +	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 {

> +		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.

> +			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);
> +		}
> +	}
> +}
> +
diff mbox series

Patch

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
+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;
+	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 {
+		int stage, len;
+		struct conflict_info *ci;
+
+		ASSIGN_AND_VERIFY_CI(ci, mi);
+		for (stage=0; stage<3; ++stage) {
+			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);
+		}
+	}
+}
+
 static int merge_3way(struct merge_options *opt,
 		      const char *path,
 		      const struct object_id *o,