[Mesa-dev] [PATCH 4/4] autoconf, scons: Move fallback HAVE_* definitions to headers.

[Jose Fonseca]

On 07/04/15 17:16, Matt Turner wrote:
> On Tue, Apr 7, 2015 at 5:14 AM, Jose Fonseca <jfonseca at vmware.com> wrote:
>> Sorry for the delay. I've been away during the Easter.
>>
>> On 02/04/15 19:02, Matt Turner wrote:
>>>
>>> On Thu, Apr 2, 2015 at 7:32 AM, Jose Fonseca <jfonseca at vmware.com> wrote:
>>>>
>>>> These were being defined in SCons, but it's not practical -- we actually
>>>> need to include Gallium headers from external source trees, with
>>>> completely disjoint build infrastructure, and it's unsustainable to
>>>> replicate the HAVE_xxx checks or even hard-coded defines across
>>>> everywhere.
>>>
>>>
>>> To confirm, you're building external sources with gcc? I don't think
>>> these macros are useful for MSVC.
>>
>>
>> Correct.
>>
>>
>>>>
>>>> No actual change in behavior for autoconf.
>>>> ---
>>>>    configure.ac         |  2 +-
>>>>    include/c99_compat.h | 45 +++++++++++++++++++++++++++++++++++++++++++++
>>>>    scons/gallium.py     | 27 ---------------------------
>>>>    src/util/macros.h    |  2 ++
>>>>    4 files changed, 48 insertions(+), 28 deletions(-)
>>>>
>>>> diff --git a/configure.ac b/configure.ac
>>>> index 520cc22..1485bba 100644
>>>> --- a/configure.ac
>>>> +++ b/configure.ac
>>>> @@ -230,7 +230,7 @@ _SAVE_LDFLAGS="$LDFLAGS"
>>>>    _SAVE_CPPFLAGS="$CPPFLAGS"
>>>>
>>>>    dnl Compiler macros
>>>> -DEFINES=""
>>>> +DEFINES="-DHAVE_AUTOCONF"
>>>>    AC_SUBST([DEFINES])
>>>>    case "$host_os" in
>>>>    linux*|*-gnu*|gnu*)
>>>> diff --git a/include/c99_compat.h b/include/c99_compat.h
>>>> index 4fc91bc..62ccd46 100644
>>>> --- a/include/c99_compat.h
>>>> +++ b/include/c99_compat.h
>>>
>>>
>>> c99_compat.h doesn't seem like the right location. I know it seems
>>> like a nice place to add this since it's included everywhere, but I
>>> worry that in a few years we're going to be cleaning it up like we've
>>> been doing with compiler.h and friends.
>>>
>>> I might make a separate header to define these? Not sure.
>>
>>
>> I can move the defines out of c99_compat.h , e.g.,
>> mesa/include/fallbackconfig.h.
>>
>> But I'd prefer to include fallbackconfig.h out of c99_compat.h , as
>> c99_compat.h is pretty much guaranteed to be included all the time.
>>
>>
>>> Since
>>> probably all cases of #ifdef HAVE___* have a fallback, that runs the
>>> risk of never noticing that you weren't including the right header.
>>
>> Precisely, this is all the more reason why it must be included from a header
>> that's included all the time.  If it depends on people to add the include on
>> a case-by-case it is bound to fail, as nobody else but us cares, and it will
>> easily go unnoticed.
>>
>>
>>>> @@ -141,4 +141,49 @@ test_c99_compat_h(const void * restrict a,
>>>>    #endif
>>>>
>>>>
>>>> +
>>>> +/* Fallback definitions, for when these headers are used by build
>>>> systems which
>>>> + * don't auto-detect these things.*/
>>>> +#ifndef HAVE_AUTOCONF
>>>
>>>
>>> I'd rather flip this condition around and not modify configure.ac. But
>>> maybe you can't do that because you're not actually building
>>> everything with scons?
>>
>>
>> No biggie either way.
>>
>>> I don't know. This seems nuts. I really don't like adding stuff to the
>>> autotools build system like this.
>>
>>
>> Sure.
>>
>>
>>> I really don't know how to deal with this. What I'm hearing is that
>>> even the custom scons build system you guys use isn't sufficient for
>>> your own needs. You're not building the external source trees with the
>>> same build system...?
>>
>>
>> I think you might be getting the wrong idea.
>>
>> We don't build the .C files from external source trees.  But we do need to
>> include .h files, so we can interface with components in Mesa tree.
>>
>> That is, I only need the .h files to make sense on their own (with Mesa
>> components, namely mesa/src/gallium/include, and gallium auxiliary
>> libraries).  But we have so many inlines functions, so many #ifdef HAVE_foo,
>> that unless all the defines match precisely, the whole hell breaks loose.
>>
>>
>> Gallium has from the start been integrated (ie. embedded) on a myriad of
>> places.  It was always meant as a framework to write any sort of 3d driver,
>> not just OpenGL drivers.  Things were much worse when Gallium was used on
>> Windows XP kernel land or Windows CE.  I'm glad that I or anybody else has
>> to deal with the quirkiness of keeping code portable across these platforms.
>> Things are still much more uniform nowadays.
>>
>>
>>> I mean, in all the build system work I've done I've tried to make sure
>>> scons continues working -- doing things like adding these HAVE_*
>>> definitions to it and such. It's kind of frustrating, and it's even
>>> more frustrating when even that isn't sufficient.
>>
>>
>>
>> All I'm doing here is basically move your defines out of scons's python
>> files into C headers.  Conceptually it's doing pretty much the same thing as
>> before, but being in a header that means that it's there for all build
>> systems to take.
>>
>>
>> Rembember that Mesa itself is not just autoconf and Scons, there's also
>> Android build system.
>>
>> I don't like it any more you do, but this is the world we live in: the fact
>> is that many platforms constraint how software must be built to a point
>> which is impracticable/impossible to build.  Even if a build system that
>> meets everybody needs existed, we'd still face the legacy of existing
>> software using other build systems.
>>
>>
>>
>> To be honest, IMHO, Mesa source tree and build systems are a failure if they
>> can't even sustain external interfaces.
>>
>>
>> For many drivers, the external interface headers are Khronos OpenGL / GLES
>> headers.  But for gallium drivers, the interface is mesa/src/gallium/include
>> (plus some .h from helper modules in src/gallium/auxiliary as it is
>> impractical to interface with gallium drivers without them.)
>>
>>
>> What would you say in a parallel reality, Khronos demanded that in order to
>> build OpenGL drivers for Linux one would need to use the Khronos own build
>> system?  Because that's basically what's at stake here: if I want to
>> interface with gallium and llvmpipe driver should I be forced to build my
>> code with Mesa build system?
>>
>>
>> So I only see three ways of dealing with this:
>>
>> a) have fallback HAVE_* foo from the headers (so that all inline functions
>> compile the same way) as I propose in this patch
>>
>> b) move all inline functions to separate headers (so that external code can
>> opt-out from including them), and provide alternative non-inline
>> implementations (so that external code can still call them)
>>
>> c) stop using inline functions altogether
>>
>>
>>
>> One way or the other, we'll need the headers to make sense on their own,
>> without having to duplicate the whole Mesa build-systems.  But b) and c) can
>> have performance impact. (Particularly because we really want to inline
>> atomic reference counting.)
>>
>>
>>
>> Jose
>
> Thanks Jose.
>
> Thanks for explaining the reality of the situation. It makes sense to me.
 >
> I'm fine with these being added to c99_compat.h or another header
> included by it. I think my only real change request is to invert the
> macro to not have to modify configure.ac.

Sure.  I'll update that patch then.

I'll submit the other 1-3 patches of these series as before that, as 
they are minor cleanups and not controversial.

> In your follow-up email you mention autoconf generating config.h. I've
> thought about that as well, but I didn't consider that it might allow
> us to solve this problem (I'm not entirely sure it would, since it
> seems like you'd have to run configure to generate config.h and *then*
> run scons and your out-of-tree build system).

What I had in mind was that SCons config.h would be hand-written (ie, 
basically contain everything that I added to c99_compat.h in the first 
version.)

So autoconf would generate its own config somewhere, and add 
`-Ipath/to/where/generated/config_h/goes`, while scons would have 
`-Ipath/to/where/handwritten/fake/config_h/lives`


> In any case, that might be a nice solution. I'll think about that some more.
>
> Thanks,
> Matt
>

Thanks.

Jose