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

From: Jonathan Tan <jonathantanmy@google.com>
Date: Jul 26, 2018, 19:16 UTC
Message-ID: <20180726191609.176343-1-jonathantanmy@google.com>
In-Reply-To: <nycvar.QRO.7.76.6.1807261233300.71@tvgsbejvaqbjf.bet>

Show 11 quoted lines

> Hi Jonathan,
> 
> 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? 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.