From 48f0b9def2f7f66ec1d74dc82a0d84f915a1b0a1 Mon Sep 17 00:00:00 2001 From: kon72 Date: Sun, 31 Dec 2023 18:06:17 +0900 Subject: [PATCH 01/11] Get the arguments emcc would pass to clang --- BUILD | 11 ++++- print_args.bat | 3 ++ print_args.py | 19 ++++++++ print_args.sh | 3 ++ refresh.template.py | 92 +++++++++++++++++++++++++++--------- refresh_compile_commands.bzl | 5 ++ 6 files changed, 109 insertions(+), 24 deletions(-) create mode 100644 print_args.bat create mode 100644 print_args.py create mode 100755 print_args.sh diff --git a/BUILD b/BUILD index 2578c6e..0bf7bd3 100644 --- a/BUILD +++ b/BUILD @@ -25,4 +25,13 @@ filegroup( # Implementation: # If you are looking into the implementation, start with the overview in ImplementationReadme.md. -exports_files(["refresh.template.py", "check_python_version.template.py"]) # For implicit use by the refresh_compile_commands macro, not direct use. +exports_files(["refresh.template.py", "check_python_version.template.py", "print_args.py"]) # For implicit use by the refresh_compile_commands macro, not direct use. + +filegroup( + name = "print_args", + srcs = select({ + "@bazel_tools//src/conditions:host_windows": [":print_args.bat"], + "//conditions:default": [":print_args.sh"], + }), + visibility = ["//visibility:public"], +) diff --git a/print_args.bat b/print_args.bat new file mode 100644 index 0000000..a61af60 --- /dev/null +++ b/print_args.bat @@ -0,0 +1,3 @@ +@ECHO OFF + +py -3 %HEDRON_COMPILE_COMMANDS_PRINT_ARGS_PY% %* diff --git a/print_args.py b/print_args.py new file mode 100644 index 0000000..694ce95 --- /dev/null +++ b/print_args.py @@ -0,0 +1,19 @@ +""" +Prints the arguments passed to the script +""" + +import sys + + +def main(): + print('===HEDRON_COMPILE_COMMANDS_BEGIN_ARGS===') + for arg in sys.argv[1:]: + print(arg) + print('===HEDRON_COMPILE_COMMANDS_END_ARGS===') + + # We purposely return a non-zero exit code to have the emcc process exit after running this fake clang wrapper. + sys.exit(1) + + +if __name__ == '__main__': + main() diff --git a/print_args.sh b/print_args.sh new file mode 100755 index 0000000..a6a8ede --- /dev/null +++ b/print_args.sh @@ -0,0 +1,3 @@ +#!/bin/bash + +exec python3 "${HEDRON_COMPILE_COMMANDS_PRINT_ARGS_PY}" "$@" diff --git a/refresh.template.py b/refresh.template.py index 6ba93f1..3461414 100644 --- a/refresh.template.py +++ b/refresh.template.py @@ -283,17 +283,6 @@ def _get_headers_gcc(compile_args: typing.List[str], source_path: str, action_ke return headers, should_cache -@functools.lru_cache(maxsize=None) -def _get_clang_or_gcc(): - """Returns clang or gcc, if you have one of them on your path.""" - if shutil.which('clang'): - return 'clang' - elif shutil.which('gcc'): - return 'gcc' - else: - return None - - def windows_list2cmdline(seq): """ Copied from list2cmdline in https://github.com/python/cpython/blob/main/Lib/subprocess.py because we need it but it's not exported as part of the public API. @@ -567,18 +556,7 @@ def _get_headers(compile_action, source_path: str): if compile_action.arguments[0].endswith('cl.exe'): # cl.exe and also clang-cl.exe headers, should_cache = _get_headers_msvc(compile_action.arguments, source_path) else: - # Emscripten is tricky. There isn't an easy way to make it emcc run without lots of environment variables. - # So...rather than doing our usual script unwrapping, we just swap in clang/gcc and use that to get headers, knowing that they'll accept the same argument format. - # You can unwrap emcc.sh to emcc.py via next(pathlib.Path('external').glob('emscripten_bin_*/emscripten/emcc.py')).as_posix() - # But then the underlying emcc needs a configuration file that itself depends on lots of environment variables. - # If we ever pick this back up, note that you can supply that config via compile_args += ["--em-config", "external/emsdk/emscripten_toolchain/emscripten_config"] - args = compile_action.arguments - if args[0].endswith('emcc.sh') or args[0].endswith('emcc.bat'): - alternate_compiler = _get_clang_or_gcc() - if not alternate_compiler: return set() # Skip getting headers. - args = args.copy() - args[0] = alternate_compiler - headers, should_cache = _get_headers_gcc(args, source_path, compile_action.actionKey) + headers, should_cache = _get_headers_gcc(compile_action.arguments, source_path, compile_action.actionKey) # Cache for future use if output_file and should_cache: @@ -767,6 +745,73 @@ def _apple_platform_patch(compile_args: typing.List[str]): return compile_args +def _get_sysroot(args: typing.List[str]): + for idx, arg in enumerate(args): + if arg == '--sysroot' or arg == '-isysroot': + if idx + 1 < len(args): + return pathlib.PurePath(args[idx + 1]) + elif arg.startswith('--sysroot='): + return pathlib.PurePath(arg[len('--sysroot='):]) + elif arg.startswith('-isysroot'): + return pathlib.PurePath(arg[len('-isysroot'):]) + return None + + +def _emscripten_platform_patch(compile_args: typing.List[str]): + """De-Bazel the command into something clangd can parse. + + This function has fixes specific to Emscripten platforms, but you should call it on all platforms. It'll determine whether the fixes should be applied or not + """ + emcc_driver = pathlib.Path(compile_args[0]) + if emcc_driver.name != 'emcc.sh' and emcc_driver.name != 'emcc.bat': + return compile_args + + workspace_absolute = pathlib.PurePath(os.environ["BUILD_WORKSPACE_DIRECTORY"]) + sysroot = _get_sysroot(compile_args) + assert sysroot, f'Emscripten sysroot not detected in CMD: {compile_args}' + + def get_workspace_root(path_from_execroot: pathlib.PurePath): + assert path_from_execroot.parts[0] == 'external' + return pathlib.PurePath('external') / path_from_execroot.parts[1] + + environment = { + 'EXT_BUILD_ROOT': str(workspace_absolute), + 'EM_BIN_PATH': str(get_workspace_root(sysroot)), + 'EM_CONFIG_PATH': str(get_workspace_root(emcc_driver) / 'emscripten_toolchain' / 'emscripten_config'), + 'EMCC_SKIP_SANITY_CHECK': '1', + 'EM_COMPILER_WRAPPER': str(pathlib.PurePath({print_args_executable})), + 'HEDRON_COMPILE_COMMANDS_PRINT_ARGS_PY': str(pathlib.PurePath({print_args_py})), + 'PATH': os.environ['PATH'], + } + + # We run the emcc process with the environment variable EM_COMPILER_WRAPPER to intercept the command line arguments passed to `clang`. + emcc_process = subprocess.run( + [emcc_driver] + compile_args[1:], + # MIN_PY=3.7: Replace PIPEs with capture_output. + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + env=environment, + encoding=locale.getpreferredencoding(), + check=False, # We explicitly ignore errors and carry on. + ) + + lines = emcc_process.stdout.splitlines() + + # Parse the arguments from the output of the emcc process. + if BEGIN_ARGS_MARKER in lines: + begin_args_idx = lines.index(BEGIN_ARGS_MARKER) + end_args_idx = lines.index(END_ARGS_MARKER, begin_args_idx + 1) + args = lines[begin_args_idx + 1:end_args_idx] + clang_driver = pathlib.PurePath(args[0]) + if _is_relative_to(clang_driver, workspace_absolute): + args[0] = clang_driver.relative_to(workspace_absolute).as_posix() + return args + + assert False, f'Failed to parse emcc output: {emcc_process.stderr}' +BEGIN_ARGS_MARKER = '===HEDRON_COMPILE_COMMANDS_BEGIN_ARGS===' +END_ARGS_MARKER = '===HEDRON_COMPILE_COMMANDS_END_ARGS===' + + def _all_platform_patch(compile_args: typing.List[str]): """Apply de-Bazeling fixes to the compile command that are shared across target platforms.""" # clangd writes module cache files to the wrong place @@ -1020,6 +1065,7 @@ def _get_cpp_command_for_files(compile_action): # Patch command by platform compile_action.arguments = _all_platform_patch(compile_action.arguments) compile_action.arguments = _apple_platform_patch(compile_action.arguments) + compile_action.arguments = _emscripten_platform_patch(compile_action.arguments) # Android and Linux and grailbio LLVM toolchains: Fine as is; no special patching needed. source_files, header_files = _get_files(compile_action) diff --git a/refresh_compile_commands.bzl b/refresh_compile_commands.bzl index 537c9fc..1f1fc79 100644 --- a/refresh_compile_commands.bzl +++ b/refresh_compile_commands.bzl @@ -98,6 +98,7 @@ def refresh_compile_commands( version_checker_script_name, script_name, ], + data = ["@hedron_compile_commands//:print_args"], imports = [''], # Allows binary to import templated script, even if this macro is being called inside a sub package. See https://github.com/hedronvision/bazel-compile-commands-extractor/issues/137 **kwargs ) @@ -115,6 +116,8 @@ def _expand_template_impl(ctx): " {windows_default_include_paths}": "\n".join([" %r," % path for path in find_cpp_toolchain(ctx).built_in_include_directories]), # find_cpp_toolchain is from https://docs.bazel.build/versions/main/integrating-with-rules-cc.html "{exclude_headers}": repr(ctx.attr.exclude_headers), "{exclude_external_sources}": repr(ctx.attr.exclude_external_sources), + "{print_args_executable}": repr(ctx.executable._print_args_executable.path), + "{print_args_py}": repr(ctx.file._print_args_py.path), }, ) return DefaultInfo(files = depset([script])) @@ -125,6 +128,8 @@ _expand_template = rule( "exclude_external_sources": attr.bool(default = False), "exclude_headers": attr.string(values = ["all", "external", ""]), # "" needed only for compatibility with Bazel < 3.6.0 "_script_template": attr.label(allow_single_file = True, default = "refresh.template.py"), + "_print_args_executable": attr.label(executable = True, allow_single_file = True, cfg = "target", default = "print_args"), + "_print_args_py": attr.label(allow_single_file = True, default = "print_args.py"), # For Windows INCLUDE. If this were eliminated, for example by the resolution of https://github.com/clangd/clangd/issues/123, we'd be able to just use a macro and skylib's expand_template rule: https://github.com/bazelbuild/bazel-skylib/pull/330 # Once https://github.com/bazelbuild/bazel/pull/17108 is widely released, we should be able to eliminate this and get INCLUDE directly. Perhaps for 7.0? Should be released in the sucessor to 6.0 "_cc_toolchain": attr.label(default = "@bazel_tools//tools/cpp:current_cc_toolchain"), From 957881674ffd1cbd0e69d1cb3979af89cb044d37 Mon Sep 17 00:00:00 2001 From: Chris Sauer Date: Sun, 31 Dec 2023 14:05:41 -0800 Subject: [PATCH 02/11] EMCC: Minor: Slightly more flexible detection in case they change extension or add suffixes in the future --- refresh.template.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/refresh.template.py b/refresh.template.py index 3461414..38a6d1e 100644 --- a/refresh.template.py +++ b/refresh.template.py @@ -763,7 +763,7 @@ def _emscripten_platform_patch(compile_args: typing.List[str]): This function has fixes specific to Emscripten platforms, but you should call it on all platforms. It'll determine whether the fixes should be applied or not """ emcc_driver = pathlib.Path(compile_args[0]) - if emcc_driver.name != 'emcc.sh' and emcc_driver.name != 'emcc.bat': + if not emcc_driver.name.startswith('emcc'): return compile_args workspace_absolute = pathlib.PurePath(os.environ["BUILD_WORKSPACE_DIRECTORY"]) From b9530a4b23cc0f612c47daae06b7e2c3e04a9fbb Mon Sep 17 00:00:00 2001 From: Chris Sauer Date: Sun, 31 Dec 2023 14:07:41 -0800 Subject: [PATCH 03/11] EMCC: Minor: Avoid seemingly unnecessary splice of compile_args @kon72, if I've missed something here, please just revert --- refresh.template.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/refresh.template.py b/refresh.template.py index 38a6d1e..d74d373 100644 --- a/refresh.template.py +++ b/refresh.template.py @@ -786,7 +786,7 @@ def get_workspace_root(path_from_execroot: pathlib.PurePath): # We run the emcc process with the environment variable EM_COMPILER_WRAPPER to intercept the command line arguments passed to `clang`. emcc_process = subprocess.run( - [emcc_driver] + compile_args[1:], + compile_args, # MIN_PY=3.7: Replace PIPEs with capture_output. stdout=subprocess.PIPE, stderr=subprocess.PIPE, From 9ba64f90ba848feec8a75b5b915982e868a2cda0 Mon Sep 17 00:00:00 2001 From: Chris Sauer Date: Sun, 31 Dec 2023 14:13:13 -0800 Subject: [PATCH 04/11] EMCC: Minor: Let's call emscripten and apple patches first I was thinking that in the future _all_platform_patch might modify things revealed by them --- refresh.template.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/refresh.template.py b/refresh.template.py index d74d373..d25aae4 100644 --- a/refresh.template.py +++ b/refresh.template.py @@ -1062,11 +1062,11 @@ def _get_cpp_command_for_files(compile_action): Undo Bazel-isms and figures out which files clangd should apply the command to. """ - # Patch command by platform - compile_action.arguments = _all_platform_patch(compile_action.arguments) + # Patch command by platform, revealing any hidden arguments. compile_action.arguments = _apple_platform_patch(compile_action.arguments) compile_action.arguments = _emscripten_platform_patch(compile_action.arguments) # Android and Linux and grailbio LLVM toolchains: Fine as is; no special patching needed. + compile_action.arguments = _all_platform_patch(compile_action.arguments) source_files, header_files = _get_files(compile_action) From dc53e4ae590a07291291102709c25de669570092 Mon Sep 17 00:00:00 2001 From: kon72 Date: Mon, 1 Jan 2024 17:08:11 +0900 Subject: [PATCH 05/11] Revert "EMCC: Minor: Avoid seemingly unnecessary splice of compile_args" This reverts commit b9530a4b23cc0f612c47daae06b7e2c3e04a9fbb. --- refresh.template.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/refresh.template.py b/refresh.template.py index d25aae4..594e0cf 100644 --- a/refresh.template.py +++ b/refresh.template.py @@ -786,7 +786,7 @@ def get_workspace_root(path_from_execroot: pathlib.PurePath): # We run the emcc process with the environment variable EM_COMPILER_WRAPPER to intercept the command line arguments passed to `clang`. emcc_process = subprocess.run( - compile_args, + [emcc_driver] + compile_args[1:], # MIN_PY=3.7: Replace PIPEs with capture_output. stdout=subprocess.PIPE, stderr=subprocess.PIPE, From ab0dc643f0471e5309e6cef89a0a67ccc854b07c Mon Sep 17 00:00:00 2001 From: kon72 Date: Mon, 1 Jan 2024 17:20:26 +0900 Subject: [PATCH 06/11] Add comment regarding the revert --- refresh.template.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/refresh.template.py b/refresh.template.py index 594e0cf..4da0058 100644 --- a/refresh.template.py +++ b/refresh.template.py @@ -786,7 +786,9 @@ def get_workspace_root(path_from_execroot: pathlib.PurePath): # We run the emcc process with the environment variable EM_COMPILER_WRAPPER to intercept the command line arguments passed to `clang`. emcc_process = subprocess.run( - [emcc_driver] + compile_args[1:], + # On windows, it fails to spawn the subprocess when the path uses forward slashes as a separator. + # Here, we convert emcc driver path to use the native path separator. + [str(emcc_driver)] + compile_args[1:], # MIN_PY=3.7: Replace PIPEs with capture_output. stdout=subprocess.PIPE, stderr=subprocess.PIPE, From 31882f3623baa9c8f5d2d5b03c9234582f436cb6 Mon Sep 17 00:00:00 2001 From: kon72 Date: Mon, 1 Jan 2024 19:06:28 +0900 Subject: [PATCH 07/11] Use cc_binary --- BUILD | 9 +++------ print_args.bat | 3 --- print_args.cc | 13 +++++++++++++ print_args.py | 19 ------------------- print_args.sh | 3 --- refresh.template.py | 1 - refresh_compile_commands.bzl | 4 +--- 7 files changed, 17 insertions(+), 35 deletions(-) delete mode 100644 print_args.bat create mode 100644 print_args.cc delete mode 100644 print_args.py delete mode 100755 print_args.sh diff --git a/BUILD b/BUILD index 0bf7bd3..5318ee8 100644 --- a/BUILD +++ b/BUILD @@ -25,13 +25,10 @@ filegroup( # Implementation: # If you are looking into the implementation, start with the overview in ImplementationReadme.md. -exports_files(["refresh.template.py", "check_python_version.template.py", "print_args.py"]) # For implicit use by the refresh_compile_commands macro, not direct use. +exports_files(["refresh.template.py", "check_python_version.template.py"]) # For implicit use by the refresh_compile_commands macro, not direct use. -filegroup( +cc_binary( name = "print_args", - srcs = select({ - "@bazel_tools//src/conditions:host_windows": [":print_args.bat"], - "//conditions:default": [":print_args.sh"], - }), + srcs = ["print_args.cc"], visibility = ["//visibility:public"], ) diff --git a/print_args.bat b/print_args.bat deleted file mode 100644 index a61af60..0000000 --- a/print_args.bat +++ /dev/null @@ -1,3 +0,0 @@ -@ECHO OFF - -py -3 %HEDRON_COMPILE_COMMANDS_PRINT_ARGS_PY% %* diff --git a/print_args.cc b/print_args.cc new file mode 100644 index 0000000..b672329 --- /dev/null +++ b/print_args.cc @@ -0,0 +1,13 @@ +// Prints the arguments passed to the script + +#include + +int main(int argc, char *argv[]) { + std::cout << "===HEDRON_COMPILE_COMMANDS_BEGIN_ARGS===\n"; + for (int i = 1; i < argc; ++i) { + std::cout << argv[i] << "\n"; + } + std::cout << "===HEDRON_COMPILE_COMMANDS_END_ARGS===\n"; + // We purposely return a non-zero exit code to have the emcc process exit after running this fake clang wrapper. + return 1; +} diff --git a/print_args.py b/print_args.py deleted file mode 100644 index 694ce95..0000000 --- a/print_args.py +++ /dev/null @@ -1,19 +0,0 @@ -""" -Prints the arguments passed to the script -""" - -import sys - - -def main(): - print('===HEDRON_COMPILE_COMMANDS_BEGIN_ARGS===') - for arg in sys.argv[1:]: - print(arg) - print('===HEDRON_COMPILE_COMMANDS_END_ARGS===') - - # We purposely return a non-zero exit code to have the emcc process exit after running this fake clang wrapper. - sys.exit(1) - - -if __name__ == '__main__': - main() diff --git a/print_args.sh b/print_args.sh deleted file mode 100755 index a6a8ede..0000000 --- a/print_args.sh +++ /dev/null @@ -1,3 +0,0 @@ -#!/bin/bash - -exec python3 "${HEDRON_COMPILE_COMMANDS_PRINT_ARGS_PY}" "$@" diff --git a/refresh.template.py b/refresh.template.py index 4da0058..bd9079f 100644 --- a/refresh.template.py +++ b/refresh.template.py @@ -780,7 +780,6 @@ def get_workspace_root(path_from_execroot: pathlib.PurePath): 'EM_CONFIG_PATH': str(get_workspace_root(emcc_driver) / 'emscripten_toolchain' / 'emscripten_config'), 'EMCC_SKIP_SANITY_CHECK': '1', 'EM_COMPILER_WRAPPER': str(pathlib.PurePath({print_args_executable})), - 'HEDRON_COMPILE_COMMANDS_PRINT_ARGS_PY': str(pathlib.PurePath({print_args_py})), 'PATH': os.environ['PATH'], } diff --git a/refresh_compile_commands.bzl b/refresh_compile_commands.bzl index 1f1fc79..e779c94 100644 --- a/refresh_compile_commands.bzl +++ b/refresh_compile_commands.bzl @@ -117,7 +117,6 @@ def _expand_template_impl(ctx): "{exclude_headers}": repr(ctx.attr.exclude_headers), "{exclude_external_sources}": repr(ctx.attr.exclude_external_sources), "{print_args_executable}": repr(ctx.executable._print_args_executable.path), - "{print_args_py}": repr(ctx.file._print_args_py.path), }, ) return DefaultInfo(files = depset([script])) @@ -128,8 +127,7 @@ _expand_template = rule( "exclude_external_sources": attr.bool(default = False), "exclude_headers": attr.string(values = ["all", "external", ""]), # "" needed only for compatibility with Bazel < 3.6.0 "_script_template": attr.label(allow_single_file = True, default = "refresh.template.py"), - "_print_args_executable": attr.label(executable = True, allow_single_file = True, cfg = "target", default = "print_args"), - "_print_args_py": attr.label(allow_single_file = True, default = "print_args.py"), + "_print_args_executable": attr.label(executable = True, cfg = "target", default = "//:print_args"), # For Windows INCLUDE. If this were eliminated, for example by the resolution of https://github.com/clangd/clangd/issues/123, we'd be able to just use a macro and skylib's expand_template rule: https://github.com/bazelbuild/bazel-skylib/pull/330 # Once https://github.com/bazelbuild/bazel/pull/17108 is widely released, we should be able to eliminate this and get INCLUDE directly. Perhaps for 7.0? Should be released in the sucessor to 6.0 "_cc_toolchain": attr.label(default = "@bazel_tools//tools/cpp:current_cc_toolchain"), From e30284b14c6707c0f289c0b67677abced1fa9c69 Mon Sep 17 00:00:00 2001 From: kon72 Date: Mon, 1 Jan 2024 20:02:40 +0900 Subject: [PATCH 08/11] Fix get_workspace_root to work inside emsdk repo --- refresh.template.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/refresh.template.py b/refresh.template.py index bd9079f..006943c 100644 --- a/refresh.template.py +++ b/refresh.template.py @@ -771,7 +771,8 @@ def _emscripten_platform_patch(compile_args: typing.List[str]): assert sysroot, f'Emscripten sysroot not detected in CMD: {compile_args}' def get_workspace_root(path_from_execroot: pathlib.PurePath): - assert path_from_execroot.parts[0] == 'external' + if path_from_execroot.parts[0] != 'external': + return pathlib.PurePath('.') return pathlib.PurePath('external') / path_from_execroot.parts[1] environment = { From 24d81fcdc91bc2031d2017f2a4dce33da50bb3dd Mon Sep 17 00:00:00 2001 From: kon72 Date: Tue, 2 Jan 2024 17:41:39 +0900 Subject: [PATCH 09/11] Rename .cc to .cpp --- BUILD | 2 +- print_args.cc => print_args.cpp | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename print_args.cc => print_args.cpp (100%) diff --git a/BUILD b/BUILD index 5318ee8..f287781 100644 --- a/BUILD +++ b/BUILD @@ -29,6 +29,6 @@ exports_files(["refresh.template.py", "check_python_version.template.py"]) # Fo cc_binary( name = "print_args", - srcs = ["print_args.cc"], + srcs = ["print_args.cpp"], visibility = ["//visibility:public"], ) diff --git a/print_args.cc b/print_args.cpp similarity index 100% rename from print_args.cc rename to print_args.cpp From 927fbd18c3ec382f43c74ad66cfc978c9ec77939 Mon Sep 17 00:00:00 2001 From: kon72 Date: Tue, 2 Jan 2024 17:46:29 +0900 Subject: [PATCH 10/11] Use environment variables from aquery output --- refresh.template.py | 44 ++++++++++++++------------------------------ 1 file changed, 14 insertions(+), 30 deletions(-) diff --git a/refresh.template.py b/refresh.template.py index 006943c..5de69d5 100644 --- a/refresh.template.py +++ b/refresh.template.py @@ -745,19 +745,7 @@ def _apple_platform_patch(compile_args: typing.List[str]): return compile_args -def _get_sysroot(args: typing.List[str]): - for idx, arg in enumerate(args): - if arg == '--sysroot' or arg == '-isysroot': - if idx + 1 < len(args): - return pathlib.PurePath(args[idx + 1]) - elif arg.startswith('--sysroot='): - return pathlib.PurePath(arg[len('--sysroot='):]) - elif arg.startswith('-isysroot'): - return pathlib.PurePath(arg[len('-isysroot'):]) - return None - - -def _emscripten_platform_patch(compile_args: typing.List[str]): +def _emscripten_platform_patch(compile_args: typing.List[str], action_env: typing.Dict[str, str]): """De-Bazel the command into something clangd can parse. This function has fixes specific to Emscripten platforms, but you should call it on all platforms. It'll determine whether the fixes should be applied or not @@ -767,22 +755,13 @@ def _emscripten_platform_patch(compile_args: typing.List[str]): return compile_args workspace_absolute = pathlib.PurePath(os.environ["BUILD_WORKSPACE_DIRECTORY"]) - sysroot = _get_sysroot(compile_args) - assert sysroot, f'Emscripten sysroot not detected in CMD: {compile_args}' - - def get_workspace_root(path_from_execroot: pathlib.PurePath): - if path_from_execroot.parts[0] != 'external': - return pathlib.PurePath('.') - return pathlib.PurePath('external') / path_from_execroot.parts[1] - - environment = { - 'EXT_BUILD_ROOT': str(workspace_absolute), - 'EM_BIN_PATH': str(get_workspace_root(sysroot)), - 'EM_CONFIG_PATH': str(get_workspace_root(emcc_driver) / 'emscripten_toolchain' / 'emscripten_config'), - 'EMCC_SKIP_SANITY_CHECK': '1', - 'EM_COMPILER_WRAPPER': str(pathlib.PurePath({print_args_executable})), - 'PATH': os.environ['PATH'], - } + + environment = dict(action_env) + environment['EXT_BUILD_ROOT'] = str(workspace_absolute) + environment['EMCC_SKIP_SANITY_CHECK'] = '1' + environment['EM_COMPILER_WRAPPER'] = str(pathlib.PurePath({print_args_executable})) + if 'PATH' not in environment: + environment['PATH'] = os.environ['PATH'] # We run the emcc process with the environment variable EM_COMPILER_WRAPPER to intercept the command line arguments passed to `clang`. emcc_process = subprocess.run( @@ -1064,9 +1043,14 @@ def _get_cpp_command_for_files(compile_action): Undo Bazel-isms and figures out which files clangd should apply the command to. """ + env_pairs = getattr(compile_action, 'environmentVariables', []) + env = {} + for pair in env_pairs: + env[pair.key] = pair.value + # Patch command by platform, revealing any hidden arguments. compile_action.arguments = _apple_platform_patch(compile_action.arguments) - compile_action.arguments = _emscripten_platform_patch(compile_action.arguments) + compile_action.arguments = _emscripten_platform_patch(compile_action.arguments, action_env=env) # Android and Linux and grailbio LLVM toolchains: Fine as is; no special patching needed. compile_action.arguments = _all_platform_patch(compile_action.arguments) From 87d0cf6a37891adc2ffaba5e23c3515f183469b7 Mon Sep 17 00:00:00 2001 From: Chris Sauer Date: Tue, 2 Jan 2024 17:23:04 -0800 Subject: [PATCH 11/11] Emscripten: Minor twiddles Mostly adding environment dict to the compile_action for easy reuse in the future --- refresh.template.py | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/refresh.template.py b/refresh.template.py index 5de69d5..b6925df 100644 --- a/refresh.template.py +++ b/refresh.template.py @@ -745,18 +745,18 @@ def _apple_platform_patch(compile_args: typing.List[str]): return compile_args -def _emscripten_platform_patch(compile_args: typing.List[str], action_env: typing.Dict[str, str]): +def _emscripten_platform_patch(compile_action): """De-Bazel the command into something clangd can parse. This function has fixes specific to Emscripten platforms, but you should call it on all platforms. It'll determine whether the fixes should be applied or not """ - emcc_driver = pathlib.Path(compile_args[0]) + emcc_driver = pathlib.Path(compile_action.arguments[0]) if not emcc_driver.name.startswith('emcc'): - return compile_args + return compile_action.arguments workspace_absolute = pathlib.PurePath(os.environ["BUILD_WORKSPACE_DIRECTORY"]) - environment = dict(action_env) + environment = compile_action.environmentVariables.copy() environment['EXT_BUILD_ROOT'] = str(workspace_absolute) environment['EMCC_SKIP_SANITY_CHECK'] = '1' environment['EM_COMPILER_WRAPPER'] = str(pathlib.PurePath({print_args_executable})) @@ -765,9 +765,9 @@ def _emscripten_platform_patch(compile_args: typing.List[str], action_env: typin # We run the emcc process with the environment variable EM_COMPILER_WRAPPER to intercept the command line arguments passed to `clang`. emcc_process = subprocess.run( - # On windows, it fails to spawn the subprocess when the path uses forward slashes as a separator. + # On Windows, it fails to spawn the subprocess when the path uses forward slashes as a separator. # Here, we convert emcc driver path to use the native path separator. - [str(emcc_driver)] + compile_args[1:], + [str(emcc_driver)] + compile_action.arguments[1:], # MIN_PY=3.7: Replace PIPEs with capture_output. stdout=subprocess.PIPE, stderr=subprocess.PIPE, @@ -1043,14 +1043,12 @@ def _get_cpp_command_for_files(compile_action): Undo Bazel-isms and figures out which files clangd should apply the command to. """ - env_pairs = getattr(compile_action, 'environmentVariables', []) - env = {} - for pair in env_pairs: - env[pair.key] = pair.value + # Condense aquery's environment variables into a dictionary, the format you might expect. + compile_action.environmentVariables = {pair.key: pair.value for pair in getattr(compile_action, 'environmentVariables', [])} # Patch command by platform, revealing any hidden arguments. compile_action.arguments = _apple_platform_patch(compile_action.arguments) - compile_action.arguments = _emscripten_platform_patch(compile_action.arguments, action_env=env) + compile_action.arguments = _emscripten_platform_patch(compile_action) # Android and Linux and grailbio LLVM toolchains: Fine as is; no special patching needed. compile_action.arguments = _all_platform_patch(compile_action.arguments)