Skip to content

Commit 3dddf20

Browse files
committed
More robust, helpful error messaging around minimum python version.
Workaround for #119 Great opportunity for more general bazel support expressed in bazelbuild/bazel#18389
1 parent 80ac7ef commit 3dddf20

File tree

6 files changed

+67
-14
lines changed

6 files changed

+67
-14
lines changed

.pre-commit-config.yaml

Lines changed: 2 additions & 0 deletions
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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,4 @@ refresh_compile_commands(
1313
# Implementation:
1414
# If you are looking into the implementation, start with the overview in ImplementationReadme.md.
1515

16-
exports_files(["refresh.template.py"]) # For implicit use by refresh_compile_commands.
16+
exports_files(["refresh.template.py", "check_python_version.template.py"]) # For implicit use by therefresh_compile_commands macro, not direct use.

ImplementationReadme.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ The `refresh_compile_commands` rule (from [`refresh_compile_commands.bzl`](./ref
5252

5353
- [`refresh.template.py`](./refresh.template.py) is the main driver of actions. Browsing it should help you figure out where you want to go. It consists of two sections: one for calling `bazel aquery` and one for constructing the `compile_commands.json` from it. The latter does the actual reformatting of command so they're usable by `clangd`. This is more involved than you might think, but not crazy. It's easy to extend the reformatting operations applied.
5454
- If you're seeing failures on a new platform, weird entries in `compile_commands.json`, or wrapped compilers, this is where you should make changes to properly undo Bazel's wrapping of the command. See the "Patch command by platform" section.
55-
- The bazel files ([`refresh_compile_commands.bzl`](./refresh_compile_commands.bzl) and others) are just wrappings. They're less likely to require your attention.
55+
- The other files ([`refresh_compile_commands.bzl`](./refresh_compile_commands.bzl) and others) are just wrappings. They're less likely to require your attention, except as directed from the README or refresh.template.py.
5656

5757

5858
## Tool Choice — Why `clangd`?

check_python_version.template.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
"""
2+
Print a nice error message if the user is running too old of a version of python.
3+
4+
Why not just put this block at the top of refresh.template.py?
5+
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.
6+
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
7+
This seems common enough that hopefully bazel will support it someday. We've filed a request: https://github.com/bazelbuild/bazel/issues/18389
8+
"""
9+
10+
import sys
11+
if sys.version_info < (3,6):
12+
sys.exit("\n\033[31mFATAL ERROR:\033[0m Python 3.6 or later is required. Please update!")
13+
14+
# Only import -> parse once we're sure we have the required python version
15+
import {to_run}
16+
{to_run}.main()

refresh.template.py

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,13 @@
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-
import sys
14-
if sys.version_info < (3,6):
15-
sys.exit("\n\033[31mFATAL ERROR:\033[0m Python 3.6 or later is required. Please update!")
16-
# 3.6 backwards compatibility required by @zhanyong-wan in https://github.com/hedronvision/bazel-compile-commands-extractor/issues/111.
17-
# 3.7 backwards compatibility required by @lummax in https://github.com/hedronvision/bazel-compile-commands-extractor/pull/27.
18-
# ^ Try to contact before upgrading.
19-
# When adding things could be cleaner if we had a higher minimum version, please add a comment with MIN_PY=3.<v>.
20-
# Similarly, when upgrading, please search for that MIN_PY= tag.
13+
14+
# This file requires python 3.6, which is enforced by check_python_version.template.py
15+
# 3.6 backwards compatibility required by @zhanyong-wan in https://github.com/hedronvision/bazel-compile-commands-extractor/issues/111.
16+
# 3.7 backwards compatibility required by @lummax in https://github.com/hedronvision/bazel-compile-commands-extractor/pull/27.
17+
# ^ Try to contact before upgrading.
18+
# When adding things could be cleaner if we had a higher minimum version, please add a comment with MIN_PY=3.<v>.
19+
# Similarly, when upgrading, please search for that MIN_PY= tag.
2120

2221

2322
import concurrent.futures
@@ -32,6 +31,7 @@
3231
import shlex
3332
import shutil
3433
import subprocess
34+
import sys
3535
import tempfile
3636
import time
3737
import types
@@ -1049,7 +1049,7 @@ def _ensure_cwd_is_workspace_root():
10491049
os.chdir(workspace_root)
10501050

10511051

1052-
if __name__ == '__main__':
1052+
def main():
10531053
_ensure_cwd_is_workspace_root()
10541054
_ensure_gitignore_entries_exist()
10551055
_ensure_external_workspaces_link_exists()

refresh_compile_commands.bzl

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,13 +77,27 @@ def refresh_compile_commands(
7777
elif type(targets) != "dict": # Assume they've supplied a single string/label and wrap it
7878
targets = {targets: ""}
7979

80-
# Generate runnable python script from template
80+
# Create a wrapper script that prints a helpful error message if the python version is too old, generated from check_python_version.template.py
81+
version_checker_script_name = name + ".check_python_version.py"
82+
_check_python_version(name = version_checker_script_name, to_run = name)
83+
84+
# Generate the core, runnable python script from refresh.template.py
8185
script_name = name + ".py"
8286
_expand_template(name = script_name, labels_to_flags = targets, exclude_headers = exclude_headers, exclude_external_sources = exclude_external_sources, **kwargs)
83-
native.py_binary(name = name, srcs = [script_name], **kwargs)
87+
88+
# Combine them so the wrapper calls the main script
89+
native.py_binary(
90+
name = name,
91+
main = version_checker_script_name,
92+
srcs = [
93+
version_checker_script_name,
94+
script_name,
95+
],
96+
**kwargs
97+
)
8498

8599
def _expand_template_impl(ctx):
86-
"""Inject targets of interest into refresh.template.py, and set it up to be run."""
100+
"""Inject targets of interest--and other settings--into refresh.template.py, and set it up to be run."""
87101
script = ctx.actions.declare_file(ctx.attr.name)
88102
ctx.actions.expand_template(
89103
output = script,
@@ -112,3 +126,24 @@ _expand_template = rule(
112126
toolchains = ["@bazel_tools//tools/cpp:toolchain_type"], # Needed for find_cpp_toolchain with --incompatible_enable_cc_toolchain_resolution
113127
implementation = _expand_template_impl,
114128
)
129+
130+
def _check_python_version_impl(ctx):
131+
"""Sets up check_python_version.template.py to call {to_run}.py's main()"""
132+
script = ctx.actions.declare_file(ctx.attr.name)
133+
ctx.actions.expand_template(
134+
output = script,
135+
is_executable = True,
136+
template = ctx.file._template,
137+
substitutions = {
138+
"{to_run}": ctx.attr.to_run,
139+
},
140+
)
141+
return DefaultInfo(files = depset([script]))
142+
143+
_check_python_version = rule(
144+
attrs = {
145+
"to_run": attr.string(mandatory = True), # Name of the python module (no .py) to import and call .main() on, should checks succeed.
146+
"_template": attr.label(allow_single_file = True, default = "check_python_version.template.py"),
147+
},
148+
implementation = _check_python_version_impl,
149+
)

0 commit comments

Comments
 (0)