8000 fix(multi-versions): correctly default 'main' arg for transition rule… · bazel-contrib/rules_python@5c5ab5b · GitHub
[go: up one dir, main page]
Skip to content

Commit 5c5ab5b

Browse files
aignasaignas
authored
fix(multi-versions): correctly default 'main' arg for transition rules (#1316)
This fixes a bug where the version-aware rules required `main` to always be explicitly specified. This was necessary because the main file is named after the outer target (e.g. "foo"), but usage of the main file is done by the inner target ("_foo"). The net effect is the inner target looks for "_foo.py", while only "foo.py" is in srcs. To fix, the wrappers set main, if it isn't already set, to their name + ".py" Work towards #1262
1 parent 5c37fa7 commit 5c5ab5b

File tree

6 files changed

+141
-9
lines changed

6 files changed

+141
-9
lines changed

‎examples/multi_python_versions/tests/BUILD.bazel

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,22 @@
1+
load("@bazel_skylib//rules:copy_file.bzl", "copy_file")
12
load("@python//3.10:defs.bzl", py_binary_3_10 = "py_binary", py_test_3_10 = "py_test")
23
load("@python//3.11:defs.bzl", py_binary_3_11 = "py_binary", py_test_3_11 = "py_test")
34
load("@python//3.8:defs.bzl", py_binary_3_8 = "py_binary", py_test_3_8 = "py_test")
45
load("@python//3.9:defs.bzl", py_binary_3_9 = "py_binary", py_test_3_9 = "py_test")
56
load("@rules_python//python:defs.bzl", "py_binary", "py_test")
67

8+
copy_file(
9+
name = "copy_version",
10+
src = "version.py",
11+
out = "version_default.py",
12+
is_executable = True,
13+
)
14+
15+
# NOTE: We are testing that the `main` is an optional param as per official
16+
# docs https://bazel.build/reference/be/python#py_binary.main
717
py_binary(
818
name = "version_default",
9-
srcs = ["version.py"],
10-
main = "version.py",
19+
srcs = ["version_default.py"],
1120
)
1221

1322
py_binary_3_8(
@@ -69,11 +78,17 @@ py_test_3_11(
6978
deps = ["//libs/my_lib"],
7079
)
7180

81+
copy_file(
82+
name = "copy_version_test",
83+
src = "version_test.py",
84+
out = "version_default_test.py",
85+
is_executable = True,
86+
)
87+
7288
py_test(
7389
name = "version_default_test",
74-
srcs = ["version_test.py"],
90+
srcs = ["version_default_test.py"],
7591
env = {"VERSION_CHECK": "3.9"}, # The default defined in the WORKSPACE.
76-
main = "version_test.py",
7792
)
7893

7994
py_test_3_8(

‎python/config_settings/private/BUILD.bazel

Whitespace-only changes.

‎python/config_settings/private/py_args.bzl

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
# Copyright 2023 The Bazel Authors. All rights reserved.
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
"""A helper to extract default args for the transition rule."""
16+
17+
def py_args(name, kwargs):
18+
"""A helper to extract common py_binary and py_test args
19+
20+
See https://bazel.build/reference/be/python#py_binary and
21+
https://bazel.build/reference/be/python#py_test for the list
22+
that should be returned
23+
24+
Args:
25+
name: The name of the target.
26+
kwargs: The kwargs to be extracted from; MODIFIED IN-PLACE.
27+
28+
Returns:
29+
A dict with the extracted arguments
30+
"""
31+
return dict(
32+
args = kwargs.pop("args", None),
33+
data = kwargs.pop("data", None),
34+
env = kwargs.pop("env", None),
35+
srcs = kwargs.pop("srcs", None),
36+
deps = kwargs.pop("deps", None),
37+
# See https://bazel.build/reference/be/python#py_binary.main
38+
# for default logic.
39+
# NOTE: This doesn't match the exact way a regular py_binary searches for
40+
# it's main amongst the srcs, but is close enough for most cases.
41+
main = kwargs.pop("main", name + ".py"),
42+
)

‎python/config_settings/transition.bzl

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ them to the desired target platform.
1818

1919
load("@bazel_skylib//lib:dicts.bzl", "dicts")
2020
load("//python:defs.bzl", _py_binary = "py_binary", _py_test = "py_test")
21+
load("//python/config_settings/private:py_args.bzl", "py_args")
2122

2223
def _transition_python_version_impl(_, attr):
2324
return {"//python/config_settings:python_version": str(attr.python_version)}
@@ -138,11 +139,13 @@ _transition_py_test = rule(
138139
)
139140

140141
def _py_rule(rule_impl, transition_rule, name, python_version, **kwargs):
141-
args = kwargs.pop("args", None)
142-
data = kwargs.pop("data", None)
143-
env = kwargs.pop("env", None)
144-
srcs = kwargs.pop("srcs", None)
145-
deps = kwargs.pop("deps", None)
142+
pyargs = py_args(name, kwargs)
143+
args = pyargs["args"]
144+
data = pyargs["data"]
145+
env = pyargs["env"]
146+
srcs = pyargs["srcs"]
147+
deps = pyargs["deps"]
148+
main = pyargs["main"]
146149

147150
# Attributes common to all build rules.
148151
# https://bazel.build/reference/be/common-definitions#common-attributes
@@ -197,6 +200,7 @@ def _py_rule(rule_impl, transition_rule, name, python_version, **kwargs):
197200
deps = deps,
198201
env = env,
199202
srcs = srcs,
203+
main = main,
200204
tags = ["manual"] + (tags if tags else []),
201205
visibility = ["//visibility:private"],
202206
**dicts.add(common_attrs, kwargs)

‎tests/config_settings/transition/BUILD.bazel

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
load(":py_args_tests.bzl", "py_args_test_suite")
2+
3+
py_args_test_suite(name = "py_args_tests")

‎tests/config_settings/transition/py_args_tests.bzl

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
# Copyright 2023 The Bazel Authors. All rights reserved.
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
""
16+
17+
load("@rules_testing//lib:test_suite.bzl", "test_suite")
18+
load("//python/config_settings/private:py_args.bzl", "py_args") # buildifier: disable=bzl-visibility
19+
20+
_tests = []
21+
22+
def _test_py_args_default(env):
23+
actual = py_args("foo", {})
24+
25+
want = {
26+
"args": None,
27+
"data": None,
28+
"deps": None,
29+
"env": None,
30+
"main": "foo.py",
31+
"srcs": None,
32+
}
33+
env.expect.that_dict(actual).contains_exactly(want)
34+
35+
_tests.append(_test_py_args_default)
36+
37+
def _test_kwargs_get_consumed(env):
38+
kwargs = {
39+
"args": ["some", "args"],
40+
"data": ["data"],
41+
"deps": ["deps"],
42+
"env": {"key": "value"},
43+
"main": "__main__.py",
44+
"srcs": ["__main__.py"],
45+
"visibility": ["//visibility:public"],
46+
}
47+
actual = py_args("bar_bin", kwargs)
48+
49+
want = {
50+
"args": ["some", "args"],
51+
"data": ["data"],
52+
"deps": ["deps"],
53+
"env": {"key": "value"},
54+
"main": "__main__.py",
55+
"srcs": ["__main__.py"],
56+
}
57+
env.expect.that_dict(actual).contains_exactly(want)
58+
env.expect.that_dict(kwargs).keys().contains_exactly(["visibility"])
59+
60+
_tests.append(_test_kwargs_get_consumed)
61+
62+
def py_args_test_suite(name):
63+
"""Create the test suite.
64+
65+
Args:
66+
name: the name of the test suite
67+
"""
68+
test_suite(name = name, basic_tests = _tests)

0 commit comments

Comments
 (0)
0