Re: [RFC 7/7] upload-pack: ack version 2

From: Bryan Turner <bturner@atlassian.com>
To: Brandon Williams <bmwill@google.com>
Cc: Git Users <git@vger.kernel.org>, Jeff King <peff@peff.net>,
	Jonathan Nieder <jrnieder@gmail.com>,
	sbeller@google.com, Junio C Hamano <gitster@pobox.com>,
	jonathantanmy@google.com
Subject: Re: [RFC 7/7] upload-pack: ack version 2
Date: Fri, 1 Sep 2017 15:02:36 -0700	[thread overview]
Message-ID: <CAGyf7-GKAW3Dn_rPKxtidKoXh7a6NmDFYzGrb9tGOszhsMignA@mail.gmail.com> (raw)
In-Reply-To: <20170824225328.8174-8-bmwill@google.com>

On Thu, Aug 24, 2017 at 3:53 PM, Brandon Williams <bmwill@google.com> wrote:
> +
> +       version = getenv("GIT_PROTOCOL");
> +       if (!strcmp(version, "2"))
> +               upload_pack_v2();
> +

I think the "if" check here needs some type of NULL check for
"version" before calling "strcmp". Without that, if the "GIT_PROTOCOL"
environment variable isn't set, git-upload-pack SEGVs.

This came up when I was testing the "protocol-v2" branch with
Bitbucket Server. For performance reasons, we sometimes run ref
advertisements as a separate process, when serving fetches, to allow
throttling the ref advertisement separately from actually generating
and sending a packfile. Since CI servers continuously poll for
changes, but usually don't find any, we want to be able to serve ref
advertisements, which have minimal overhead, with much higher
parallelism than serving packs. To do that, we run "git-upload-pack
--advertize-refs" directly, and that command has never *required*
"GIT_PROTOCOL" before this change.