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_UNUSEDAs 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); > + } > + } > +} > +