On Tue, Apr 7, 2015 at 5:14 AM, Jose Fonseca <jfons...@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 <jfons...@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 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
