| Message ID | b70ef4d7000a1d83b1588a9be1e8f4273a382cc7.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> > > When we have a modify/delete conflict, but the only change to the > modification is e.g. change of line endings, then if renormalization is > requested then we should be able to recognize such a case as a > not-modified/delete and resolve the conflict automatically. > > This fixes t6418.10 under GIT_TEST_MERGE_ALGORITHM=ort. > > Signed-off-by: Elijah Newren <newren@gmail.com> > --- > merge-ort.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 61 insertions(+), 2 deletions(-) > > diff --git a/merge-ort.c b/merge-ort.c > index 87c553c0882c..c4bd88b9d3db 100644 > --- a/merge-ort.c > +++ b/merge-ort.c > @@ -2416,6 +2416,60 @@ static int string_list_df_name_compare(const char *one, const char *two) > return onelen - twolen; > } > > +static int read_oid_strbuf(struct merge_options *opt, > + const struct object_id *oid, > + struct strbuf *dst) > +{ > + void *buf; > + enum object_type type; > + unsigned long size; > + buf = read_object_file(oid, &type, &size); > + if (!buf) > + return err(opt, _("cannot read object %s"), oid_to_hex(oid)); > + if (type != OBJ_BLOB) { > + free(buf); > + return err(opt, _("object %s is not a blob"), oid_to_hex(oid)); As an aside I've got another series I'll submit soon which refactors all these "object is not xyz" calls to a utility function, so in this case we'd also say what it was other than a blob. Fine to keep this here, just a #leftoverbits note to myself to eventually migrate this. > + } > + strbuf_attach(dst, buf, size, size + 1); > + return 0; > +} > + > +static int blob_unchanged(struct merge_options *opt, > + const struct version_info *base, > + const struct version_info *side, > + const char *path) > +{ > + struct strbuf basebuf = STRBUF_INIT; > + struct strbuf sidebuf = STRBUF_INIT; > + int ret = 0; /* assume changed for safety */ > + const struct index_state *idx = &opt->priv->attr_index; > + > + initialize_attr_index(opt); > + > + if (base->mode != side->mode) > + return 0; > + if (oideq(&base->oid, &side->oid)) > + return 1; > + > + if (read_oid_strbuf(opt, &base->oid, &basebuf) || > + read_oid_strbuf(opt, &side->oid, &sidebuf)) > + goto error_return; > + /* > + * Note: binary | is used so that both renormalizations are > + * performed. Comparison can be skipped if both files are > + * unchanged since their sha1s have already been compared. > + */ > + if (renormalize_buffer(idx, path, basebuf.buf, basebuf.len, &basebuf) | > + renormalize_buffer(idx, path, sidebuf.buf, sidebuf.len, &sidebuf)) > + ret = (basebuf.len == sidebuf.len && > + !memcmp(basebuf.buf, sidebuf.buf, basebuf.len)); > + > +error_return: > + strbuf_release(&basebuf); > + strbuf_release(&sidebuf); > + return ret; > +} > + > > struct directory_versions { > /* > * versions: list of (basename -> version_info) > @@ -3003,8 +3057,13 @@ static void process_entry(struct merge_options *opt, > modify_branch = (side == 1) ? opt->branch1 : opt->branch2; > delete_branch = (side == 1) ? opt->branch2 : opt->branch1; > > - if (ci->path_conflict && > - oideq(&ci->stages[0].oid, &ci->stages[side].oid)) { > + if (opt->renormalize && > + blob_unchanged(opt, &ci->stages[0], &ci->stages[side], > + path)) { > + ci->merged.is_null = 1; > + ci->merged.clean = 1; > + } else if (ci->path_conflict && > + oideq(&ci->stages[0].oid, &ci->stages[side].oid)) { Small note (no need for re-roll or whatever) on having read a bit of merge-ort.c code recently: I'd find this thing a bit easier on the eyes if ci->stages[0] and ci->stages[side] were split into a variable before the if/else, i.e. used as "side_0.oid and side_n.oid" and "side_0 and side_n" in this case.. That would also avoid the wrapping of at least one argument list here.
diff --git a/merge-ort.c b/merge-ort.c index 87c553c0882c..c4bd88b9d3db 100644 --- a/merge-ort.c +++ b/merge-ort.c @@ -2416,6 +2416,60 @@ static int string_list_df_name_compare(const char *one, const char *two) return onelen - twolen; } +static int read_oid_strbuf(struct merge_options *opt, + const struct object_id *oid, + struct strbuf *dst) +{ + void *buf; + enum object_type type; + unsigned long size; + buf = read_object_file(oid, &type, &size); + if (!buf) + return err(opt, _("cannot read object %s"), oid_to_hex(oid)); + if (type != OBJ_BLOB) { + free(buf); + return err(opt, _("object %s is not a blob"), oid_to_hex(oid)); + } + strbuf_attach(dst, buf, size, size + 1); + return 0; +} + +static int blob_unchanged(struct merge_options *opt, + const struct version_info *base, + const struct version_info *side, + const char *path) +{ + struct strbuf basebuf = STRBUF_INIT; + struct strbuf sidebuf = STRBUF_INIT; + int ret = 0; /* assume changed for safety */ + const struct index_state *idx = &opt->priv->attr_index; + + initialize_attr_index(opt); + + if (base->mode != side->mode) + return 0; + if (oideq(&base->oid, &side->oid)) + return 1; + + if (read_oid_strbuf(opt, &base->oid, &basebuf) || + read_oid_strbuf(opt, &side->oid, &sidebuf)) + goto error_return; + /* + * Note: binary | is used so that both renormalizations are + * performed. Comparison can be skipped if both files are + * unchanged since their sha1s have already been compared. + */ + if (renormalize_buffer(idx, path, basebuf.buf, basebuf.len, &basebuf) | + renormalize_buffer(idx, path, sidebuf.buf, sidebuf.len, &sidebuf)) + ret = (basebuf.len == sidebuf.len && + !memcmp(basebuf.buf, sidebuf.buf, basebuf.len)); + +error_return: + strbuf_release(&basebuf); + strbuf_release(&sidebuf); + return ret; +} + struct directory_versions { /* * versions: list of (basename -> version_info) @@ -3003,8 +3057,13 @@ static void process_entry(struct merge_options *opt, modify_branch = (side == 1) ? opt->branch1 : opt->branch2; delete_branch = (side == 1) ? opt->branch2 : opt->branch1; - if (ci->path_conflict && - oideq(&ci->stages[0].oid, &ci->stages[side].oid)) { + if (opt->renormalize && + blob_unchanged(opt, &ci->stages[0], &ci->stages[side], + path)) { + ci->merged.is_null = 1; + ci->merged.clean = 1; + } else if (ci->path_conflict && + oideq(&ci->stages[0].oid, &ci->stages[side].oid)) { /* * This came from a rename/delete; no action to take, * but avoid printing "modify/delete" conflict notice