Re: [PATCH] fetch: do not pass ref-prefixes for fetch by exact SHA1

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Jun 1, 2018, 02:49 UTC
Message-ID: <20180601024924.GA111965@aiede.svl.corp.google.com>
In-Reply-To: <xmqqtvqn9rwa.fsf@gitster-ct.c.googlers.com>

Junio C Hamano wrote:

> Jonathan Nieder <jrnieder@gmail.com> writes:

Show 15 quoted lines

>> This patch adds a test to check this behavior that notices another
>> behavior difference between protocol v0 and v2 in the process.  Add a
>> NEEDSWORK comment to clear it up.
>
> Thanks.
>
> I wonder if there is a more effective way to smoke out other bugs
> remaining in proto v2.  When the fetch-by-SHA1 feature was added
> originally, we certainly would have added a test or two to make sure
> it won't break.  The root cause of this breakage is that we lack the
> ability to easily exercise proto v2 on these existing tests that
> were written back in the proto v0 days.  It there were such a way
> (like, a common set of tests that are run with all supported
> protos), we would have caught the breakge even before the topic hit
> 'next'.

I had a similar thought.

I am not sure I agree about the root cause, but root causes are generally slippery to define. Because this bug had significant internal impact, we came up with a few next steps:

- shore up protocol v2 test coverage, as you described

- arrange for long refactoring series we submit to be divided up for
  the team to review, to avoid reviewer fatigue.  Hopefully this will
  make us a better example for other submitters of long series.  We're
  open to cooperating with others --- maybe we can set up a volunteer
  reviewer brigade to get a more diverse set of eyes on each series
  --- though organizing that is harder.

- improve telemetry for our internal deployment, to get earlier notice
  when Git is producing more errors.  I suspect other installations
  may want something like this too --- e.g. I think this is one of the
  benefits of what Jeff Hostetler is starting to build with json-writer.

- help internal users triage errors from Git (like those decision
  trees parents have to help decide when to bring a child to the
  doctor), so that we get earlier notice and can roll back and report
  upstream more quickly when they've run into a Git bug

Or in other words, please expect more in this area soon, and feel free to pester me if the test coverage doesn't arrive. :)

Thanks, Jonathan