Add rules_python toolchain to the rule by furtib · Pull Request #174 · Ericsson/rules_codechecker

furtib · 2026-01-29T15:40:32Z

Why:
Necessary for Bazel 8 support.

What:

Added rules_python as a dependency
Removed registration of our own toolchain from the MODULE.bazel; if left there, its priority would shadow the rules_python toolchain. Bazel 7, during unittest, seems to prefer our own, while if included by another project, it seems to prefer the rules_python toolchain. (Adding the following flag will reveal the process of toolchain selection: --toolchain_resolution_debug=.*)
(The logic behind this:
- Bazel 6 prefers the WORKSPACE, so it finds the old toolchain.
- Bazel 7 uses the MODULE system, but also checks the WORKSPACE file, so it finds the old toolchain too. In projects including it with only a MODULE file, e.g., zlib-modules, it will use the rules_python toolchain. (My theory for this is that register_toolchains takes the highest precedence in the order of toolchain resolution, and in other projects, we don't register the toolchain)
- Bazel 8 ignores the WORKSPACE file, so it does not find the old toolchain; they can't interfere with each other.
  )
Changed from using shebang to changing the executor. This is necessary, since the interpreter provided by rules_python is inside the sandbox, and we will get a relative path, not an absolute one. This is why we can't use shebang. Also, if use_default_shell_env is enabled, the path to an executable is accepted as executable.
Created that absolutely ugly wrapper script for codechecker_test - I have no better idea of how I return an executable that, without any parameters, would execute the script using the interpreter, that maybe inside the sandbox, maybe not. I'm open to any better ideas. (The only idea I have is dropping the template expansion entirely, and using py_binary targets instead)
It is possible to set a specific Python version for the rules_python toolchain. I opted not to do this and let the including project decide which version they want to use.

Addresses:
#108

Szelethus

Bazel 7, during unittest, seems to prefer our own, while if included by another project, it seems to prefer the rules_python toolchain.
What does "prefer" mean? Does this mean that we are committing to supporting both WORKSPACE and Module system for bazel 7?

I want to know whether this is an actual requirement, because the way you explain it, this seems to be a non-trivial complexity. Would the code look significantly cleaner if we dropped WORKSPACE support for bazel 7?

Szelethus · 2026-02-04T12:04:10Z

src/common.bzl

    elif hasattr(py_toolchain, "py3_runtime"):
        py_runtime = py_toolchain.py3_runtime
        python_path = py_runtime.interpreter_path
+        if python_path == None: # Bazel 8


Thats a vague comment on its own.

Szelethus · 2026-02-04T12:06:55Z

src/common.bzl


+def python_interpreter_tool(ctx):
+    """
+    Returns version specific Python interpreter object in a list


Do we have to return a list?

Suggested change

Returns version specific Python interpreter object in a list

Returns version specific Python interpreter object that is appropriate for the Bazel version that is running

We don't have to return a list, but handling the case where there is no tool to return (Bazel 6) will become very ugly.
Now we have (removing unimportant parts):

ctx.actions.run( executable = python_path(ctx), tools = python_interpreter_tool(ctx), arguments = [ ... ], use_default_shell_env = True, )

This would look like:

ctx.actions.run( executable = python_path(ctx), tools = [python_interpreter_tool(ctx)] if python_interpreter_tool(ctx) != None else [], arguments = [ ... ], use_default_shell_env = True, )

or we calculate tools ahead of time:

tools = [python_interpreter_tool(ctx)] if python_interpreter_tool(ctx) != None else [], ctx.actions.run(.....)

Szelethus · 2026-02-04T12:08:14Z

MODULE.bazel

+#python = use_extension("@rules_python//python/extensions:python.bzl", "python")
+#python.toolchain(
+#    python_version = "3.10",
+#    is_default = True,
+#)


What is the story with these?

I tried everything to leave the toolchain registration for the old toolchain in the MODULE file, and have rules_python have a higher priority, but I failed.
These options I have come across during that. (I left them in thinking they might be interesting)

furtib · 2026-02-04T12:59:57Z

My explanation might not been the best, mostly it was just noting my experience trying this change out in different environments.
We cannot "drop" support for WORKSPACE files in Bazel 7. Bazel 7 will check the contents of the WORKSPACE file.

The strange behaviour, while I'm not completely sure I understand (which we should before merging), is most likely due to the way the rule is included in a project.

In the codechecker_bazel repository, both MODULE and WORKSPACE files are present, and thus the toolchain that is only registered in the WORKSPACE file will be found and used.
Other projects that include the rule through the MODULE system will not find it, since WORKSPACE files are non-transitive, but they will find the rules_python included in the MODULE file, since that is transitive.

This theory holds up in FOSS tests:

zlib uses our toolchain (registered in its own WORKSPACE file)
zlib-module uses the rules_python toolchain.

This would also mean that Bazel 8 will fail on tests using WORKSPACE files (Bazel 8 can be forced to read WORKSPACE files)

src/codechecker.bzl

src/common.bzl

Szelethus

I personally approve the patch as-is. @nettle, can you give you two cents?

src/codechecker.bzl

Szelethus · 2026-02-24T10:43:35Z

src/codechecker.bzl

+    ctx.actions.write(
+        output = ctx.outputs.test_script_wrapper,
+        is_executable = True,
+        content = """
+            {} {}
+        """.format(
+            python_interpreter_path,
+            ctx.outputs.codechecker_test_script.short_path,
+        ),
+    )
+
    # Return test script and all required files
-    run_files = default_runfiles + [ctx.outputs.codechecker_test_script]
+    run_files = default_runfiles + [
+        ctx.outputs.codechecker_test_script,
+        ctx.outputs.test_script_wrapper,
+    ] + python_interpreter_tool(ctx)


@nettle I understand you find this solution inelegant, and I won't contest it. However, would it be agreeable for you if we pushed this PR as-is, and found a nicer solution after the fact? This code is not the architectural backbone of the patch, so I think it is fine to improve it later.

What is the problem with current implementation?
Well, it is also not nice :) but I dont think we should change "not nice" with another "not nice" :)

We cannot use the Python binary provided by rules_python with the old implementation (shebangs).
This is because a shebang must be an absolute path.
I'm not aware of any way to get the absolute path for the rules_python provided binary (remote executors further complicate this).
Also, doing this is not completely unheard of:

actions.run arguments are not passed when executing a test rule bazelbuild/bazel#19661 (comment)

https://github.com/keith/rules_multirun/blob/e2245c6a4e1fb26cea7db6fac16d856693cf1ec1/multirun.bzl#L124

I guess a nicer solution would be if the script were a py_binary target in the BUILD file.

We could also just hardcode #!/usr/bin/env python3.

Okay, I have tried out the py_binary solution in #199.
If we agree with that, then we should merge that one and return here afterwards.

nettle · 2026-02-24T16:01:36Z

src/codechecker.bzl

+    # If we use our custom toolchain, the path will be an absolute path,
+    # executable by bazel
+    python_interpreter_path = python_path(ctx)
+
+    # If we can find an interpreter tool provided by bazel,
+    # use that as an executable
+    if python_interpreter_tool(ctx) != []:
+        python_interpreter_path = python_interpreter_tool(ctx)[0].short_path
+
+    # Since we cannot give parameters to the runfiles
+    # we wrap the executable (a python binary) in a script.
+    # In the script we give the CodeChecker script as an argument.
+    ctx.actions.write(
+        output = ctx.outputs.test_script_wrapper,
+        is_executable = True,
+        content = """
+            {} {}
+        """.format(
+            python_interpreter_path,
+            ctx.outputs.codechecker_test_script.short_path,
+        ),
+    )
+


This part is exactly why I dont like this change.
Do we have any good reasons for this?

nettle · 2026-02-24T16:05:18Z

src/codechecker.bzl

+    ctx.actions.write(
+        output = ctx.outputs.test_script_wrapper,
+        is_executable = True,
+        content = """
+            {} {}
+        """.format(
+            python_interpreter_path,
+            ctx.outputs.codechecker_test_script.short_path,
+        ),
+    )
+
    # Return test script and all required files
-    run_files = default_runfiles + [ctx.outputs.codechecker_test_script]
+    run_files = default_runfiles + [
+        ctx.outputs.codechecker_test_script,
+        ctx.outputs.test_script_wrapper,
+    ] + python_interpreter_tool(ctx)


What is the problem with current implementation?
Well, it is also not nice :) but I dont think we should change "not nice" with another "not nice" :)

Co-authored-by: Kristóf Umann <dkszelethus@gmail.com>

furtib self-assigned this Jan 29, 2026

furtib added the enhancement New feature or request label Jan 29, 2026

furtib force-pushed the add-rules-python branch 6 times, most recently from b64ea51 to b602fbe Compare January 30, 2026 09:55

furtib marked this pull request as ready for review January 30, 2026 09:59

furtib requested review from Szelethus and nettle and removed request for nettle January 30, 2026 10:01

Szelethus reviewed Feb 4, 2026

View reviewed changes

furtib force-pushed the add-rules-python branch 5 times, most recently from e947e0b to e11868f Compare February 4, 2026 14:35

furtib force-pushed the add-rules-python branch from a40e8a0 to 276963e Compare February 23, 2026 10:07

Szelethus reviewed Feb 23, 2026

View reviewed changes

src/codechecker.bzl Show resolved Hide resolved

src/codechecker.bzl Outdated Show resolved Hide resolved

src/codechecker.bzl Outdated Show resolved Hide resolved

src/common.bzl Outdated Show resolved Hide resolved

furtib force-pushed the add-rules-python branch from 366aef9 to 410c09f Compare February 24, 2026 09:14

furtib requested a review from Szelethus February 24, 2026 09:22

Szelethus approved these changes Feb 24, 2026

View reviewed changes

nettle requested changes Feb 24, 2026

View reviewed changes

furtib force-pushed the add-rules-python branch from b9d43b6 to 34c5855 Compare February 25, 2026 12:56

furtib added 3 commits February 25, 2026 13:57

Add rules_python

cdf8aad

Run buildifier on new changes

a5db3eb

Finetune dependency versions for very old bazel versions

91ac99e

furtib and others added 4 commits February 25, 2026 13:57

Comment code

907d1e1

Make path selection more robustú

08c2e13

Update src/codechecker.bzl

591dce7

Co-authored-by: Kristóf Umann <dkszelethus@gmail.com>

make format easier to read

2f1f0b3

furtib force-pushed the add-rules-python branch from 34c5855 to 2f1f0b3 Compare February 25, 2026 12:57

	Returns version specific Python interpreter object in a list
	Returns version specific Python interpreter object that is appropriate for the Bazel version that is running

Conversation

furtib commented Jan 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Szelethus left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

furtib commented Feb 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Szelethus left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

furtib Feb 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

furtib commented Jan 29, 2026 •

edited

Loading

furtib commented Feb 4, 2026 •

edited

Loading

furtib Feb 25, 2026 •

edited

Loading