Re: [PATCH] negotiator/skipping: skip commits during fetch

> On Thu, 26 Jul 2018, Jonathan Tan wrote:
> 
> > > On Mon, 16 Jul 2018, Jonathan Tan wrote:
> > > 
> > > >  t/t5552-skipping-fetch-negotiator.sh | 179 +++++++++++++++++++
> > > 
> > > This test seems to be failing consistently in the recent `pu` builds:
> > > 
> > > https://git-for-windows.visualstudio.com/git/_build/results?buildId=14337&view=logs
> > > 
> > > Could you have a look, please?
> > 
> > Hmm...on my Linux computer, this test passes on both pu (as of the time
> > of writing) and 838143aa5c ("Merge branch 'ab/newhash-is-sha256' into
> > pu", 2018-07-25) (pu at the time of that build, according to the website
> > you linked above). If you could rerun that test with additional code,
> > could you add a "cat trace" and show me what the client sends?
> 
> I can give you something even better: a playground. Just open a PR at
> https://github.com/gitgitgadget/git (all of the branches on gitster/git ar
> mirrored, including yours, I am sure, so you can target that branch
> specifically).
> 
> Once you open a Pull Request, it will automatically build and run the test
> suite on Windows, macOS and Linux. You will see it in the "checks" section
> on the bottom. Example for my range-diff series:
> 
> https://git-for-windows.visualstudio.com/git/_build/results?buildId=14279
> 
> For a quicker turnaround, you could add a commit that forces the `all`
> target in `t/Makefile` to run only your test.
> 
> > When I do that, the relevant parts are:
> > 
> >   packet:        fetch> have 9ab46928dc282aa09f4dbf96893a252e058e7e8e
> >   packet:        fetch> have dc824fafb05f3229aedf1f320bbe572e35364dfe
> >   packet:        fetch> have caef059de69917b9119176a11b88afcef769331d
> >   packet:        fetch> have 41bd8dc092ee110ba80e350a346ec507ab2e42a0
> >   packet:        fetch> have e9a2c092a8e911567a377c881a7f6031e7f892ea
> >   packet:        fetch> done
> > 
> > which is exactly as I (and the test) expect.
> > 
> > Two possible reasons for the discrepancy that I can think of offhand are
> > that (1) my computer generates different commits from your test system,
> > and (2) the priority queue pops commits in a different order. For (1),
> > that's not possible because the SHA-1s are the same (as can be seen by
> > comparing your link and the "have" lines I quoted above), and for (2),
> > the code seems OK:
> > 
> >   static int compare(const void *a_, const void *b_, void *unused)
> >   {
> >   	const struct entry *a = a_;
> >   	const struct entry *b = b_;
> >   	return compare_commits_by_commit_date(a->commit, b->commit, NULL);
> >   }
> > 
> > Let me know if you can observe the output of "cat trace" or if you have
> > any other ideas.
> 
> Like I said, you can use those "CI" builds, I think that would be more
> effective than if you waited for me to react, I am quite overwhelmed these
> days.