Re: [PATCH v3] cache.h: auto-detect if zlib has uncompress2()

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 23, 2022, 19:18 UTC
Message-ID: <xmqqtudu9s7k.fsf@gitster.g>
In-Reply-To: <74d35354-20a6-9cc1-3452-573460c694bd@drbeat.li>

Beat Bolli <dev+git@drbeat.li> writes:

Show 24 quoted lines

> On 22.01.22 00:23, Junio C Hamano wrote:
>> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>> 
>>> As noted in the updated commit message this approach of having an
>>> object just for this fallback function comes at the cost of some
>>> complexity, but now the compat object lives neatly in its own object.
>> I do not see any change in this patch adding costly complexity, but
>> I notice lack of one possible trick that might become problem with
>> some compilers and linkers when their zlib has uncompress2()
>> function.  Let's have this graduate very early in the next cycle, to
>> see if anybody on a rarer system sees a complaint due to having to
>> deal with a totally empty object file.
>
> OpenSSL has a macro in include/openssl/macros.h to counteract exactly this:
>
>     /*
>      * Sometimes OPENSSL_NO_xxx ends up with an empty file and some
>        compilers
>      * don't like that.  This will hopefully silence them.
>      */
>     #define NON_EMPTY_TRANSLATION_UNIT static void *dummy = &dummy;
>
> They insert it in the otherwise empty "#else" branch of conditionally
> complied code.

Between my "I recall some compilers and linkers had trouble with an totally empty object files, and we'd better be prepared for them" and "I didn't see any system that had such a problem during my tests", I still lean toward "it merely shows that the problem is rarer than the set of systems you tried", even without further input.

But "It is a problem for which a real workaround is used in a system that is used more widely than we are" is a clear enough evidence that we should have one, especially the solution is so trivial.

https://github.com/openssl/openssl/pull/10425 seems to indicate that they are moving in a direction that removes the necessity to use this macro, by switching to tell the build system to not compile and build a file that would become empty due to conditionals, but nobody in the discussion seems to question the need for the macro for portability if an otherwise empty file need to be dealt with.

Thanks.