Re: [Mesa-dev] [PATCH] gallium/auxiliary: Fix Autotools on Android (v2)
On Fri, Jul 27, 2018 at 6:52 AM Chad Versace wrote: > > On Wed 25 Jul 2018, Tomasz Figa wrote: > > Hi Chad, > > > > On Wed, Jul 25, 2018 at 10:11 AM Chad Versace > > wrote: > > > > > > Problem 1: u_debug_stack_android.cpp transitively included > > > "pipe/p_compiler.h", but src/gallium/include was missing from the C++ > > > include path. > > > > > > Problem 2: Add -std=c++11 to AM_CXXFLAGS. Android's libbacktrace headers > > > require C++11, but the Android toolchain (at least in the Chrome OS SDK) > > > does not enable C++11 by default. > > > > > > v2: Add -std=c++11. > > > > > > Cc: Gurchetan Singh > > > Cc: Eric Engestrom > > > --- > > > src/gallium/auxiliary/Makefile.am | 6 +- > > > 1 file changed, 5 insertions(+), 1 deletion(-) > > [snip] > > > > if HAVE_PLATFORM_ANDROID > > > +# Android's libbacktrace headers required C++11, but the Android > > > toolchain (at > > > +# least in the Chrome OS SDK) does not enable C++11 by default. > > > +AM_CXXFLAGS += $(CXX11_CXXFLAGS) > > > + > > > > This is something that would normally be handled by the .pc file for > > given library. Package build system shouldn't be polluted with such > > system-specific low level dependencies. > > Normally, I would agree. "If libbacktrace needs a CXXFLAG, then put in > the pc file". That's reasonable for most flags, because most flags are > *additive*. But the -std flag is not. If backtrace.pc added '-std=c++11' > to CXXFLAGS, but a different package required c++14, how should Mesa > resolve that conflict? > > As precedent, I searched all pc files on my Debian machine. The only pc > files that add -std to CFLAGS or CXXFLAGS are the icu-*.pc files. And > those files should serve as anti-precedent in most cases; they are badly > written compared to all other pc files I've seen. Okay, fair enough. Sorry for the noise then. Thanks for rechecking. Reviewed-by: Tomasz Figa Best regards, Tomasz ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] gallium/auxiliary: Fix Autotools on Android (v2)
On Wed 25 Jul 2018, Tomasz Figa wrote: > Hi Chad, > > On Wed, Jul 25, 2018 at 10:11 AM Chad Versace > wrote: > > > > Problem 1: u_debug_stack_android.cpp transitively included > > "pipe/p_compiler.h", but src/gallium/include was missing from the C++ > > include path. > > > > Problem 2: Add -std=c++11 to AM_CXXFLAGS. Android's libbacktrace headers > > require C++11, but the Android toolchain (at least in the Chrome OS SDK) > > does not enable C++11 by default. > > > > v2: Add -std=c++11. > > > > Cc: Gurchetan Singh > > Cc: Eric Engestrom > > --- > > src/gallium/auxiliary/Makefile.am | 6 +- > > 1 file changed, 5 insertions(+), 1 deletion(-) [snip] > > if HAVE_PLATFORM_ANDROID > > +# Android's libbacktrace headers required C++11, but the Android toolchain > > (at > > +# least in the Chrome OS SDK) does not enable C++11 by default. > > +AM_CXXFLAGS += $(CXX11_CXXFLAGS) > > + > > This is something that would normally be handled by the .pc file for > given library. Package build system shouldn't be polluted with such > system-specific low level dependencies. Normally, I would agree. "If libbacktrace needs a CXXFLAG, then put in the pc file". That's reasonable for most flags, because most flags are *additive*. But the -std flag is not. If backtrace.pc added '-std=c++11' to CXXFLAGS, but a different package required c++14, how should Mesa resolve that conflict? As precedent, I searched all pc files on my Debian machine. The only pc files that add -std to CFLAGS or CXXFLAGS are the icu-*.pc files. And those files should serve as anti-precedent in most cases; they are badly written compared to all other pc files I've seen. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] gallium/auxiliary: Fix Autotools on Android (v2)
Hi Chad, On Wed, Jul 25, 2018 at 10:11 AM Chad Versace wrote: > > Problem 1: u_debug_stack_android.cpp transitively included > "pipe/p_compiler.h", but src/gallium/include was missing from the C++ > include path. > > Problem 2: Add -std=c++11 to AM_CXXFLAGS. Android's libbacktrace headers > require C++11, but the Android toolchain (at least in the Chrome OS SDK) > does not enable C++11 by default. > > v2: Add -std=c++11. > > Cc: Gurchetan Singh > Cc: Eric Engestrom > --- > src/gallium/auxiliary/Makefile.am | 6 +- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/src/gallium/auxiliary/Makefile.am > b/src/gallium/auxiliary/Makefile.am > index 03908198772..4bfa7648389 100644 > --- a/src/gallium/auxiliary/Makefile.am > +++ b/src/gallium/auxiliary/Makefile.am > @@ -13,6 +13,7 @@ AM_CFLAGS = \ > $(MSVC2013_COMPAT_CFLAGS) > > AM_CXXFLAGS = \ > + $(GALLIUM_CFLAGS) \ > $(VISIBILITY_CXXFLAGS) \ > $(MSVC2013_COMPAT_CXXFLAGS) > > @@ -22,6 +23,10 @@ libgallium_la_SOURCES = \ > $(GENERATED_SOURCES) > > if HAVE_PLATFORM_ANDROID > +# Android's libbacktrace headers required C++11, but the Android toolchain > (at > +# least in the Chrome OS SDK) does not enable C++11 by default. > +AM_CXXFLAGS += $(CXX11_CXXFLAGS) > + This is something that would normally be handled by the .pc file for given library. Package build system shouldn't be polluted with such system-specific low level dependencies. Best regards, Tomasz ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] gallium/auxiliary: Fix Autotools on Android (v2)
Reviewed-by: Gurchetan Singh On Tue, Jul 24, 2018 at 6:11 PM Chad Versace wrote: > > Problem 1: u_debug_stack_android.cpp transitively included > "pipe/p_compiler.h", but src/gallium/include was missing from the C++ > include path. > > Problem 2: Add -std=c++11 to AM_CXXFLAGS. Android's libbacktrace headers > require C++11, but the Android toolchain (at least in the Chrome OS SDK) > does not enable C++11 by default. > > v2: Add -std=c++11. > > Cc: Gurchetan Singh > Cc: Eric Engestrom > --- > src/gallium/auxiliary/Makefile.am | 6 +- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/src/gallium/auxiliary/Makefile.am > b/src/gallium/auxiliary/Makefile.am > index 03908198772..4bfa7648389 100644 > --- a/src/gallium/auxiliary/Makefile.am > +++ b/src/gallium/auxiliary/Makefile.am > @@ -13,6 +13,7 @@ AM_CFLAGS = \ > $(MSVC2013_COMPAT_CFLAGS) > > AM_CXXFLAGS = \ > + $(GALLIUM_CFLAGS) \ > $(VISIBILITY_CXXFLAGS) \ > $(MSVC2013_COMPAT_CXXFLAGS) > > @@ -22,6 +23,10 @@ libgallium_la_SOURCES = \ > $(GENERATED_SOURCES) > > if HAVE_PLATFORM_ANDROID > +# Android's libbacktrace headers required C++11, but the Android toolchain > (at > +# least in the Chrome OS SDK) does not enable C++11 by default. > +AM_CXXFLAGS += $(CXX11_CXXFLAGS) > + > libgallium_la_SOURCES += util/u_debug_stack_android.cpp > endif > > @@ -41,7 +46,6 @@ AM_CFLAGS += \ > $(LLVM_CFLAGS) > > AM_CXXFLAGS += \ > - $(GALLIUM_CFLAGS) \ > $(LLVM_CXXFLAGS) > > libgallium_la_SOURCES += \ > -- > 2.18.0.233.g985f88cf7e-goog > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] gallium/auxiliary: Fix Autotools on Android (v2)
Problem 1: u_debug_stack_android.cpp transitively included "pipe/p_compiler.h", but src/gallium/include was missing from the C++ include path. Problem 2: Add -std=c++11 to AM_CXXFLAGS. Android's libbacktrace headers require C++11, but the Android toolchain (at least in the Chrome OS SDK) does not enable C++11 by default. v2: Add -std=c++11. Cc: Gurchetan Singh Cc: Eric Engestrom --- src/gallium/auxiliary/Makefile.am | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/gallium/auxiliary/Makefile.am b/src/gallium/auxiliary/Makefile.am index 03908198772..4bfa7648389 100644 --- a/src/gallium/auxiliary/Makefile.am +++ b/src/gallium/auxiliary/Makefile.am @@ -13,6 +13,7 @@ AM_CFLAGS = \ $(MSVC2013_COMPAT_CFLAGS) AM_CXXFLAGS = \ + $(GALLIUM_CFLAGS) \ $(VISIBILITY_CXXFLAGS) \ $(MSVC2013_COMPAT_CXXFLAGS) @@ -22,6 +23,10 @@ libgallium_la_SOURCES = \ $(GENERATED_SOURCES) if HAVE_PLATFORM_ANDROID +# Android's libbacktrace headers required C++11, but the Android toolchain (at +# least in the Chrome OS SDK) does not enable C++11 by default. +AM_CXXFLAGS += $(CXX11_CXXFLAGS) + libgallium_la_SOURCES += util/u_debug_stack_android.cpp endif @@ -41,7 +46,6 @@ AM_CFLAGS += \ $(LLVM_CFLAGS) AM_CXXFLAGS += \ - $(GALLIUM_CFLAGS) \ $(LLVM_CXXFLAGS) libgallium_la_SOURCES += \ -- 2.18.0.233.g985f88cf7e-goog ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
