Re: [RFC PATCH] sequencer - tipped merge strategy

From: Edmundo Carmona Antoranz <eantoranz@gmail.com>
Date: Mar 5, 2023, 14:00 UTC
Message-ID: <CAOc6etawNSEjwt9QsJE90ok5pJjf4xOJ6ZaFr9HkHu3+Sw89+A@mail.gmail.com>
In-Reply-To: <CABPp-BGOtsjgfN5f=dSb0ZSEx8nzFs6SKrUm=2TtPPH5cKa4cA@mail.gmail.com>

Hey, Elijah!

Thank you for all of that feedback. Really good Will need to go through it which will take me some time to digest but I did want to go over one of the subjects.

Show 7 quoted lines

> So, if I'm reading correctly (please double check me), since your
> merge strategy made no changes to the index or working tree, and
> returned a status of 2, and since !2 == !1 == 0, we'll treat this the
> same as a successful merge and commit the "results", i.e. the tree of
> the first parent.  Doesn't this tipped merge strategy thus behave the
> same as a `--strategy ours` merge when its preconditions are not
> satisfied?  If it does, that would be horrifying.

I think that is not correct. The possible values that come out of try_tms_merge are 0 if nothing happened and 1 if the merge was successful and we changed the index and the working tree. Then I think I wrote this correctly following the call:

if (ret) {
  discard_index(r->index);
  if (repo_read_index(r) < 0) {
    ret = error(_("could not read index"));
    goto leave_merge;
  }
  goto ran_merge;
}
// regain lock to go into recursive
if (repo_hold_locked_index(r, &lock, LOCK_REPORT_ON_ERROR) < 0) {
  ret = -1;
  goto leave_merge;
}

Actually, if we will be switching to using a library, then this won't be that important because we might be able to pull it off without having to release the lock given that we would be running in-process, but I wanted to clear up what the intended flow is there, just in case.

Ok.... more questions or comments will be coming in the following days. And thank you, again.

BR!