Re: [PATCH] http.c: fix parsing of http.sslCertPasswordProtected variable

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 15, 2013, 15:54 UTC
Message-ID: <7v4nbvddiy.fsf@alter.siamese.dyndns.org>
In-Reply-To: <EFAADB2C-5774-4076-99CE-887AFB2B5CCA@gmail.com>

"Kyle J. McKay" <mackyle@gmail.com> writes:

Show 20 quoted lines

> That works fine for GIT_SSL_CERT_PASSWORD_PROTECTED and
> GIT_CURL_VERBOSE, but it's a little bit awkward for GIT_SSL_NO_VERIFY
> and GIT_CURL_FTP_NO_EPSV since they have "_NO_" in their names.
>
> If the user wants to override a "http.sslVerify=false" then
> "GIT_SSL_NO_VERIFY=false" is needed rather than "GIT_SSL_VERIFY=true".
>
> We could:
>
> 1) Introduce GIT_SSL_VERIFY and GIT_CURL_FTP_EPSV and say if they are
> set the "_NO_" version is ignored.
>
> 2) Go ahead with "GIT_SSL_NO_VERIFY=false" to force http.sslVerify
> back to true (and similarly for EPSV).
>
> 3) Just leave GIT_SSL_NO_VERIFY and GIT_CURL_FTP_NO_EPSV alone.
>
> 4) Do something else, ideas?
>
> Comments?

The usual way we have done this kind of thing so far is:

 (1) Add GIT_SSL_VERIFY and GIT_CURL_FTP_EPSV support, that is
     boolean.  If that is set, it takes precedence to
     GIT_SSL_NO_VERIFY and GIT_CURL_FTP_NO_EPSV.  If not,
     GIT_SSL_NO_VERIFY and GIT_CURL_FTP_NO_EPSV are used just like
     before (setting it to any value will decline VERIFY or EPSV).

 (2) Issue a warning to tell users to use GIT_SSL_VERIFY or
     GIT_CURL_FTP_EPSV instead, when GIT_SSL_NO_VERIFY and
     GIT_CURL_FTP_NO_EPSV are used.

 (3) Later at a large version bump, remove support for *_NO_*
     variants.

We can stop at (1) and not do (2) and (3), if the resulting code after (1) is not too cumbersome to maintain. If we do (2) then we must follow it through with (3), and if we plan to do (3) then we must precede it with (2).