Re: [PATCH 1/1] imap-send: include strbuf.h

From: Christian Hesse <list@eworm.de>
Date: May 17, 2023, 20:12 UTC
Message-ID: <20230517221237.590fb984@leda.eworm.net>
In-Reply-To: <xmqqilcrq6a9.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> on Wed, 2023/05/17 09:19:

Show 16 quoted lines

> Taylor Blau <me@ttaylorr.com> writes:
> 
> > On Wed, May 17, 2023 at 08:49:37AM -0700, Junio C Hamano wrote:  
> >> Christian Hesse <list@eworm.de> writes:
> >>  
> >> > From: Christian Hesse <mail@eworm.de>
> >> >
> >> > We use xstrfmt() here, so let's include the header file.
> >> >
> >> > Signed-off-by: Christian Hesse <mail@eworm.de>
> >> > ---
> >> >  imap-send.c | 1 +
> >> >  1 file changed, 1 insertion(+)  
> >>
> >> Puzzled.  For me Git 2.41-rc0 builds as-is without this change just
> >> fine, it seems.

I prepared cgit to build with libgit.a 2.41.0-rc0. While cgit itself builds fine (with some justifications of course), building git for the test suite failed.

> > It will fail to build for ancient versions of curl (pre-7.34.0, which
> > was released in 2013), or if you build with `NO_CURL=1`.

Indeed we have NO_CURL=1 in cgit's Makefile...

> xstrfmt() is used at exactly one place, inside "#ifndef NO_OPENSSL",
> in the implementation of the static function cram().
>
> Ah, the mention of that function was a huge red herring.

Well, the warning about implicit declaration of xstrfmt() was this one that popped up... :) Sorry for the confusion.

Show 13 quoted lines

> There are
> tons of strbuf API calls in the file outside any conditional
> compilation, and where it inherits the include from is "http.h",
> that is conditionally included.
> 
> OK, so the fix seems to make sense, but the justification for the
> change needs to be rewritten, I think.
> 
>     We make liberal use of the strbuf API functions and types, but
>     the inclusion of <strbuf.h> comes indirectly by including
>     <http.h>, which does not happen if you build with NO_CURL.
> 
> or something like that?

Fine with me!
Do you want me to re-send the patch or do you modify this on the fly?

> Thanks.

-- 
main(a){char*c=/*    Schoene Gruesse                         */"B?IJj;MEH"
"CX:;",b;for(a/*    Best regards             my address:    */=0;b=c[a++];)
putchar(b-1/(/*    Chris            cc -ox -xc - && ./x    */b/42*2-3)*42);}