Web lists-archives.com

Re: nd/merge-quit, was Re: What's cooking in git.git (May 2019, #01; Thu, 9)

Hi Duy,

On Fri, 10 May 2019, Duy Nguyen wrote:

> On Fri, May 10, 2019 at 3:54 AM Johannes Schindelin
> <Johannes.Schindelin@xxxxxx> wrote:
> >
> > Hi Junio & Duy,
> >
> > On Thu, 9 May 2019, Junio C Hamano wrote:
> >
> > > * nd/merge-quit (2019-05-07) 2 commits
> > >  - merge: add --quit
> > >  - merge: remove drop_save() in favor of remove_merge_branch_state()
> > >
> > >  "git merge" learned "--quit" option that cleans up the in-progress
> > >  merge while leaving the working tree and the index still in a mess.
> > >
> > >  Hmph, why is this a good idea?
> >
> > It also seems to work *only* on Linux. At least the tests break on macOS
> > and on Windows:
> >
> > https://dev.azure.com/gitgitgadget/git/_build/results?buildId=8313&view=ms.vss-test-web.build-test-results-tab
> Sorry I have no idea what the problem is. That's basically the same as
> the 'merge detects mod-256 conflicts (recursive)' test earlier but
> with rerere enabled. It does not even look like some leftover rerere
> records accidentally fix the conflict.
> I tried with a case-insensitive filesytem (on linux) and with
> --valgrind, no problem found. Travis on pu seemed ok with t7600 on
> mac.
> One difference I notice is the the failed test looks like it found the
> wrong merge base
> found 1 common ancestor:
> c4c4222 commit 1
> while my tests have "commit 0" as the base. "git log --graph
> --oneline" indicates "commit 1" is the wrong base.
> Something is wrong with the merge code (this has not even reached the
> new --quit code). I could change the setup steps to be more stable,
> using a simpler commit history, but this looks like something we
> should find and fix.

Yeah... someone should look at this... Someone. But who?


Well, since you seemed quite reluctant to figure out why your patches fail
the test suite, and since we're about to enter the -rc0 phase (where we
all spend all of our time to hammer out the next version, right? Right?),
I figured out I better look into it before nobody does.

Turns out that the culprit is not even hard to figure out. All I had to do
is to compare, carefully, the logs from the Azure Pipelines and from a
local run in a local Ubuntu.

It has nothing to do with our merge code. There might be bugs, but this
breakage is safely in this here patch series: the test case you introduced
relies on side effects.

Namely, when test cases 51 and 52 are skipped because of a missing GPG
prerequisite [*1*], and those two are obviously required to run for the
`git merge to fail in your test case, as you can very easily verify by
downloading the artifact containing the `trash directory.t7600-merge`
directory and re-running the last steps on Linux (where the `git -c
rerere.enabled=true merge master` *succeeds*).

In fact, you can very, very easily emulate the whole situation on your box
by running:

	sh t7600-merge.sh -i -v -x --run=1-50,53-59

And then you can fix your test case so that it does not need to rely on
test cases that may, or may not, have run previously.


Footnote *1*: GNU Privacy Guard is not actually missing from Git for
Windows' SDK, quite to the contrary. But it fails to start a gpg-agent due
to the fact that we pass a `--homedir` that contains a colon, something
that is totally expected on Windows, and at the same something that GNU
Privacy Guard totally cannot handle.