Skip to content

Commit

Permalink
fix: prepend Pybind11Extension flags rather than appending them. (pyb…
Browse files Browse the repository at this point in the history
…ind#2808)

This allows users to set e.g. `extra_compile_args=["-g"]` (temporarily,
for debugging purposes) and not have that get overridden by
Pybind11Extension's default `-g0`.

In order to minimize "order inversion" (not that it really matters,
though) I chose to collect all flags and add them at once (hence the
dropping of `*args`).  I also dropped flag deduplication, which seems
unneeded.
  • Loading branch information
anntzer authored Jan 19, 2021
1 parent 4853408 commit e8c4f54
Showing 1 changed file with 25 additions and 18 deletions.
43 changes: 25 additions & 18 deletions pybind11/setup_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,15 +99,14 @@ class Pybind11Extension(_Extension):
this is an ugly old-style class due to Distutils.
"""

def _add_cflags(self, *flags):
for flag in flags:
if flag not in self.extra_compile_args:
self.extra_compile_args.append(flag)
# flags are prepended, so that they can be further overridden, e.g. by
# ``extra_compile_args=["-g"]``.

def _add_lflags(self, *flags):
for flag in flags:
if flag not in self.extra_link_args:
self.extra_link_args.append(flag)
def _add_cflags(self, flags):
self.extra_compile_args[:0] = flags

def _add_ldflags(self, flags):
self.extra_link_args[:0] = flags

def __init__(self, *args, **kwargs):

Expand Down Expand Up @@ -139,13 +138,17 @@ def __init__(self, *args, **kwargs):
# Have to use the accessor manually to support Python 2 distutils
Pybind11Extension.cxx_std.__set__(self, cxx_std)

cflags = []
ldflags = []
if WIN:
self._add_cflags("/EHsc", "/bigobj")
cflags += ["/EHsc", "/bigobj"]
else:
self._add_cflags("-fvisibility=hidden", "-g0")
cflags += ["-fvisibility=hidden", "-g0"]
if MACOS:
self._add_cflags("-stdlib=libc++")
self._add_lflags("-stdlib=libc++")
cflags += ["-stdlib=libc++"]
ldflags += ["-stdlib=libc++"]
self._add_cflags(cflags)
self._add_ldflags(ldflags)

@property
def cxx_std(self):
Expand Down Expand Up @@ -174,7 +177,8 @@ def cxx_std(self, level):
if not level:
return

self.extra_compile_args.append(STD_TMPL.format(level))
cflags = [STD_TMPL.format(level)]
ldflags = []

if MACOS and "MACOSX_DEPLOYMENT_TARGET" not in os.environ:
# C++17 requires a higher min version of macOS. An earlier version
Expand All @@ -186,18 +190,21 @@ def cxx_std(self, level):
desired_macos = (10, 9) if level < 17 else (10, 14)
macos_string = ".".join(str(x) for x in min(current_macos, desired_macos))
macosx_min = "-mmacosx-version-min=" + macos_string
self.extra_compile_args.append(macosx_min)
self.extra_link_args.append(macosx_min)
cflags += [macosx_min]
ldflags += [macosx_min]

if PY2:
if WIN:
# Will be ignored on MSVC 2015, where C++17 is not supported so
# this flag is not valid.
self.extra_compile_args.append("/wd5033")
cflags += ["/wd5033"]
elif level >= 17:
self.extra_compile_args.append("-Wno-register")
cflags += ["-Wno-register"]
elif level >= 14:
self.extra_compile_args.append("-Wno-deprecated-register")
cflags += ["-Wno-deprecated-register"]

self._add_cflags(cflags)
self._add_ldflags(ldflags)


# Just in case someone clever tries to multithread
Expand Down

0 comments on commit e8c4f54

Please sign in to comment.