On Thu, Sep 09, 2021 at 04:12:26PM -0700, Junio C Hamano wrote: > > But in terms of compiling, all we care about is that the constant is > > there. So I think the cutoff point you found is what we want. Presumably > > when the file format isn't supported we'd get some error, though it's > > not clear if that would come during the actual curl_*_perform(), or if > > we should be checking the curl_easy_setopt() result. > > If we were evaluating a patch to add support for pinnedpublickey > afresh back in, say, 2017, perhaps we cared enough about the > distinction between 7.39 and 7.44 (Nov 2014 and Aug 2015, > respectively), but I'd say cut-off at 7.44 for this, once it is > written and committed in our codebase, is good enough for us. > > If the code originally had cut-off at 7.39 and we were raising the > floor to 7.44 with "sha256 weren't usable before that version" as > the justification, it would be a totally different situation and it > may be worth the code change, but I am not sure if going backwards > is worth it. > > So, I dunno. I don't have a sense of whether the functionality difference between 7.39 and 7.44 actually matters. I just saw it as: if you have 7.39 then before it would not work because we didn't bother to compile it, and after it _might_ work depending on how you use it. So it's a strict increase in functionality. Likewise, if you compile Git against 7.39 and then later upgrade the shared library, you'd get the increased functionality. But I admit I don't really care that much, either. This is an obscure-ish feature in a 7 year old version of libcurl we are talking about. But there is one thing that does get weird if we don't do this patch. If we later take the approach of checking: #ifdef CURLOPT_PINNEDPUBLICKEY then that will subtly shift the cutoff point from 7.44 to 7.39 anyway. _If_ we are going to do that conversion in a later patch (as this series does), I think it makes sense to shift the version number explicitly in a commit with an explanation, as this commit does. -Peff
