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

[Matt Turner]

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.

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).

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

Thanks,
Matt