fmeum / with_cfg.bzl

Apache License 2.0
40 stars 2 forks source link

Make variable expansion does not work #105

Closed RuneSkovrupHansen closed 2 months ago

RuneSkovrupHansen commented 2 months ago

Make variable expansion does not work for transitioned rules.

I have a transition for the native py_binary rule which does nothing, it simply wraps the native rule.

I define two identical targets using the native and the transitioned rule. The python script tries to read and print the contents of a file whose path is specify through an environment variable using the make variable $(rootpath ...).

The native py_binary rule correctly expands the variable and is able to read and print the file contents, however the transitioned rule fails to expand the variable and in turn fails at trying to open the file.

defs.bzl:

load("@with_cfg.bzl", "with_cfg")

transitioned_py_binary, _transitioned_py_binary_internal = with_cfg(native.py_binary).build()

BUILD:

load("defs.bzl", "transitioned_py_binary")

py_binary(
    name = "print_file",
    srcs = ["print_file.py"],
    data = ["file.txt"],
    env = {"FILE": "$(rootpath :file.txt)"},
    main = "print_file.py",
    visibility = ["//visibility:public"],
)

transitioned_py_binary(
    name = "print_file_transitioned",
    srcs = ["print_file.py"],
    data = ["file.txt"],
    env = {"FILE": "$(rootpath :file.txt)"},
    main = "print_file.py",
    visibility = ["//visibility:public"],
)

print_file.py:

import os

def main():
    file_path = os.environ["FILE"]
    with open(file_path) as f:
        print(f.read())

if __name__ == "__main__":
    main()

Here's the error message from running target print_file_transitioned which shows that the variable has not been expanded:

FileNotFoundError: [Errno 2] No such file or directory: '$(rootpath :file.txt)'
fmeum commented 2 months ago

Are you on Bazel 6? I can confirm that this happens on Bazel 6.5.0, but not with Bazel 7. It happens because in Bazel 6 py_binary is still a native rule and its env attribute relies on some native Bazel magic.

I recently landed a commit to support this with args (https://github.com/fmeum/with_cfg.bzl/commit/d7193dee6667b4ef81325e1da9f84f02af7a5054), but due to the high complexity of the solution, I would prefer not to extend it to env just for Bazel 6 if possible. Is it possible for you to update to Bazel 7?

RuneSkovrupHansen commented 2 months ago

Yes, I am currently on Bazel 6.4.0, sorry for not including that in the description.

Thank you for the explanation, that makes sense. I was already planning to upgrade to Bazel 7, so I will simply do that first.

Thanks!