From 1d7c6068df5a08524a826c84d4fabdbbbc938a6c Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=A9mie=20Galarneau?= Date: Wed, 1 Apr 2020 10:55:04 -0400 Subject: [PATCH] bt2: honor build system compiler/linker preferences MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Cherry-pick note: I see the following build failure on Arch Linux: ccache gcc -DNDEBUG -g -fwrapv -O3 -Wall -march=x86-64 -mtune=generic -O3 -pipe -fno-plt -fexceptions -Wp,-D_FORTIFY_SOURCE=2 -Wformat -Werror=format-security -fstack-clash-protection -fcf-protection -g -ffile-prefix-map=/build/python/src=/usr/src/debug/python -flto=auto -ffat-lto-objects -march=x86-64 -mtune=generic -O3 -pipe -fno-plt -fexceptions -Wp,-D_FORTIFY_SOURCE=2 -Wformat -Werror=format-security -fstack-clash-protection -fcf-protection -g -ffile-prefix-map=/build/python/src=/usr/src/debug/python -flto=auto -march=x86-64 -mtune=generic -O3 -pipe -fno-plt -fexceptions -Wp,-D_FORTIFY_SOURCE=2 -Wformat -Werror=format-security -fstack-clash-protection -fcf-protection -g -ffile-prefix-map=/build/python/src=/usr/src/debug/python -flto=auto -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/sysprof-4 -pthread -pthread -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/sysprof-4 -pthread -fno-strict-aliasing -Wnested-externs -Wmissing-prototypes -Wstrict-prototypes -Wdeclaration-after-statement -Wimplicit-function-declaration -Wold-style-definition -Wjump-misses-init -Wall -Wextra -Wundef -Wwrite-strings -Wpointer-arith -Wmissing-declarations -Wredundant-decls -Wno-unused-parameter -Wno-missing-field-initializers -Wformat=2 -Wcast-align -Wformat-nonliteral -Wformat-security -Wsign-compare -Wstrict-aliasing -Wshadow -Winline -Wpacked -Wmissing-format-attribute -Wmissing-noreturn -Winit-self -Wmissing-include-dirs -Wunused-but-set-variable -Warray-bounds -Wreturn-type -Wswitch-enum -Wswitch-default -Wduplicated-cond -Wduplicated-branches -Wlogical-op -Wrestrict -Wnull-dereference -Wdouble-promotion -Wno-sign-compare -Wno-inline -Wno-declaration-after-statement -Wno-switch-enum -Wno-switch-default -Wno-packed -Wno-pointer-arith -Wno-format-nonliteral -Wno-double-promotion -Wno-cast-align -Wno-cast-function-type -Wno-suggest-attribute=format -Werror -Wno-error=unused-parameter -Wno-error=missing-field-initializers -Wno-error=sign-compare -Wno-error=inline -Wno-error=declaration-after-statement -Wno-error=switch-enum -Wno-error=switch-default -Wno-error=packed -Wno-error=pointer-arith -Wno-error=format-nonliteral -Wno-error=double-promotion -Wno-error=cast-align -Wno-error=cast-function-type -Wold-style-definition -Werror=implicit-function-declaration -g3 -O0 -fmax-errors=1 -fdiagnostics-color=always -Wno-shadow -Wno-null-dereference -Wno-deprecated-declarations -Wno-redundant-decls -I/home/smarchi/src/babeltrace/include -I../../../../src -I/home/smarchi/src/babeltrace/src -include common/config.h -I/home/smarchi/src/babeltrace/src/bindings/python/bt2/bt2 -fPIC -I/usr/include/python3.11 -c /home/smarchi/src/babeltrace/src/bindings/python/bt2/bt2/logging.c -o build/temp.linux-x86_64-3.11/home/smarchi/src/babeltrace/src/bindings/python/bt2/bt2/logging.o In file included from /usr/include/errno.h:25, from /home/smarchi/src/babeltrace/src/logging/log.h:12, from /home/smarchi/src/babeltrace/src/bindings/python/bt2/bt2/logging.c:24: /usr/include/features.h:413:4: error: #warning _FORTIFY_SOURCE requires compiling with optimization (-O) [-Werror=cpp] 413 | # warning _FORTIFY_SOURCE requires compiling with optimization (-O) | ^~~~~~~ Cherry-picking this patch fixes it. And now the original commit message: This is an attempt to fix the dreaded "compiling the Python native module with a custom compiler or compiler-specific flag" problem. The problem may arise when building with a CC different than what the Python interpreter was compiled with. It can manifests itself in two different ways, as far as I know. But in both cases, it's due to us overriding the compiler and compiler flags when invoking setup.py to build the native modules, and how distutils invokes the compiler and linker to build the native modules. Scenario 1 ---------- Things fail when trying to compile with a compiler other than what Python has been compiled with, and using in `CFLAGS` a compiler flag unknown to the compiler Python has been compiled with. For example, on Ubuntu 18.04, we get this when building with CC=clang (the -Wtautological-constant-out-of-range-compare is a clang-specific warning flag currently specified in configure.ac): x86_64-linux-gnu-gcc ... -Wtautological-constant-out-of-range-compare ... -o build/build_lib/bt2/_native_bt.cpython-38-x86_64-linux-gnu.so x86_64-linux-gnu-gcc: error: unrecognized command line option ‘-Wtautological-constant-out-of-range-compare’ error: command 'x86_64-linux-gnu-gcc' failed with exit status 1 This is due to the fact that: - The default link command used by distutils to link shared objects uses its default compiler driver (gcc on Linux) - It passes `CFLAGS` to the linker Because of this, we end up passing a clang-specific warning flag to gcc, which proceeds to error out. To fix this, I have considered setting the LDSHARED variable to override the command used for linking shared objects. If the compiler driver used to drive the linking is the same as the compiler driver used for compiling, it should understand all the passed flags. However, it is not obvious to do this right, especially, considering the various platforms. But most importantly, it is doesn't help with the next scenario. Scenario 2 ---------- On Arch Linux (and presumably on Fedora too [1]), the flags passed by distutils to the compiler contain `-fno-semantic-interposition`, which is a gcc-specific flag. When building with CC=clang, we therefore get this when compiling: building 'bt2._native_bt' extension clang ... -fno-semantic-interposition ... -c bt2/native_bt.c -o build/temp.linux-x86_64-3.8/bt2/native_bt.o clang-9: error: unknown argument: '-fno-semantic-interposition' clang errors out, because it gets passed a gcc-specific flag. [1] https://fedoraproject.org/wiki/Changes/PythonNoSemanticInterpositionSpeedup The fix ------- Two important distutils configuration variables are at play in this problem: - LDSHARED: defines a command to link a build target - CFLAGS: contains the compiler flags used to build the python interpreter. The configuration variables are changed on access by overriding distutils' get_config_vars(). In the case of `LDSHARED`, distutils allows it to be specified as an environment variable which would probably be cleaner (if it actually works, I haven't tested). distutils seems to honor the environment `CC` _sometimes_ (depending on whether or not it has performed the `customize_compiler` step?) The override always uses the environment `CC` and completely foregoes the `LDSHARED` configuration variable if `LDFLAGS` are provided. If no `LDFLAGS` are specified, the compiler is replaced in the LDSHARED command. Unfortunately, the CFLAGS environment variable is only appended to the Python interpreter's build-time CFLAGS, which doesn't solve our problem. get_cflags() completely overrides the CFLAGS configuration variable to use babeltrace's build system-specified CFLAGS. Drawbacks --------- I can't think of a scenario where this doesn't work, but I'm afraid this will break in non-trivial build environments. Signed-off-by: Jérémie Galarneau Change-Id: If3142ddb228758e63a986e735cb8f9d89dd39b67 Reviewed-on: https://review.lttng.org/c/babeltrace/+/3286 Tested-by: jenkins (cherry picked from commit afdd1f8201f579d81959ce85759dcc62fcc0a85d) --- src/bindings/python/bt2/Makefile.am | 2 +- src/bindings/python/bt2/setup.py.in | 68 +++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 1 deletion(-) diff --git a/src/bindings/python/bt2/Makefile.am b/src/bindings/python/bt2/Makefile.am index 0e9b9a3f..20f142e4 100644 --- a/src/bindings/python/bt2/Makefile.am +++ b/src/bindings/python/bt2/Makefile.am @@ -123,7 +123,7 @@ WARN_CFLAGS += -Wno-redundant-decls BUILD_FLAGS=CC="$(CC)" \ CFLAGS="$(GLIB_CFLAGS) $(AM_CFLAGS) $(CFLAGS) $(WARN_CFLAGS)" \ CPPFLAGS="$(AM_CPPFLAGS) $(CPPFLAGS) -I$(srcdir)/bt2" \ - LDFLAGS="$(AM_LDFLAGS) $(LDFLAGS) $(GLIB_LIBS) $(LIBS)" + LDFLAGS="$(AM_LDFLAGS) $(LDFLAGS) $(GLIB_LIBS) $(PYTHON_LDFLAGS) $(LIBS)" all-local: build-python-bindings.stamp diff --git a/src/bindings/python/bt2/setup.py.in b/src/bindings/python/bt2/setup.py.in index 620f5194..acb64509 100644 --- a/src/bindings/python/bt2/setup.py.in +++ b/src/bindings/python/bt2/setup.py.in @@ -1,6 +1,7 @@ # The MIT License (MIT) # # Copyright (C) 2017 - Francis Deslauriers +# Copyright (C) 2020 - Jérémie Galarneau # # Permission is hereby granted, free of charge, to any person obtaining a copy # of this software and associated documentation files (the "Software"), to deal @@ -21,6 +22,8 @@ # THE SOFTWARE. import sys +import os +import distutils.sysconfig from distutils.core import setup, Extension @@ -34,6 +37,71 @@ following command to your .bashrc/.zshrc: -------------------------------------------------------------------------------- """ +original_get_config_vars = distutils.sysconfig.get_config_vars + + +def get_cflags(): + cflags = os.environ.get('CFLAGS') + + if cflags is None: + [cflags] = original_get_config_vars('CFLAGS') + + return cflags + + +# distutils performs a similar transformation step on LDSHARED on +# darwin to use the overriden CC as the default command for LDSHARED +# (see distutils' customize_compiler() step in the sysconfig module). +# +# This takes it a step further by using our own LDFLAGS (when available) +# along with the overriden compiler and ensure that flags that are unsupported +# by either the Python interprter's CC or the overriden CC don't cause a +# build failure. +def get_ldshared(): + cc = os.environ.get('CC') + ldflags = os.environ.get('LDFLAGS') + [py_cc] = original_get_config_vars('CC') + [py_ldshared] = original_get_config_vars('LDSHARED') + + if not py_ldshared.startswith(py_cc): + return py_ldshared + + if cc and ldflags: + return '{} -shared {}'.format(cc, ldflags) + elif cc: + return cc + py_ldshared[len(py_cc) :] + elif ldflags: + return py_cc + py_ldshared[len(py_cc) :] + else: + return py_ldshared + + +def our_get_config_vars(*args): + overridden_config_vars = { + 'CFLAGS': get_cflags(), + 'LDSHARED': get_ldshared(), + } + + if len(args) == 0: + all_config_vars = original_get_config_vars() + for name in overridden_config_vars: + all_config_vars[name] = overridden_config_vars[name] + return all_config_vars + + subset_config_vars = [] + for name in args: + if name not in overridden_config_vars: + [var] = original_get_config_vars(name) + subset_config_vars.append(var) + continue + + subset_config_vars.append(overridden_config_vars[name]) + + return subset_config_vars + + +distutils.sysconfig.get_config_vars = our_get_config_vars + def main(): babeltrace_ext = Extension( -- 2.34.1