Skip to content

Commit

Permalink
[Backport-to-1.70.x] Fix python build script to handle C and C++ std …
Browse files Browse the repository at this point in the history
…options properly for MSVC (Backport of #38410) (#38425)
  • Loading branch information
veblush authored Jan 10, 2025
1 parent 6c8df89 commit f1e4dfa
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 66 deletions.
7 changes: 4 additions & 3 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -263,19 +263,20 @@ def check_linker_need_libatomic():
if EXTRA_ENV_COMPILE_ARGS is None:
EXTRA_ENV_COMPILE_ARGS = ""
if "win32" in sys.platform:
# MSVC by defaults uses C++14 so C11 needs to be specified.
# MSVC by defaults uses C++14 and C89 so both needs to be configured.
EXTRA_ENV_COMPILE_ARGS += " /std:c++17"
EXTRA_ENV_COMPILE_ARGS += " /std:c11"
# We need to statically link the C++ Runtime, only the C runtime is
# available dynamically
EXTRA_ENV_COMPILE_ARGS += " /MT"
elif "linux" in sys.platform:
# GCC by defaults uses C17 so only C++14 needs to be specified.
# GCC by defaults uses C17 so only C++17 needs to be specified.
EXTRA_ENV_COMPILE_ARGS += " -std=c++17"
EXTRA_ENV_COMPILE_ARGS += (
" -fvisibility=hidden -fno-wrapv -fno-exceptions"
)
elif "darwin" in sys.platform:
# AppleClang by defaults uses C17 so only C++14 needs to be specified.
# AppleClang by defaults uses C17 so only C++17 needs to be specified.
EXTRA_ENV_COMPILE_ARGS += " -std=c++17"
EXTRA_ENV_COMPILE_ARGS += (
" -stdlib=libc++ -fvisibility=hidden -fno-wrapv -fno-exceptions"
Expand Down
8 changes: 8 additions & 0 deletions src/python/grpcio/_spawn_patch.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,14 @@


def _commandfile_spawn(self, command, **kwargs):
if os.name == "nt":
if any(arg.startswith("/Tc") for arg in command):
# Remove /std:c++17 option if this is a MSVC C complation
command = [arg for arg in command if arg != "/std:c++17"]
elif any(arg.startswith("/Tp") for arg in command):
# Remove /std:c11 option if this is a MSVC C++ complation
command = [arg for arg in command if arg != "/std:c11"]

command_length = sum([len(arg) for arg in command])
if os.name == "nt" and command_length > MAX_COMMAND_LENGTH:
# Even if this command doesn't support the @command_file, it will
Expand Down
63 changes: 16 additions & 47 deletions src/python/grpcio/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -248,53 +248,22 @@ def get_ext_filename(self, ext_name):
return filename

def build_extensions(self):
def compiler_ok_with_extra_std():
"""Test if default compiler is okay with specifying c++ version
when invoked in C mode. GCC is okay with this, while clang is not.
"""
try:
# TODO(lidiz) Remove the generated a.out for success tests.
cc = os.environ.get("CC", "cc")
cc_test = subprocess.Popen(
[cc, "-x", "c", "-std=c++17", "-"],
stdin=subprocess.PIPE,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
)
_, cc_err = cc_test.communicate(input=b"int main(){return 0;}")
return not "invalid argument" in str(cc_err)
except:
sys.stderr.write(
"Non-fatal exception:" + traceback.format_exc() + "\n"
)
return False

# This special conditioning is here due to difference of compiler
# behavior in gcc and clang. The clang doesn't take --stdc++11
# flags but gcc does. Since the setuptools of Python only support
# all C or all C++ compilation, the mix of C and C++ will crash.
# *By default*, macOS and FreeBSD use clang and Linux use gcc
#
# If we are not using a permissive compiler that's OK with being
# passed wrong std flags, swap out compile function by adding a filter
# for it.
if not compiler_ok_with_extra_std():
old_compile = self.compiler._compile

def new_compile(obj, src, ext, cc_args, extra_postargs, pp_opts):
if src.endswith(".c"):
extra_postargs = [
arg for arg in extra_postargs if "-std=c++" not in arg
]
elif src.endswith(".cc") or src.endswith(".cpp"):
extra_postargs = [
arg for arg in extra_postargs if "-std=gnu99" not in arg
]
return old_compile(
obj, src, ext, cc_args, extra_postargs, pp_opts
)

self.compiler._compile = new_compile
# This is to let UnixCompiler get either C or C++ compiler options depending on the source.
# Note that this doesn't work for MSVCCompiler and will be handled by _spawn_patch.py.
old_compile = self.compiler._compile

def new_compile(obj, src, ext, cc_args, extra_postargs, pp_opts):
if src.endswith(".c"):
extra_postargs = [
arg for arg in extra_postargs if arg != "-std=c++17"
]
elif src.endswith((".cc", ".cpp")):
extra_postargs = [
arg for arg in extra_postargs if arg != "-std=c11"
]
return old_compile(obj, src, ext, cc_args, extra_postargs, pp_opts)

self.compiler._compile = new_compile

compiler = self.compiler.compiler_type
if compiler in BuildExt.C_OPTIONS:
Expand Down
2 changes: 1 addition & 1 deletion tools/distrib/gen_compilation_database.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ def modifyCompileCommand(target, args):
options += " -Wno-pragma-once-outside-header -Wno-unused-const-variable"
options += " -Wno-unused-function"
if not target["file"].startswith("external/"):
# *.h file is treated as C header by default while our headers files are all C++14.
# *.h file is treated as C header by default while our headers files are all C++17.
options = "-x c++ -std=c++17 -fexceptions " + options

target["command"] = " ".join([cc, options])
Expand Down
8 changes: 8 additions & 0 deletions tools/distrib/python/grpcio_tools/_spawn_patch.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,14 @@


def _commandfile_spawn(self, command, **kwargs):
if os.name == "nt":
if any(arg.startswith("/Tc") for arg in command):
# Remove /std:c++17 option if this is a MSVC C complation
command = [arg for arg in command if arg != "/std:c++17"]
elif any(arg.startswith("/Tp") for arg in command):
# Remove /std:c11 option if this is a MSVC C++ complation
command = [arg for arg in command if arg != "/std:c11"]

command_length = sum([len(arg) for arg in command])
if os.name == "nt" and command_length > MAX_COMMAND_LENGTH:
# Even if this command doesn't support the @command_file, it will
Expand Down
24 changes: 9 additions & 15 deletions tools/distrib/python/grpcio_tools/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,25 +130,18 @@ def get_ext_filename(self, ext_name):
return filename

def build_extensions(self):
# This special conditioning is here due to difference of compiler
# behavior in gcc and clang. The clang doesn't take --stdc++11
# flags but gcc does. Since the setuptools of Python only support
# all C or all C++ compilation, the mix of C and C++ will crash.
# *By default*, macOS and FreeBSD use clang and Linux use gcc
#
# If we are not using a permissive compiler that's OK with being
# passed wrong std flags, swap out compile function by adding a filter
# for it.
# This is to let UnixCompiler get either C or C++ compiler options depending on the source.
# Note that this doesn't work for MSVCCompiler and will be handled by _spawn_patch.py.
old_compile = self.compiler._compile

def new_compile(obj, src, ext, cc_args, extra_postargs, pp_opts):
if src.endswith(".c"):
extra_postargs = [
arg for arg in extra_postargs if "-std=c++" not in arg
arg for arg in extra_postargs if arg != "-std=c++17"
]
elif src.endswith(".cc") or src.endswith(".cpp"):
elif src.endswith((".cc", ".cpp")):
extra_postargs = [
arg for arg in extra_postargs if "-std=c11" not in arg
arg for arg in extra_postargs if arg != "-std=c11"
]
return old_compile(obj, src, ext, cc_args, extra_postargs, pp_opts)

Expand Down Expand Up @@ -184,20 +177,21 @@ def new_compile(obj, src, ext, cc_args, extra_postargs, pp_opts):
if EXTRA_ENV_COMPILE_ARGS is None:
EXTRA_ENV_COMPILE_ARGS = ""
if "win32" in sys.platform:
# MSVC by defaults uses C++14 so C11 needs to be specified.
# MSVC by defaults uses C++14 and C89 so both needs to be configured.
EXTRA_ENV_COMPILE_ARGS += " /std:c++17"
EXTRA_ENV_COMPILE_ARGS += " /std:c11"
# We need to statically link the C++ Runtime, only the C runtime is
# available dynamically
EXTRA_ENV_COMPILE_ARGS += " /MT"
elif "linux" in sys.platform:
# GCC by defaults uses C17 so only C++14 needs to be specified.
# GCC by defaults uses C17 so only C++17 needs to be specified.
EXTRA_ENV_COMPILE_ARGS += " -std=c++17"
EXTRA_ENV_COMPILE_ARGS += " -fno-wrapv -frtti"
# Reduce the optimization level from O3 (in many cases) to O1 to
# workaround gcc misalignment bug with MOVAPS (internal b/329134877)
EXTRA_ENV_COMPILE_ARGS += " -O1"
elif "darwin" in sys.platform:
# AppleClang by defaults uses C17 so only C++14 needs to be specified.
# AppleClang by defaults uses C17 so only C++17 needs to be specified.
EXTRA_ENV_COMPILE_ARGS += " -std=c++17"
EXTRA_ENV_COMPILE_ARGS += " -fno-wrapv -frtti"
EXTRA_ENV_COMPILE_ARGS += " -stdlib=libc++ -DHAVE_UNISTD_H"
Expand Down

0 comments on commit f1e4dfa

Please sign in to comment.