Skip to content

Commit 0b821b7

Browse files
committed
Switch back off rules_python.
Mostly reverts 0e5b1aa Tracking restoration at #168 Please see - #163 - bazel-contrib/rules_python#1732 - #165 - (rules_python issue to come) - #166 - bazel-contrib/rules_python#1169
1 parent 40a51d6 commit 0b821b7

11 files changed

+143
-103
lines changed

.pre-commit-config.yaml

+2
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,9 @@ repos:
77
- id: trailing-whitespace
88
- id: end-of-file-fixer
99
- id: check-ast
10+
exclude: ^check_python_version\.template\.py$ # Template for bazel creates syntax error
1011
- id: debug-statements
12+
exclude: ^check_python_version\.template\.py$ # Template for bazel creates syntax error
1113
- id: mixed-line-ending
1214
- id: check-case-conflict
1315
- id: fix-byte-order-marker

BUILD

+1-11
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,6 @@ filegroup(
1616
visibility = ["//visibility:public"],
1717
srcs = glob(["**/*.bzl"]) + [
1818
"@bazel_tools//tools:bzl_srcs",
19-
"@hedron_compile_commands_pip//:requirements.bzl",
20-
"@python_3_11//:defs.bzl",
21-
"@rules_python//:bzl",
2219
],
2320
)
2421

@@ -28,17 +25,10 @@ filegroup(
2825
# Implementation:
2926
# If you are looking into the implementation, start with the overview in ImplementationReadme.md.
3027

31-
exports_files(["refresh.template.py"]) # For implicit use by the refresh_compile_commands macro, not direct use.
28+
exports_files(["refresh.template.py", "check_python_version.template.py"]) # For implicit use by the refresh_compile_commands macro, not direct use.
3229

3330
cc_binary(
3431
name = "print_args",
3532
srcs = ["print_args.cpp"],
3633
visibility = ["//visibility:public"],
3734
)
38-
39-
# Quick test for https://github.com/bazelbuild/rules_python/issues/1732#issuecomment-1918268343. Delete when resolved.
40-
load("@python_3_11//:defs.bzl", "py_binary")
41-
py_binary(
42-
name = "nvcc_clang_diff",
43-
srcs = ["nvcc_clang_diff.py"],
44-
)

MODULE.bazel

-16
Original file line numberDiff line numberDiff line change
@@ -4,19 +4,3 @@ p = use_extension("//:workspace_setup.bzl", "hedron_compile_commands_extension")
44
pt = use_extension("//:workspace_setup_transitive.bzl", "hedron_compile_commands_extension")
55
ptt = use_extension("//:workspace_setup_transitive_transitive.bzl", "hedron_compile_commands_extension")
66
pttt = use_extension("//:workspace_setup_transitive_transitive_transitive.bzl", "hedron_compile_commands_extension")
7-
8-
9-
bazel_dep(name = "rules_python", version = "0.29.0")
10-
python = use_extension("@rules_python//python/extensions:python.bzl", "python")
11-
python.toolchain(
12-
python_version = "3.11",
13-
)
14-
use_repo(python, "python_3_11")
15-
pip = use_extension("@rules_python//python/extensions:pip.bzl", "pip")
16-
pip.parse(
17-
hub_name = "hedron_compile_commands_pip",
18-
# Available versions are listed in @rules_python//python:versions.bzl.
19-
python_version = "3.11",
20-
requirements_lock = "//:requirements.txt",
21-
)
22-
use_repo(pip, "hedron_compile_commands_pip")

check_python_version.template.py

+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
"""
2+
Print a nice error message if the user is running too old of a version of python.
3+
Why not just put this block at the top of refresh.template.py?
4+
Python versions introduce constructs that don't parse in older versions, leading to an error before the version check is executed, since python parses files eagerly.
5+
For examples of this issue, see https://github.com/hedronvision/bazel-compile-commands-extractor/issues/119 and https://github.com/hedronvision/bazel-compile-commands-extractor/issues/95
6+
This seems common enough that hopefully bazel will support it someday. We've filed a request: https://github.com/bazelbuild/bazel/issues/18389
7+
"""
8+
9+
import sys
10+
if sys.version_info < (3,6):
11+
sys.exit("\n\033[31mFATAL ERROR:\033[0m Python 3.6 or later is required. Please update!")
12+
13+
# Only import -> parse once we're sure we have the required python version
14+
import {to_run}
15+
{to_run}.main()

refresh.template.py

+68-36
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,20 @@
1010
- Crucially, this output is de-Bazeled; The result is a command that could be run from the workspace root directly, with no Bazel-specific requirements, environment variables, etc.
1111
"""
1212

13+
# This file requires python 3.6, which is enforced by check_python_version.template.py
14+
# 3.6 backwards compatibility required by @zhanyong-wan in https://github.com/hedronvision/bazel-compile-commands-extractor/issues/111.
15+
# 3.7 backwards compatibility required by @lummax in https://github.com/hedronvision/bazel-compile-commands-extractor/pull/27.
16+
# ^ Try to contact before upgrading.
17+
# When adding things could be cleaner if we had a higher minimum version, please add a comment with MIN_PY=3.<v>.
18+
# Similarly, when upgrading, please search for that MIN_PY= tag.
19+
1320

1421
import concurrent.futures
1522
import enum
16-
import functools
23+
import functools # MIN_PY=3.9: Replace `functools.lru_cache(maxsize=None)` with `functools.cache`.
1724
import itertools
1825
import json
1926
import locale
20-
import orjson # orjson is much faster than the standard library's json module (1.9 seconds vs 6.6 seconds for a ~140 MB file). See https://github.com/hedronvision/bazel-compile-commands-extractor/pull/118
2127
import os
2228
import pathlib
2329
import re
@@ -28,7 +34,7 @@
2834
import tempfile
2935
import time
3036
import types
31-
import typing
37+
import typing # MIN_PY=3.9: Switch e.g. typing.List[str] -> List[str]
3238

3339

3440
@enum.unique
@@ -89,7 +95,7 @@ def _print_header_finding_warning_once():
8995
_print_header_finding_warning_once.has_logged = False
9096

9197

92-
@functools.lru_cache
98+
@functools.lru_cache(maxsize=None)
9399
def _get_bazel_version():
94100
"""Gets the Bazel version as a tuple of (major, minor, patch).
95101
@@ -99,7 +105,9 @@ def _get_bazel_version():
99105
"""
100106
bazel_version_process = subprocess.run(
101107
['bazel', 'version'],
102-
capture_output=True,
108+
# MIN_PY=3.7: Replace PIPEs with capture_output.
109+
stdout=subprocess.PIPE,
110+
stderr=subprocess.PIPE,
103111
encoding=locale.getpreferredencoding(),
104112
check=True, # Should always succeed.
105113
)
@@ -118,12 +126,14 @@ def _get_bazel_version():
118126
return tuple(int(match.group(i)) for i in range(1, 4))
119127

120128

121-
@functools.lru_cache
129+
@functools.lru_cache(maxsize=None)
122130
def _get_bazel_cached_action_keys():
123131
"""Gets the set of actionKeys cached in bazel-out."""
124132
action_cache_process = subprocess.run(
125133
['bazel', 'dump', '--action_cache'],
126-
capture_output=True,
134+
# MIN_PY=3.7: Replace PIPEs with capture_output.
135+
stdout=subprocess.PIPE,
136+
stderr=subprocess.PIPE,
127137
encoding=locale.getpreferredencoding(),
128138
check=True, # Should always succeed.
129139
)
@@ -177,7 +187,7 @@ def _parse_headers_from_makefile_deps(d_file_content: str, source_path_for_sanit
177187
return set(headers)
178188

179189

180-
@functools.lru_cache
190+
@functools.lru_cache(maxsize=None)
181191
def _get_cached_modified_time(path: str):
182192
"""Returns 0 if the file doesn't exist.
183193
@@ -222,7 +232,7 @@ def _is_nvcc(path: str):
222232
return os.path.basename(path).startswith('nvcc')
223233

224234

225-
def _get_headers_gcc(compile_args: list[str], source_path: str, action_key: str):
235+
def _get_headers_gcc(compile_args: typing.List[str], source_path: str, action_key: str):
226236
"""Gets the headers used by a particular compile command that uses gcc arguments formatting (including clang.)
227237
228238
Relatively slow. Requires running the C preprocessor if we can't hit Bazel's cache.
@@ -277,7 +287,9 @@ def _get_headers_gcc(compile_args: list[str], source_path: str, action_key: str)
277287

278288
header_search_process = _subprocess_run_spilling_over_to_param_file_if_needed( # Note: gcc/clang can be run from Windows, too.
279289
header_cmd,
280-
capture_output=True,
290+
# MIN_PY=3.7: Replace PIPEs with capture_output.
291+
stdout=subprocess.PIPE,
292+
stderr=subprocess.PIPE,
281293
encoding=locale.getpreferredencoding(),
282294
check=False, # We explicitly ignore errors and carry on.
283295
)
@@ -368,7 +380,7 @@ def windows_list2cmdline(seq):
368380
return ''.join(result)
369381

370382

371-
def _subprocess_run_spilling_over_to_param_file_if_needed(command: list[str], **kwargs):
383+
def _subprocess_run_spilling_over_to_param_file_if_needed(command: typing.List[str], **kwargs):
372384
"""Same as subprocess.run, but it handles the case where the command line length is exceeded on Windows and we need a param file."""
373385

374386
# On non-Windows, we have to run directly via a special case.
@@ -396,7 +408,7 @@ def _subprocess_run_spilling_over_to_param_file_if_needed(command: list[str], **
396408
raise
397409

398410

399-
def _get_headers_msvc(compile_args: list[str], source_path: str):
411+
def _get_headers_msvc(compile_args: typing.List[str], source_path: str):
400412
"""Gets the headers used by a particular compile command that uses msvc argument formatting (including clang-cl.)
401413
402414
Relatively slow. Requires running the C preprocessor.
@@ -471,13 +483,23 @@ def _get_headers_msvc(compile_args: list[str], source_path: str):
471483
return headers, should_cache
472484

473485

486+
def _is_relative_to(sub: pathlib.PurePath, parent: pathlib.PurePath):
487+
"""Determine if one path is relative to another."""
488+
# MIN_PY=3.9: Eliminate helper in favor of `PurePath.is_relative_to()`.
489+
try:
490+
sub.relative_to(parent)
491+
except ValueError:
492+
return False
493+
return True
494+
495+
474496
def _file_is_in_main_workspace_and_not_external(file_str: str):
475497
file_path = pathlib.PurePath(file_str)
476498
if file_path.is_absolute():
477499
workspace_absolute = pathlib.PurePath(os.environ["BUILD_WORKSPACE_DIRECTORY"])
478-
if not file_path.is_relative_to(workspace_absolute):
500+
if not _is_relative_to(file_path, workspace_absolute):
479501
return False
480-
file_path = file_path.relative_to(workspace_absolute)
502+
file_path = _is_relative_to(file_path, workspace_absolute)
481503
# You can now assume that the path is relative to the workspace.
482504
# [Already assuming that relative paths are relative to the main workspace.]
483505

@@ -540,7 +562,7 @@ def _get_headers(compile_action, source_path: str):
540562
cache_last_modified = os.path.getmtime(cache_file_path) # Do before opening just as a basic hedge against concurrent write, even though we won't handle the concurrent delete case perfectly.
541563
try:
542564
with open(cache_file_path) as cache_file:
543-
action_key, cached_headers = orjson.loads(cache_file.read())
565+
action_key, cached_headers = json.load(cache_file)
544566
except json.JSONDecodeError:
545567
# Corrupted cache, which can happen if, for example, the user kills the program, since writes aren't atomic.
546568
# But if it is the result of a bug, we want to print it before it's overwritten, so it can be reported
@@ -569,11 +591,13 @@ def _get_headers(compile_action, source_path: str):
569591
# Cache for future use
570592
if output_file and should_cache:
571593
os.makedirs(os.path.dirname(cache_file_path), exist_ok=True)
572-
with open(cache_file_path, 'wb') as cache_file:
573-
cache_file.write(orjson.dumps(
594+
with open(cache_file_path, 'w') as cache_file:
595+
json.dump(
574596
(compile_action.actionKey, list(headers)),
575-
option=orjson.OPT_INDENT_2,
576-
))
597+
cache_file,
598+
indent=2,
599+
check_circular=False,
600+
)
577601
elif not headers and cached_headers: # If we failed to get headers, we'll fall back on a stale cache.
578602
headers = set(cached_headers)
579603

@@ -689,7 +713,7 @@ def _get_files(compile_action):
689713
_get_files.extensions_to_language_args = {ext : flag for exts, flag in _get_files.extensions_to_language_args.items() for ext in exts} # Flatten map for easier use
690714

691715

692-
@functools.lru_cache
716+
@functools.lru_cache(maxsize=None)
693717
def _get_apple_SDKROOT(SDK_name: str):
694718
"""Get path to xcode-select'd root for the given OS."""
695719
SDKROOT_maybe_versioned = subprocess.check_output(
@@ -707,7 +731,7 @@ def _get_apple_SDKROOT(SDK_name: str):
707731
# Traditionally stored in SDKROOT environment variable, but not provided by Bazel. See https://github.com/bazelbuild/bazel/issues/12852
708732

709733

710-
def _get_apple_platform(compile_args: list[str]):
734+
def _get_apple_platform(compile_args: typing.List[str]):
711735
"""Figure out which Apple platform a command is for.
712736
713737
Is the name used by Xcode in the SDK files, not the marketing name.
@@ -721,15 +745,15 @@ def _get_apple_platform(compile_args: list[str]):
721745
return None
722746

723747

724-
@functools.lru_cache
748+
@functools.lru_cache(maxsize=None)
725749
def _get_apple_DEVELOPER_DIR():
726750
"""Get path to xcode-select'd developer directory."""
727751
return subprocess.check_output(('xcode-select', '--print-path'), encoding=locale.getpreferredencoding()).rstrip()
728752
# Unless xcode-select has been invoked (like for a beta) we'd expect, e.g., '/Applications/Xcode.app/Contents/Developer' or '/Library/Developer/CommandLineTools'.
729753
# Traditionally stored in DEVELOPER_DIR environment variable, but not provided by Bazel. See https://github.com/bazelbuild/bazel/issues/12852
730754

731755

732-
def _apple_platform_patch(compile_args: list[str]):
756+
def _apple_platform_patch(compile_args: typing.List[str]):
733757
"""De-Bazel the command into something clangd can parse.
734758
735759
This function has fixes specific to Apple platforms, but you should call it on all platforms. It'll determine whether the fixes should be applied or not.
@@ -803,7 +827,9 @@ def get_workspace_root(path_from_execroot: pathlib.PurePath):
803827
# On Windows, it fails to spawn the subprocess when the path uses forward slashes as a separator.
804828
# Here, we convert emcc driver path to use the native path separator.
805829
[str(emcc_driver)] + compile_action.arguments[1:],
806-
capture_output=True,
830+
# MIN_PY=3.7: Replace PIPEs with capture_output.
831+
stdout=subprocess.PIPE,
832+
stderr=subprocess.PIPE,
807833
env=environment,
808834
encoding=locale.getpreferredencoding(),
809835
check=False, # We explicitly ignore errors and carry on.
@@ -817,7 +843,7 @@ def get_workspace_root(path_from_execroot: pathlib.PurePath):
817843
end_args_idx = lines.index(END_ARGS_MARKER, begin_args_idx + 1)
818844
args = lines[begin_args_idx + 1:end_args_idx]
819845
clang_driver = pathlib.PurePath(args[0])
820-
if clang_driver.is_relative_to(workspace_absolute):
846+
if _is_relative_to(clang_driver, workspace_absolute):
821847
args[0] = clang_driver.relative_to(workspace_absolute).as_posix()
822848
return args
823849

@@ -826,7 +852,7 @@ def get_workspace_root(path_from_execroot: pathlib.PurePath):
826852
END_ARGS_MARKER = '===HEDRON_COMPILE_COMMANDS_END_ARGS==='
827853

828854

829-
def _all_platform_patch(compile_args: list[str]):
855+
def _all_platform_patch(compile_args: typing.List[str]):
830856
"""Apply de-Bazeling fixes to the compile command that are shared across target platforms."""
831857
# clangd writes module cache files to the wrong place
832858
# Without this fix, you get tons of module caches dumped into the VSCode root folder.
@@ -865,7 +891,7 @@ def _all_platform_patch(compile_args: list[str]):
865891

866892
# Discover compilers that are actually symlinks to ccache--and replace them with the underlying compiler
867893
if os.path.islink(compile_args[0]):
868-
compiler_path = os.readlink(compile_args[0])
894+
compiler_path = os.readlink(compile_args[0]) # MIN_PY=3.9 Switch to pathlib path.readlink()
869895
if os.path.basename(compiler_path) == "ccache":
870896
compiler = os.path.basename(compile_args[0])
871897
real_compiler_path = shutil.which(compiler)
@@ -877,7 +903,7 @@ def _all_platform_patch(compile_args: list[str]):
877903
return compile_args
878904

879905

880-
def _nvcc_patch(compile_args: list[str]) -> list[str]:
906+
def _nvcc_patch(compile_args: typing.List[str]) -> typing.List[str]:
881907
"""Apply fixes to args to nvcc.
882908
883909
Basically remove everything that's an nvcc arg that is not also a clang arg, converting what we can.
@@ -1120,7 +1146,9 @@ def _convert_compile_commands(aquery_output):
11201146

11211147
# Process each action from Bazelisms -> file paths and their clang commands
11221148
# Threads instead of processes because most of the execution time is farmed out to subprocesses. No need to sidestep the GIL. Might change after https://github.com/clangd/clangd/issues/123 resolved
1123-
with concurrent.futures.ThreadPoolExecutor() as threadpool:
1149+
with concurrent.futures.ThreadPoolExecutor(
1150+
max_workers=min(32, (os.cpu_count() or 1) + 4) # Backport. Default in MIN_PY=3.8. See "using very large resources implicitly on many-core machines" in https://docs.python.org/3/library/concurrent.futures.html#concurrent.futures.ThreadPoolExecutor
1151+
) as threadpool:
11241152
outputs = threadpool.map(_get_cpp_command_for_files, aquery_output.actions)
11251153

11261154
# Yield as compile_commands.json entries
@@ -1210,7 +1238,9 @@ def _get_commands(target: str, flags: str):
12101238

12111239
aquery_process = subprocess.run(
12121240
aquery_args,
1213-
capture_output=True,
1241+
# MIN_PY=3.7: Replace PIPEs with capture_output.
1242+
stdout=subprocess.PIPE,
1243+
stderr=subprocess.PIPE,
12141244
encoding=locale.getpreferredencoding(),
12151245
check=False, # We explicitly ignore errors from `bazel aquery` and carry on.
12161246
)
@@ -1273,7 +1303,7 @@ def _ensure_external_workspaces_link_exists():
12731303
# This seemed to be the cleanest way to detect both.
12741304
# Note that os.path.islink doesn't detect junctions.
12751305
try:
1276-
current_dest = source.readlink()
1306+
current_dest = source.readlink() # MIN_PY=3.9 source.readlink()
12771307
except OSError:
12781308
log_error(f">>> //external already exists, but it isn't a {'junction' if is_windows else 'symlink'}. //external is reserved by Bazel and needed for this tool. Please rename or delete your existing //external and rerun. More details in the README if you want them.") # Don't auto delete in case the user has something important there.
12791309
sys.exit(1)
@@ -1368,7 +1398,7 @@ def _ensure_cwd_is_workspace_root():
13681398
os.chdir(workspace_root)
13691399

13701400

1371-
if __name__ == '__main__':
1401+
def main():
13721402
_ensure_cwd_is_workspace_root()
13731403
_ensure_gitignore_entries_exist()
13741404
_ensure_external_workspaces_link_exists()
@@ -1389,8 +1419,10 @@ def _ensure_cwd_is_workspace_root():
13891419
sys.exit(1)
13901420

13911421
# Chain output into compile_commands.json
1392-
with open('compile_commands.json', 'wb') as output_file:
1393-
output_file.write(orjson.dumps(
1422+
with open('compile_commands.json', 'w') as output_file:
1423+
json.dump(
13941424
compile_command_entries,
1395-
option=orjson.OPT_INDENT_2,
1396-
))
1425+
output_file,
1426+
indent=2, # Yay, human readability!
1427+
check_circular=False # For speed.
1428+
)

0 commit comments

Comments
 (0)