From ecc03fee24d1a72ad5e69d74b00e797cac23574e Mon Sep 17 00:00:00 2001 From: Jonathan Giroux Date: Wed, 26 Jun 2024 10:54:51 +0200 Subject: [PATCH 1/9] fix(windows): symlink bootstrap script when not building zip Fixes #1840 --- python/config_settings/transition.bzl | 47 +++++++++++++++++++-------- 1 file changed, 34 insertions(+), 13 deletions(-) diff --git a/python/config_settings/transition.bzl b/python/config_settings/transition.bzl index 48b0447ede..6416efba4b 100644 --- a/python/config_settings/transition.bzl +++ b/python/config_settings/transition.bzl @@ -43,24 +43,45 @@ def _transition_py_impl(ctx): output = executable, target_file = target[DefaultInfo].files_to_run.executable, ) - zipfile_symlink = None + symlinks = [] if target_is_windows: + files = target[DefaultInfo].default_runfiles.files.to_list() + + def try_get_file_from_path(short_path): + for file in files: + if file.short_path == short_path: + return file + return None + # Under Windows, the expected ".zip" does not exist, so we have to # create the symlink ourselves to achieve the same behaviour as in macOS # and Linux. - zipfile = None - expected_target_path = target[DefaultInfo].files_to_run.executable.short_path[:-4] + ".zip" - for file in target[DefaultInfo].default_runfiles.files.to_list(): - if file.short_path == expected_target_path: - zipfile = file - - if zipfile: - zipfile_symlink = ctx.actions.declare_file(ctx.attr.name + ".zip") + expected_zip_path = target[DefaultInfo].files_to_run.executable.short_path[:-4] + ".zip" + zip_file = try_get_file_from_path(expected_zip_path) + if zip_file: + zip_file_symlink = ctx.actions.declare_file(ctx.attr.name + ".zip") ctx.actions.symlink( is_executable = True, - output = zipfile_symlink, - target_file = zipfile, + output = zip_file_symlink, + target_file = zip_file, ) + symlinks.append(zip_file_symlink) + + else: + # In case --build_python_zip=false, bootstrap script needs to be symlinked as well. + expected_bootstrap_path = target[DefaultInfo].files_to_run.executable.short_path[:-4] + bootstrap_file = try_get_file_from_path(expected_bootstrap_path) + if bootstrap_file: + bootstrap_file_symlink = ctx.actions.declare_file(ctx.attr.name) + ctx.actions.symlink( + output = bootstrap_file_symlink, + target_file = bootstrap_file, + ) + symlinks.append(bootstrap_file_symlink) + + else: + fail("Bootstrap file should have been generated.") + env = {} for k, v in ctx.attr.env.items(): env[k] = ctx.expand_location(v) @@ -85,8 +106,8 @@ def _transition_py_impl(ctx): providers = [ DefaultInfo( executable = executable, - files = depset([zipfile_symlink] if zipfile_symlink else [], transitive = [target[DefaultInfo].files]), - runfiles = ctx.runfiles([zipfile_symlink] if zipfile_symlink else []).merge(target[DefaultInfo].default_runfiles), + files = depset(symlinks, transitive = [target[DefaultInfo].files]), + runfiles = ctx.runfiles(symlinks).merge(target[DefaultInfo].default_runfiles), ), py_info, py_runtime_info, From 28f2b50f22bf4c08b15fb7c1acf58624734f05ca Mon Sep 17 00:00:00 2001 From: Jonathan Giroux Date: Mon, 1 Jul 2024 12:12:48 +0200 Subject: [PATCH 2/9] refactor(windows): take the value of --build-python_zip into account --- python/config_settings/transition.bzl | 34 +++++++++++++++++---------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/python/config_settings/transition.bzl b/python/config_settings/transition.bzl index 6416efba4b..83b7063c15 100644 --- a/python/config_settings/transition.bzl +++ b/python/config_settings/transition.bzl @@ -53,19 +53,25 @@ def _transition_py_impl(ctx): return file return None - # Under Windows, the expected ".zip" does not exist, so we have to - # create the symlink ourselves to achieve the same behaviour as in macOS - # and Linux. - expected_zip_path = target[DefaultInfo].files_to_run.executable.short_path[:-4] + ".zip" - zip_file = try_get_file_from_path(expected_zip_path) - if zip_file: - zip_file_symlink = ctx.actions.declare_file(ctx.attr.name + ".zip") - ctx.actions.symlink( - is_executable = True, - output = zip_file_symlink, - target_file = zip_file, - ) - symlinks.append(zip_file_symlink) + build_zip_enabled = ctx.fragments.py.build_python_zip + + if build_zip_enabled: + # Under Windows, the expected ".zip" does not exist, so we have to + # create the symlink ourselves to achieve the same behaviour as in macOS + # and Linux. + expected_zip_path = target[DefaultInfo].files_to_run.executable.short_path[:-4] + ".zip" + zip_file = try_get_file_from_path(expected_zip_path) + if zip_file: + zip_file_symlink = ctx.actions.declare_file(ctx.attr.name + ".zip") + ctx.actions.symlink( + is_executable = True, + output = zip_file_symlink, + target_file = zip_file, + ) + symlinks.append(zip_file_symlink) + + else: + fail("Zip file should have been generated.") else: # In case --build_python_zip=false, bootstrap script needs to be symlinked as well. @@ -190,6 +196,7 @@ _transition_py_binary = rule( attrs = _COMMON_ATTRS | _PY_TEST_ATTRS, cfg = _transition_python_version, executable = True, + fragments = ["py"], ) _transition_py_test = rule( @@ -197,6 +204,7 @@ _transition_py_test = rule( attrs = _COMMON_ATTRS | _PY_TEST_ATTRS, cfg = _transition_python_version, test = True, + fragments = ["py"], ) def _py_rule(rule_impl, transition_rule, name, python_version, **kwargs): From 8878085f85d74f5b63f0aea4e1b4d643ec808f09 Mon Sep 17 00:00:00 2001 From: Jonathan Giroux Date: Mon, 1 Jul 2024 12:15:45 +0200 Subject: [PATCH 3/9] refactor(windows): find zip/bootstrap files among default outputs So that calling `to_list()` is faster. --- python/config_settings/transition.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/config_settings/transition.bzl b/python/config_settings/transition.bzl index 83b7063c15..209a796daf 100644 --- a/python/config_settings/transition.bzl +++ b/python/config_settings/transition.bzl @@ -45,7 +45,7 @@ def _transition_py_impl(ctx): ) symlinks = [] if target_is_windows: - files = target[DefaultInfo].default_runfiles.files.to_list() + files = target[DefaultInfo].files.to_list() def try_get_file_from_path(short_path): for file in files: From 93f0c64dd191996ec25772eb86d5ed49f541a285 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Wed, 3 Jul 2024 09:21:37 -0700 Subject: [PATCH 4/9] add a basic test to check outputs --- .../transition/multi_version_tests.bzl | 35 ++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/tests/config_settings/transition/multi_version_tests.bzl b/tests/config_settings/transition/multi_version_tests.bzl index f3707dba20..842c161d99 100644 --- a/tests/config_settings/transition/multi_version_tests.bzl +++ b/tests/config_settings/transition/multi_version_tests.bzl @@ -15,7 +15,7 @@ load("@rules_testing//lib:analysis_test.bzl", "analysis_test") load("@rules_testing//lib:test_suite.bzl", "test_suite") -load("@rules_testing//lib:util.bzl", rt_util = "util") +load("@rules_testing//lib:util.bzl", "TestingAspectInfo", rt_util = "util") load("//python/config_settings:transition.bzl", py_binary_transitioned = "py_binary", py_test_transitioned = "py_test") # NOTE @aignas 2024-06-04: we are using here something that is registered in the MODULE.Bazel @@ -68,6 +68,39 @@ def _test_py_binary_with_transition_impl(env, target): _tests.append(_test_py_binary_with_transition) +def _test_py_binary_windows_build_python_zip_false(name): + rt_util.helper_target( + py_binary_transitioned, + name = name + "_subject", + srcs = [name + "_subject.py"], + python_version = _PYTHON_VERSION, + ) + + analysis_test( + name = name, + target = name + "_subject", + impl = _test_py_binary_windows_build_python_zip_false_impl, + config_settings = { + "//command_line_option:build_python_zip": "false", + "//command_line_option:platforms": str(Label("//tests/support:windows_x86_64")), + "//command_line_option:extra_toolchains": "//tests/cc:all", + }, + ) + +def _test_py_binary_windows_build_python_zip_false_impl(env, target): + # todo: assert that the default outputs of target (the outer wrapper) + # matches the inner py_binary target) + print(target.files) + print(target[DefaultInfo].files_to_run.executable) + print("subject:\n ", target.files) + print("inner :\n ", target[TestingAspectInfo].attrs.target.files) + env.expect.that_target(target).default_outputs().contains_exactly([ + "{package}/{test_name}_subject.exe", + "{package}/{test_name}_subject", + ]) + +_tests.append(_test_py_binary_windows_build_python_zip_false) + def multi_version_test_suite(name): test_suite( name = name, From a563803800e436ba3e9be878ae62adfe1f2f8d10 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Wed, 3 Jul 2024 21:00:54 -0700 Subject: [PATCH 5/9] refactor code for clarity add basic analysis tests --- python/config_settings/transition.bzl | 35 ++++++++++---- .../transition/multi_version_tests.bzl | 46 +++++++++++++++---- 2 files changed, 62 insertions(+), 19 deletions(-) diff --git a/python/config_settings/transition.bzl b/python/config_settings/transition.bzl index 209a796daf..bc22992312 100644 --- a/python/config_settings/transition.bzl +++ b/python/config_settings/transition.bzl @@ -43,7 +43,7 @@ def _transition_py_impl(ctx): output = executable, target_file = target[DefaultInfo].files_to_run.executable, ) - symlinks = [] + default_outputs = [] if target_is_windows: files = target[DefaultInfo].files.to_list() @@ -55,10 +55,27 @@ def _transition_py_impl(ctx): build_zip_enabled = ctx.fragments.py.build_python_zip - if build_zip_enabled: - # Under Windows, the expected ".zip" does not exist, so we have to - # create the symlink ourselves to achieve the same behaviour as in macOS - # and Linux. + # Under Windows, the expected ".zip" does not exist, so we have to + # create the symlink ourselves to achieve the same behaviour as in macOS + # and Linux. + if ctx.fragments.py.build_python_zip: + suffix = ".zip" + symlink_executable = True + else: + suffix = "" + symlink_executable = False + + # The -4 chars stripped are the ".exe" part + search_for = target[DefaultInfo].files_to_run.executable.short_path[:-4] + suffix + underlying_launched_file = try_get_file_from_path(search_for) + if underlying_launched_file: + launched_file_symlink = ctx.actions.declare_file(ctx.attr.name + suffix) + ctx.actions.symlink( + executable = executable, + output = launched_file_symlink, + target_file = underlying_launched_file, + ) + default_outputs.append(launched_file_symlink) expected_zip_path = target[DefaultInfo].files_to_run.executable.short_path[:-4] + ".zip" zip_file = try_get_file_from_path(expected_zip_path) if zip_file: @@ -68,7 +85,7 @@ def _transition_py_impl(ctx): output = zip_file_symlink, target_file = zip_file, ) - symlinks.append(zip_file_symlink) + default_outputs.append(zip_file_symlink) else: fail("Zip file should have been generated.") @@ -83,7 +100,7 @@ def _transition_py_impl(ctx): output = bootstrap_file_symlink, target_file = bootstrap_file, ) - symlinks.append(bootstrap_file_symlink) + default_outputs.append(bootstrap_file_symlink) else: fail("Bootstrap file should have been generated.") @@ -112,8 +129,8 @@ def _transition_py_impl(ctx): providers = [ DefaultInfo( executable = executable, - files = depset(symlinks, transitive = [target[DefaultInfo].files]), - runfiles = ctx.runfiles(symlinks).merge(target[DefaultInfo].default_runfiles), + files = depset(default_outputs, transitive = [target[DefaultInfo].files]), + runfiles = ctx.runfiles(default_outputs).merge(target[DefaultInfo].default_runfiles), ), py_info, py_runtime_info, diff --git a/tests/config_settings/transition/multi_version_tests.bzl b/tests/config_settings/transition/multi_version_tests.bzl index 842c161d99..65ed14943e 100644 --- a/tests/config_settings/transition/multi_version_tests.bzl +++ b/tests/config_settings/transition/multi_version_tests.bzl @@ -68,7 +68,7 @@ def _test_py_binary_with_transition_impl(env, target): _tests.append(_test_py_binary_with_transition) -def _test_py_binary_windows_build_python_zip_false(name): +def _setup_py_binary_windows(name, *, impl, build_python_zip): rt_util.helper_target( py_binary_transitioned, name = name + "_subject", @@ -79,28 +79,54 @@ def _test_py_binary_windows_build_python_zip_false(name): analysis_test( name = name, target = name + "_subject", - impl = _test_py_binary_windows_build_python_zip_false_impl, + impl = impl, config_settings = { - "//command_line_option:build_python_zip": "false", + "//command_line_option:build_python_zip": build_python_zip, "//command_line_option:platforms": str(Label("//tests/support:windows_x86_64")), "//command_line_option:extra_toolchains": "//tests/cc:all", }, ) +def _test_py_binary_windows_build_python_zip_false(name): + _setup_py_binary_windows( + name, + build_python_zip = "false", + impl = _test_py_binary_windows_build_python_zip_false_impl, + ) + def _test_py_binary_windows_build_python_zip_false_impl(env, target): - # todo: assert that the default outputs of target (the outer wrapper) - # matches the inner py_binary target) - print(target.files) - print(target[DefaultInfo].files_to_run.executable) - print("subject:\n ", target.files) - print("inner :\n ", target[TestingAspectInfo].attrs.target.files) + # TODO: These outputs aren't correct. The outputs shouldn't + # have the "_" prefix on them (those are coming from the underlying + # wrapped binary). env.expect.that_target(target).default_outputs().contains_exactly([ - "{package}/{test_name}_subject.exe", + "{package}/_{test_name}_subject", + "{package}/_{test_name}_subject.exe", "{package}/{test_name}_subject", + "{package}/{test_name}_subject.py", ]) _tests.append(_test_py_binary_windows_build_python_zip_false) +def _test_py_binary_windows_build_python_zip_true(name): + _setup_py_binary_windows( + name, + build_python_zip = "true", + impl = _test_py_binary_windows_build_python_zip_true_impl, + ) + +def _test_py_binary_windows_build_python_zip_true_impl(env, target): + # TODO: These outputs aren't correct. The outputs shouldn't + # have the "_" prefix on them (those are coming from the underlying + # wrapped binary). + env.expect.that_target(target).default_outputs().contains_exactly([ + "{package}/_{test_name}_subject.exe", + "{package}/_{test_name}_subject.zip", + "{package}/{test_name}_subject.py", + "{package}/{test_name}_subject.zip", + ]) + +_tests.append(_test_py_binary_windows_build_python_zip_true) + def multi_version_test_suite(name): test_suite( name = name, From 170f765fbe1bbba3a6fd006dce97ca1316b40925 Mon Sep 17 00:00:00 2001 From: Jonathan Giroux Date: Thu, 4 Jul 2024 11:38:09 +0200 Subject: [PATCH 6/9] continue refactor --- python/config_settings/transition.bzl | 41 ++++----------------------- 1 file changed, 6 insertions(+), 35 deletions(-) diff --git a/python/config_settings/transition.bzl b/python/config_settings/transition.bzl index bc22992312..c4122bbf44 100644 --- a/python/config_settings/transition.bzl +++ b/python/config_settings/transition.bzl @@ -53,17 +53,14 @@ def _transition_py_impl(ctx): return file return None - build_zip_enabled = ctx.fragments.py.build_python_zip - - # Under Windows, the expected ".zip" does not exist, so we have to - # create the symlink ourselves to achieve the same behaviour as in macOS - # and Linux. if ctx.fragments.py.build_python_zip: + # Under Windows, the expected ".zip" does not exist, so we have to + # create the symlink ourselves to achieve the same behaviour as in macOS + # and Linux. suffix = ".zip" - symlink_executable = True else: + # In case --build_python_zip=false, bootstrap script needs to be symlinked as well. suffix = "" - symlink_executable = False # The -4 chars stripped are the ".exe" part search_for = target[DefaultInfo].files_to_run.executable.short_path[:-4] + suffix @@ -71,39 +68,13 @@ def _transition_py_impl(ctx): if underlying_launched_file: launched_file_symlink = ctx.actions.declare_file(ctx.attr.name + suffix) ctx.actions.symlink( - executable = executable, + is_executable = True, output = launched_file_symlink, target_file = underlying_launched_file, ) default_outputs.append(launched_file_symlink) - expected_zip_path = target[DefaultInfo].files_to_run.executable.short_path[:-4] + ".zip" - zip_file = try_get_file_from_path(expected_zip_path) - if zip_file: - zip_file_symlink = ctx.actions.declare_file(ctx.attr.name + ".zip") - ctx.actions.symlink( - is_executable = True, - output = zip_file_symlink, - target_file = zip_file, - ) - default_outputs.append(zip_file_symlink) - - else: - fail("Zip file should have been generated.") - else: - # In case --build_python_zip=false, bootstrap script needs to be symlinked as well. - expected_bootstrap_path = target[DefaultInfo].files_to_run.executable.short_path[:-4] - bootstrap_file = try_get_file_from_path(expected_bootstrap_path) - if bootstrap_file: - bootstrap_file_symlink = ctx.actions.declare_file(ctx.attr.name) - ctx.actions.symlink( - output = bootstrap_file_symlink, - target_file = bootstrap_file, - ) - default_outputs.append(bootstrap_file_symlink) - - else: - fail("Bootstrap file should have been generated.") + fail("file with path `{}` should have been generated by underlying target".format(search_for)) env = {} for k, v in ctx.attr.env.items(): From fdbb9455efef720e214dadc115d3133763c665ce Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Thu, 4 Jul 2024 16:19:00 -0700 Subject: [PATCH 7/9] make bazel 6 happy --- python/config_settings/transition.bzl | 41 +++++++++++-------- .../transition/multi_version_tests.bzl | 8 ++++ 2 files changed, 32 insertions(+), 17 deletions(-) diff --git a/python/config_settings/transition.bzl b/python/config_settings/transition.bzl index c4122bbf44..da48a1fdb3 100644 --- a/python/config_settings/transition.bzl +++ b/python/config_settings/transition.bzl @@ -45,26 +45,29 @@ def _transition_py_impl(ctx): ) default_outputs = [] if target_is_windows: - files = target[DefaultInfo].files.to_list() + # NOTE: Bazel 6 + host=linux + target=windows results in the .exe extension missing + inner_bootstrap_path = _strip_suffix(target[DefaultInfo].files_to_run.executable.short_path, ".exe") + inner_bootstrap = None + inner_zip_file_path = inner_bootstrap_path + ".zip" + inner_zip_file = None + for file in target[DefaultInfo].files.to_list(): + if file.short_path == inner_bootstrap_path: + inner_bootstrap = file + elif file.short_path == inner_zip_file_path: + inner_zip_file = file - def try_get_file_from_path(short_path): - for file in files: - if file.short_path == short_path: - return file - return None - - if ctx.fragments.py.build_python_zip: - # Under Windows, the expected ".zip" does not exist, so we have to - # create the symlink ourselves to achieve the same behaviour as in macOS - # and Linux. + # TODO: Use `fragments.py.build_python_zip` once Bazel 6 support is dropped. + # Which file the Windows .exe looks for depends on the --build_python_zip file. + # Bazel 7+ has APIs to know the effective value of that flag, but not Bazel 6. + # To work around this, we treat the existence of a .zip in the default outputs + # to mean --build_python_zip=true. + if inner_zip_file: suffix = ".zip" + underlying_launched_file = inner_zip_file else: - # In case --build_python_zip=false, bootstrap script needs to be symlinked as well. suffix = "" + underlying_launched_file = inner_bootstrap - # The -4 chars stripped are the ".exe" part - search_for = target[DefaultInfo].files_to_run.executable.short_path[:-4] + suffix - underlying_launched_file = try_get_file_from_path(search_for) if underlying_launched_file: launched_file_symlink = ctx.actions.declare_file(ctx.attr.name + suffix) ctx.actions.symlink( @@ -73,8 +76,6 @@ def _transition_py_impl(ctx): target_file = underlying_launched_file, ) default_outputs.append(launched_file_symlink) - else: - fail("file with path `{}` should have been generated by underlying target".format(search_for)) env = {} for k, v in ctx.attr.env.items(): @@ -280,3 +281,9 @@ def py_binary(name, python_version, **kwargs): def py_test(name, python_version, **kwargs): return _py_rule(_py_test, _transition_py_test, name, python_version, **kwargs) + +def _strip_suffix(s, suffix): + if s.endswith(suffix): + return s[:-len(suffix)] + else: + return s diff --git a/tests/config_settings/transition/multi_version_tests.bzl b/tests/config_settings/transition/multi_version_tests.bzl index 65ed14943e..5750d93f17 100644 --- a/tests/config_settings/transition/multi_version_tests.bzl +++ b/tests/config_settings/transition/multi_version_tests.bzl @@ -17,6 +17,7 @@ load("@rules_testing//lib:analysis_test.bzl", "analysis_test") load("@rules_testing//lib:test_suite.bzl", "test_suite") load("@rules_testing//lib:util.bzl", "TestingAspectInfo", rt_util = "util") load("//python/config_settings:transition.bzl", py_binary_transitioned = "py_binary", py_test_transitioned = "py_test") +load("//python/private:util.bzl", "IS_BAZEL_7_OR_HIGHER") # NOTE @aignas 2024-06-04: we are using here something that is registered in the MODULE.Bazel # and if you find tests failing, it could be because of the toolchain resolution issues here. @@ -69,6 +70,13 @@ def _test_py_binary_with_transition_impl(env, target): _tests.append(_test_py_binary_with_transition) def _setup_py_binary_windows(name, *, impl, build_python_zip): + # Testing this with Bazel 6 is hard because Bazel checks the host OS (not + # desired target platform) to determine the --build_python_zip value and + # using the .exe extension. APIs for those are available in Bazel 7+. + if not IS_BAZEL_7_OR_HIGHER: + rt_util.skip_test(name = name) + return + rt_util.helper_target( py_binary_transitioned, name = name + "_subject", From de544ea9d5df1846919f0a28d25817839b2e3fdd Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Thu, 4 Jul 2024 16:27:18 -0700 Subject: [PATCH 8/9] buildifer format --- .../transition/multi_version_tests.bzl | 62 +++++++++++-------- 1 file changed, 35 insertions(+), 27 deletions(-) diff --git a/tests/config_settings/transition/multi_version_tests.bzl b/tests/config_settings/transition/multi_version_tests.bzl index 5750d93f17..a7fd662012 100644 --- a/tests/config_settings/transition/multi_version_tests.bzl +++ b/tests/config_settings/transition/multi_version_tests.bzl @@ -17,7 +17,7 @@ load("@rules_testing//lib:analysis_test.bzl", "analysis_test") load("@rules_testing//lib:test_suite.bzl", "test_suite") load("@rules_testing//lib:util.bzl", "TestingAspectInfo", rt_util = "util") load("//python/config_settings:transition.bzl", py_binary_transitioned = "py_binary", py_test_transitioned = "py_test") -load("//python/private:util.bzl", "IS_BAZEL_7_OR_HIGHER") +load("//python/private:util.bzl", "IS_BAZEL_7_OR_HIGHER") # buildifier: disable=bzl-visiblity # NOTE @aignas 2024-06-04: we are using here something that is registered in the MODULE.Bazel # and if you find tests failing, it could be because of the toolchain resolution issues here. @@ -70,13 +70,6 @@ def _test_py_binary_with_transition_impl(env, target): _tests.append(_test_py_binary_with_transition) def _setup_py_binary_windows(name, *, impl, build_python_zip): - # Testing this with Bazel 6 is hard because Bazel checks the host OS (not - # desired target platform) to determine the --build_python_zip value and - # using the .exe extension. APIs for those are available in Bazel 7+. - if not IS_BAZEL_7_OR_HIGHER: - rt_util.skip_test(name = name) - return - rt_util.helper_target( py_binary_transitioned, name = name + "_subject", @@ -90,8 +83,8 @@ def _setup_py_binary_windows(name, *, impl, build_python_zip): impl = impl, config_settings = { "//command_line_option:build_python_zip": build_python_zip, - "//command_line_option:platforms": str(Label("//tests/support:windows_x86_64")), "//command_line_option:extra_toolchains": "//tests/cc:all", + "//command_line_option:platforms": str(Label("//tests/support:windows_x86_64")), }, ) @@ -103,15 +96,22 @@ def _test_py_binary_windows_build_python_zip_false(name): ) def _test_py_binary_windows_build_python_zip_false_impl(env, target): - # TODO: These outputs aren't correct. The outputs shouldn't - # have the "_" prefix on them (those are coming from the underlying - # wrapped binary). - env.expect.that_target(target).default_outputs().contains_exactly([ - "{package}/_{test_name}_subject", - "{package}/_{test_name}_subject.exe", - "{package}/{test_name}_subject", - "{package}/{test_name}_subject.py", - ]) + default_outputs = env.expect.that_target(target).default_outputs() + if IS_BAZEL_7_OR_HIGHER: + # TODO: These outputs aren't correct. The outputs shouldn't + # have the "_" prefix on them (those are coming from the underlying + # wrapped binary). + env.expect.that_target(target).default_outputs().contains_exactly([ + "{package}/_{test_name}_subject", + "{package}/_{test_name}_subject.exe", + "{package}/{test_name}_subject", + "{package}/{test_name}_subject.py", + ]) + else: + inner_exe = target[TestingAspectInfo].attrs.target[DefaultInfo].files_to_run.executable + default_outputs.contains_at_least([ + inner_exe.short_path, + ]) _tests.append(_test_py_binary_windows_build_python_zip_false) @@ -123,15 +123,23 @@ def _test_py_binary_windows_build_python_zip_true(name): ) def _test_py_binary_windows_build_python_zip_true_impl(env, target): - # TODO: These outputs aren't correct. The outputs shouldn't - # have the "_" prefix on them (those are coming from the underlying - # wrapped binary). - env.expect.that_target(target).default_outputs().contains_exactly([ - "{package}/_{test_name}_subject.exe", - "{package}/_{test_name}_subject.zip", - "{package}/{test_name}_subject.py", - "{package}/{test_name}_subject.zip", - ]) + default_outputs = env.expect.that_target(target).default_outputs() + if IS_BAZEL_7_OR_HIGHER: + # TODO: These outputs aren't correct. The outputs shouldn't + # have the "_" prefix on them (those are coming from the underlying + # wrapped binary). + default_outputs.contains_exactly([ + "{package}/_{test_name}_subject.exe", + "{package}/_{test_name}_subject.zip", + "{package}/{test_name}_subject.py", + "{package}/{test_name}_subject.zip", + ]) + else: + inner_exe = target[TestingAspectInfo].attrs.target[DefaultInfo].files_to_run.executable + default_outputs.contains_at_least([ + "{package}/{test_name}_subject.zip", + inner_exe.short_path, + ]) _tests.append(_test_py_binary_windows_build_python_zip_true) From eb118da99110c3197b5e7480a96c4c788c0713cd Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Thu, 4 Jul 2024 16:30:56 -0700 Subject: [PATCH 9/9] fix buildifier disable typo --- tests/config_settings/transition/multi_version_tests.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/config_settings/transition/multi_version_tests.bzl b/tests/config_settings/transition/multi_version_tests.bzl index a7fd662012..e35590bbb6 100644 --- a/tests/config_settings/transition/multi_version_tests.bzl +++ b/tests/config_settings/transition/multi_version_tests.bzl @@ -17,7 +17,7 @@ load("@rules_testing//lib:analysis_test.bzl", "analysis_test") load("@rules_testing//lib:test_suite.bzl", "test_suite") load("@rules_testing//lib:util.bzl", "TestingAspectInfo", rt_util = "util") load("//python/config_settings:transition.bzl", py_binary_transitioned = "py_binary", py_test_transitioned = "py_test") -load("//python/private:util.bzl", "IS_BAZEL_7_OR_HIGHER") # buildifier: disable=bzl-visiblity +load("//python/private:util.bzl", "IS_BAZEL_7_OR_HIGHER") # buildifier: disable=bzl-visibility # NOTE @aignas 2024-06-04: we are using here something that is registered in the MODULE.Bazel # and if you find tests failing, it could be because of the toolchain resolution issues here.