| Message ID | 815af5d30ebd5e7f80aa42e4a54808af2e3781e0.1614905738.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Complete merge-ort implementation...almost | expand |
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 --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,