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

Tue Apr 7 08:21:35 PDT 2015

On 07/04/15 15:01, Emil Velikov wrote:
> On 7 April 2015 at 13:14, 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.)
>>
>
> So let see if I got this correct, apologies in advance if it comes out
> too blunt.
>
> Unless I'm mistaken the gallium interfaces are internal/private, so
> comparing them with public ones (like the Khronos OpenGL) seems like
> comparing apples to oranges. Yet as one tries to have/use gallium
> interfaces as if they were public, the idea of gettting some of this
> #ifdef-ery into a single, isolated and easily manageable place is
> valid and honourable.

 From my POV, Gallium interfaces are public and always have been. 
Admittedly, there's no standards body, and the interface is neither 
stable nor does it provide backwards compatibility.  But pretty much 
from as far as I can remember (which is 2007) there were external (as in 
out-of-tree) state-trackers and even externals drivers.

http://jrfonseca.blogspot.co.uk/2008/04/gallium3d-introduction.html

Maybe this is not widely know nowadays, but we never made secret of 
that. In particular we always mentioned that Tungsten Graphics (now part 
of VMware) had a Direct3D 9 state tracker.

> I guess what I'm failing to see is that why one does not integrate the
> in-house code-base with mesa's (be that in a public or private repo) ?
> The situation seems similar to rbug-gui, which depends on internal
> API/libraries yet it lives out-of-tree (patches that integrate it are
> on the ML). With the possibility of things ending up broken (in a very
> in a subtle ways), upon each update.

I don't know how rbug-gui was working before.  Probably didn't need to 
do anything special -- we didn't either.

This is indeed a problem I hadn't foreseen, an unexpected consequence of 
unifying the classic Mesa and gallium utility headers/helpers/etc.

I still think we all did the right thing.  I just need some way of 
getting headers that are meant to be consumed outside Mesa less reliant 
on build system specificities.

BTW, another solution would be for autotools to generate a "config.h".

And have SCons, etc, include a hand-written drop-in config.h (living in 
a separate directory.)

This is actually a practice that many projects (out of my head I can 
name zlib, linpng, tiff, etc) do.

But as I said, I'm fine leaving autotools as is, unsullied, no changes 
what so ever.

Another alternative is for me to pre-include this fakeconfig.h , ie., 
`gcc --include fakeconfig.h`, MSVC's `/Fifakeconfig.h`

> Afaict the overhead of rebasing an integrated solution on top of newer
> mesa, would be less than having it out-of-tree. Plus it seems like the
> better engineering approach. Perhaps I'm missing something and this
> does not hold true ?

I'm afraid it doesn't hold true.  It's not worth going into specifics, 
but imagine the following: there's Mesa, theres our Product, and there's 
the Component linking both.  Mesa has its build system. The Product has 
its build system.  Both Product and Mesa are huge, so building one 
inside the other it's just impractical.  What you can do is choose to 
build the Component inside Mesa or inside the Product, but either way 
you'll end up the variations of the same problem.  Which is one is 
easier depends on how tightly the Component is integrated with Mesa vs 
the Product.

The component is sort of a Direct3D state tracker, and is way more 
tightly integrated into the rest of the Product than Mesa, as it really 
one needs the gallium headers and a few of the helper modules.

> On the Android topic - currently it hard-codes the defines as the
> build system a) lacks the functionality equivalent to ac_check_funcs
> (afaict) and b) uses GCC explicitly, plus it is new enough to support
> the all the built-ins/attributes/etc.
> When any of that changes I would happily update it.

I see.

In my case, updating our two internal build systems requires review 
request to external teams.  But above all, I'm just afraid some bug 
slips through and get caught too late.  By having SCons and our two 
internal build systems using the same fallback set of #defines, I can be 
sure that all the test coverage with have through scons builds apply.

Jose