Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bazel CI] Multiple failures in integration tests #2469

Closed
sgowroji opened this issue Dec 5, 2024 · 6 comments
Closed

[Bazel CI] Multiple failures in integration tests #2469

sgowroji opened this issue Dec 5, 2024 · 6 comments
Assignees

Comments

@sgowroji
Copy link
Member

sgowroji commented Dec 5, 2024

CI: https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/4313#01939501-8a44-41dc-a7b2-fb29fd9561d3

Platform: Ubuntu

Logs:

FAIL: //tests/integration:compile_pip_requirements_workspace_test_bazel_self 
FAIL //tests/integration:ignore_root_user_error_workspace_test_bazel_self
FAIL //tests/integration:pip_parse_workspace_test_bazel_self
FAIL //tests/integration:py_cc_toolchain_registered_workspace_test_bazel_self

Culprit:

Steps:

git clone https://github.com/bazelbuild/rules_python
git reset 60da4a39e24381486d3d3bd089d516e3e0cad732  --hard
export USE_BAZEL_VERSION=3615f64a96f98efdb96ad48ed15cb0966d9961be
bazel test ... 

CC Greenteam @Wyverald

@rickeylev
Copy link
Collaborator

rickeylev commented Dec 5, 2024

Fixed by bazelbuild/continuous-integration#2141

*rules_python (RBE)
ERROR: While parsing option --toolchain_resolution_debug --remote_download_toplevel: Failed to build filter: value looks like another flag (--remote_download_toplevel). Either escape the value with "\-\-", or pass an explicit value to the flag.

I don't think this is a rules_python issue? We just use the Bazel CI provided RBE configs.


Fixed

  • rules_python (default, windows)
    • //tests/bootstrap_impls:run_binary_zip_no_test -- AssertionError: Cannot find .runfiles directory for c:\b\5bw2dcqy\execroot\_main\bazel-out\x64_windows-fastbuild\bin\tests\bootstrap_impls\run_binary_zip_no_test.exe.runfiles/_main/tests/bootstrap_impls/_run_binary_zip_no_test_bin Test case failed: using no runfiles env vars

Caused by odd sys.argv0 change to paths under bazel 9; see other comment.


Fixed

  • rules_python (default, workspace) (mac, linux)
    rules_python/python/private/reexports.bzl:37:17: name 'PyInfo' is not defined (did you mean 'CcInfo'?) (same with PyRuntimeInfo)

I thought I had fixed this previously? Well, I'll have to look again.


Fixed

  • examples/pip_repository_annotations
  • examples/py_proto_library
  • examples/pip_parse_vendored
  • examples/build_file_generation

ERROR: error loading package 'external': Both --enable_bzlmod and --enable_workspace are disabled, but one of them must be enabled to fetch external dependencies.`

I think this is because that example disable bzlmod, and with newer versions workspace must be explicitly enabled. Fix should be to modify its bazelrc to enable workspace.


Fixed

  • examples/multi_python_versions

/rules_python/examples/multi_python_versions/tests/BUILD.bazel:162:8: in sh_test rule //tests:version_test_binary_3_8: label '//tests:version_3_8' in $(location) expression expands to more than one file, please use $(locations //tests:version_3_8) instead. Files (at most 5 shown) are: [tests/_version_3_8.exe, tests/_version_3_8.zip, tests/version.py, tests/version_3_8.zip]

I had to fix a similar error in another test. Fix is to switch to using $(locations), then have the shell script parse out the appropriate entry. Long term, we need to change the rule behavior so that executables only produce 1 output, but that's a separate issue.

@rickeylev rickeylev self-assigned this Dec 5, 2024
github-merge-queue bot pushed a commit that referenced this issue Dec 6, 2024
Bazel 9 won't support workspace builds, so skip them in the Bazel@head
pipeline. The bzlmod
equivalents of these examples is covered elsewhere.

Along the way, enable workspace by default in the examples for a bit of
futureproofing.

Work towards #2469
@rickeylev
Copy link
Collaborator

rules_python/python/private/reexports.bzl:37:17: name 'PyInfo' is not defined

Ok, this one is weird -- the version of rules_python being used, in rules_python itself, is version 0.28.0. Reproing this is easy: USE_BAZEL_VERSION=last_green bazel build --nobuild ... --noenable_bzlmod --enable_workspace

@rickeylev
Copy link
Collaborator

rickeylev commented Dec 6, 2024

//tests/bootstrap_impls:run_binary_zip_no_test fail to find runfiles on windows

This seems to be because argv[0] is different on bazel 9, namely, forward slashes aren't normalized to backslashes. What makes this weird is it doesn't seem to be Bazel doing this?

The test is a shell script that runs a python program. It looks like this:

bin=$(rlocation $RUNFILE_PATH)
$bin

Where RUNFILE_PATH is the result of $(rootpaths), so it's a value like "foo/bar/baz". Thus, the result of bin is something like c:\users\whatever\bazel-bin\test.runfiles/foo/bar/baz. Note the mix of forward and backslashes.

On Bazel 7, when Python sees sys.argv[0], it sees all backslashes. On Bazel 9, it sees the original string with a mixture of forward and backslashes.

Later in the bootstrap logic, a regex is looking for .runfiles\ (a backslash, because windows). Since the actual string is runfiles/ (forward slash), the regex fails to match, and eventually an error occurs. Normalizing it to the OS separator fixes the issue.

It's weird that changing the Bazel version seems to cause this. Everything else seems to be the same (Python version, bash version).

github-merge-queue bot pushed a commit that referenced this issue Dec 7, 2024
This is basically part of #2395, but for the workspace test. Same as
that PR, the `$(rootpath)`
expansion isn't valid for a target with multiple outputs. To fix, use
`$(rootpaths)` and parse
out the particular value of interest.

Work towards #2469
aignas pushed a commit to ewianda/rules_python that referenced this issue Dec 7, 2024
This is for overall code hygiene, but also because it seems to make some
progress on Bazel 9
being able to load files in WORKSPACE mode (something about defs.bzl
triggers loading
more symbols which can't be found)

Work towards bazelbuild#2469
github-merge-queue bot pushed a commit that referenced this issue Dec 7, 2024
…zel 9 (#2481)

When the shell test invokes the python binary, it uses a combination of
forward slashes and
backslashes. Under Bazel 9, that mixture of slashes is preserved. This
later breaks a regex
that looks for the OS-specific path separator.

To fix, normalize forward slashes to the OS path separator.

Oddly, it's not Bazel that is passing the mixture of slashes (it's the
shell), but behavior seems to
vary based on which version of Bazel is used.

Along the way, copy the nicer `print_verbose` function from the stage2
bootstrap into the old
bootstrap. It prints debug information in a nicer format.

Work towards #2469
github-merge-queue bot pushed a commit that referenced this issue Dec 7, 2024
Update code and examples to load the object-specific bzl files instead
of the
generic `defs.bzl`. This is mostly for code hygiene, but came out of
trying to diagnose
why Bazel 9 workspace builds kept erroing with defs.bzl somehow related.
Removing
the internal usages of defs.bzl doesn't seem to fully fix it, but does
seem to eliminate
some errors, make some progress, and narrow down what's going on.

Work towards #2469
@rickeylev
Copy link
Collaborator

rules_python/python/private/reexports.bzl:37:17: name 'PyInfo' is not defined

I think I figured this out: defining the rules_python_internal repo was happening too late in the workspace processing. And then, i think bazel_tools ended up defining rules_python with an old version, somehow. In any case, defining rules_python_internal earlier seems to fix this. Another work around was to do local_repository(name="rules_python", path = ".")

ewianda pushed a commit to ewianda/rules_python that referenced this issue Dec 7, 2024
…zel 9 (bazelbuild#2481)

When the shell test invokes the python binary, it uses a combination of
forward slashes and
backslashes. Under Bazel 9, that mixture of slashes is preserved. This
later breaks a regex
that looks for the OS-specific path separator.

To fix, normalize forward slashes to the OS path separator.

Oddly, it's not Bazel that is passing the mixture of slashes (it's the
shell), but behavior seems to
vary based on which version of Bazel is used.

Along the way, copy the nicer `print_verbose` function from the stage2
bootstrap into the old
bootstrap. It prints debug information in a nicer format.

Work towards bazelbuild#2469
ewianda pushed a commit to ewianda/rules_python that referenced this issue Dec 7, 2024
…d#2483)

Update code and examples to load the object-specific bzl files instead
of the
generic `defs.bzl`. This is mostly for code hygiene, but came out of
trying to diagnose
why Bazel 9 workspace builds kept erroing with defs.bzl somehow related.
Removing
the internal usages of defs.bzl doesn't seem to fully fix it, but does
seem to eliminate
some errors, make some progress, and narrow down what's going on.

Work towards bazelbuild#2469
github-merge-queue bot pushed a commit that referenced this issue Dec 9, 2024
rules_proto is deprecated and recent versions simply forward onto
com_google_protobuf.
Older versions (e.g. 6.x used by us today), however, use the rules_proto
or native
(Bazel builtin) implementation. When those older versions are used with
Bazel 9,
which has removed various proto things, errors occur.

To fix, switch to using com_google_protobuf directly. More recent
versions of rules_proto
just forward onto com_google_protobuf anyways, so this just removes the
extra dependency
and having to deal with WORKSPACE setup.

Work towards #2469
@rickeylev
Copy link
Collaborator

rules_python/python/private/reexports.bzl:37:17: name 'PyInfo' is not defined

Ok, I give up trying to find a clean fix. Defining rules_python_internal helps, as did removing some unnecessary loads of bazel_tools in rules_python. However, I can't get past these errors with a nice fix:

ERROR: error loading package '@@bazel_tools//tools/jdk':
at /.../external/rules_python/python/defs.bzl:23:6:
at /.../external/rules_python/python/py_test.bzl:20:6:
at /.../external/rules_python/python/private/common/py_test_macro_bazel.bzl:17:6:
at /.../external/rules_python/python/private/common/py_test_rule_bazel.bzl:18:6:
at /.../external/rules_python/python/private/common/common.bzl:16:6: compilation of module 'python/private/reexports.bzl' failed
ERROR: /.../external/rules_java/toolchains/BUILD:356:27: error loading package '@@bazel_tools//tools/jdk':
at /.../external/rules_python/python/defs.bzl:23:6:
at /.../external/rules_python/python/py_test.bzl:20:6:
at /.../external/rules_python/python/private/common/py_test_macro_bazel.bzl:17:6:
at /.../external/rules_python/python/private/common/py_test_rule_bazel.bzl:18:6:
at /.../external/rules_python/python/private/common/common.bzl:16:6: compilation of module 'python/private/reexports.bzl' failed
  and referenced by '@@rules_java//toolchains:toolchain_java11_definition'
ERROR: /home/rlevasseur/p/rickeylev/rules_python/tests/pypi/whl_installer/BUILD.bazel:19:8: Analysis failed

Where ... is /home/rlevasseur/.cache/bazel/_bazel_rlevasseur/f967450e3051c3f6e7591e325c574df5`

My best guess about that cause is a java toolchain in bazel_tools needs @bazel_tools//tools/jdk:BUILD, which loads rules_python, and then autoloading defines rules_python using rules_python 0.28.0.

I can't fix those loads, though, so I'll just use the local_repository() trick I mentioned before.

github-merge-queue bot pushed a commit that referenced this issue Dec 13, 2024
…#2501)

Building with Bazel 9 using WORKSPACE results in an odd error that
`PyInfo`
isn't defined. Oddly, the error refers to
`rules_python/python/private/reexports.bzl`
for rules_python 0.28.0. This seems to only happen when the main module
is rules_python.

While Bazel 9 is supposed to drop workspace support, I've been advised
it's better to
keep testing WORKSPACE support until closer to when Bazel 9 fully
removes it.

My best guess about what's happening is Bazel's autoloading is
triggering and somehow
defining rules_python before it's recognized that the main module is
rules_python.
The autoloading appears to be triggered, eventually, by things in
bazel_tools loading
rules_python.

While removing unnecessary `@bazel_tools` loads in rules_python helps,
the particular
case I can't find a clean solution to is when
`@@rules_java//toolchains:toolchain_java11_definition` causes
rules_python to be
loaded. This appears to end up loading rules_python via
`@bazel_tools//tools/jdk:BUILD`, which has has some py rules defined in
it.

To fix/work around this issue, `local_repository` can be used to define
the `rules_python`
repo before autoloading happens. This appears to take precedence over
whatever logic
autoloading has.

Work towards #2469
@rickeylev
Copy link
Collaborator

I think everything here is fixed. The issue with the flag added by bazel ci was fixed by bazelbuild/continuous-integration#2141

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants