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 7.0.1 reports cyclic dependency in rules_python #20942

Closed
rickeylev opened this issue Jan 18, 2024 · 16 comments
Closed

Bazel 7.0.1 reports cyclic dependency in rules_python #20942

rickeylev opened this issue Jan 18, 2024 · 16 comments
Labels
area-Bzlmod Bzlmod-specific PRs, issues, and feature requests P1 I'll work on this now. (Assignee required) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: bug

Comments

@rickeylev
Copy link
Contributor

rickeylev commented Jan 18, 2024

Description of the bug:

Bazel 7.0.1 reports a cyclic dependency in rules_python. However, 7.0.0 and the 8.x/rolling releases don't exhibit this bug

Which category does this issue belong to?

Core

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

Checkout rules python, enable the starlark implementation, and build with 7.0.1

git clone https://github.com/bazelbuild/rules_python.git
env USE_BAZEL_VERSION=7.0.1 bazel build --nobuild //:distribution

The 7.0.0 release and rolling (8.x) releases don't have this problem

See also: https://buildkite.com/bazel/rules-python-python/builds/6938#018d1ee3-ca77-4167-bcce-3ac335c7d203

The error is

(23:24:37) ERROR: Failed to load .bzl file '@@_main~internal_deps~rules_python_internal//:rules_python_config.bzl': possible dependency cycle detected.
(23:24:37) ERROR: Error computing the main repository mapping: cycles detected during computation of main repo mapping

Which operating system are you running Bazel on?

linux

What is the output of bazel info release?

release 7.0.1

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

ssh://[email protected]:443/rickeylev/rules_python
6607a4ae4f680af879a0148e95a7ea78eca9dafa
6607a4ae4f680af879a0148e95a7ea78eca9dafa

Is this a regression? If yes, please try to identify the Bazel commit where the bug was introduced.

Yes. Not sure, I'll look into it. Just want to file this sooner rather than later

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

No response

@rickeylev
Copy link
Contributor Author

Hm, odd. Trying to build in the root of rules_python gives that failure, but not when building in one of the child workspaces. So that's good news -- maybe this is just localized to something about rules_python's root setup

@rickeylev
Copy link
Contributor Author

export RULES_PYTHON_ENABLE_PYSTAR=1
bazelisk --bisect=7.0.0..7.0.1 build //:distribution

Pointed to an innocent looking commit (39b9954); I can't imagine why retrying ipv6 binding would be the culprit.

So the commit must be somewhere between 7.0.0 and that commit?

There's only 20 commits (7.0.0...7.0.1), so this shouldn't be too hard to narrow down. I see a few changes to repo mapping stuff, which seem like likely culprits. Still odd that 8.x doesn't have this problem.

@Wyverald
Copy link
Member

huh. I tried the same bisect, with BAZELISK_CLEAN=1 even, and got the same result. What's really fishy is that the parent commit of 39b9954, f4da34d, also fails when I try it manually, but Bazelisk bisect seems to think it's good:

$ USE_BAZEL_VERSION=f4da34dcfe7b83388e3d963f35581a4fe710fc14 bazel build --nobuild //...
Starting local Bazel server and connecting to it...
ERROR: Failed to load .bzl file '@@_main~internal_deps~rules_python_internal//:rules_python_config.bzl': possible dependency cycle detected.
ERROR: Error computing the main repository mapping: cycles detected during computation of main repo mapping
Computing main repo mapping: 
$ bazelisk --bisect=7.0.0..7.0.1 build --nobuild //...
...<lines omitted>...
--- Testing with Bazel built at f4da34dcfe7b83388e3d963f35581a4fe710fc14, 9 commits remaining...

bazel clean --expunge
Starting local Bazel server and connecting to it...
INFO: Starting clean (this may take a while). Consider using --async if the clean takes more than several minutes.

bazel build --nobuild //...
Starting local Bazel server and connecting to it...
WARNING: /Users/wyv/github/rules_python/docs/BUILD.bazel:31:6: target '//docs:bazel_repo_tools' is deprecated: Use @bazel_tools//tools:bzl_srcs instead; targets under //docs are internal.
WARNING: /Users/wyv/github/rules_python/docs/BUILD.bazel:25:6: target '//docs:defs' is deprecated: Use //python:defs_bzl instead; targets under //docs are internal.
WARNING: /Users/wyv/github/rules_python/docs/BUILD.bazel:37:12: target '//docs:pip_install_bzl' is deprecated: Use //python:pip_bzl or //python/pip_install:pip_repository_bzl instead; targets under //docs are internal.
WARNING: /Users/wyv/github/rules_python/docs/BUILD.bazel:47:6: target '//docs:requirements_parser_bzl' is deprecated: Use //python/pip_install:pip_repository_bzl instead; Both the requirements parser and targets under //docs are internal
INFO: Analyzed 626 targets (275 packages loaded, 15188 targets configured).
INFO: Found 626 targets...
INFO: Elapsed time: 25.618s, Critical Path: 0.00s
INFO: 0 processes.
INFO: Build completed successfully, 0 total actions


--- Succeeded at f4da34dcfe7b83388e3d963f35581a4fe710fc14
...<lines omitted>...

@Wyverald
Copy link
Member

A manual linear-sect pointed at ce993c4 as the culprit. Building with --lockfile_mode=off succeeds, though.

cc @fmeum @meteorcloudy @SalmaSamy

Now to find out:

  • why this isn't failing on HEAD Bazel
  • why bazelisk --bisect found the wrong commit
  • why turning off the lockfile avoids the failure
  • why there was no error message for the cycle

Will keep investigating tomorrow.

@Wyverald Wyverald added P1 I'll work on this now. (Assignee required) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. area-Bzlmod Bzlmod-specific PRs, issues, and feature requests and removed untriaged labels Jan 19, 2024
@rickeylev
Copy link
Contributor Author

I found a faster repro is to build //:distribution instead of //... btw. That's just a fileset of filesets of glob(**)'s

@rickeylev
Copy link
Contributor Author

rickeylev commented Jan 19, 2024

Yeah, the lockfile seems to be related.

Setting RULES_PYTHON_ENABLE_PYSTAR=1 in the environment doesn't seem to be necessary. I'm able to repro this with or without the set now. I'll update the original post.

After running bisects, I then ran the original repro command: success

  • 2nd time: failure
  • 3rd time: failure
  • 4th time: deleted the lock file, then ran: success
  • 5th time: failure
  • 6th time: ran with --lockfile_mode=off: success

Thanks! We can add --lockfile_mode=off to our bazelrc to work around this in the meantime.

The only thing special about rules_python repo/module stuff I can think of is it sets os/arch dependent=True.

I tried modifying internal_config_repo.bzl in various ways, but the error persists. When I simplify it down to a no-op -- a repo rule that creates nothing but an empty BUILD file, the first build gives a (correct and expected) error about a load() failing because the repo is missing a file (because it wasn't generated). The second build gives the cyclic dependency error. Setting lockfile_mode=off makes it go back to giving the first error. (I mention this because that's the only thing mentioned in the error, and just wanted to make sure it's not related to what that repo rule is doing; it appears not)

rickeylev added a commit to rickeylev/rules_python that referenced this issue Jan 19, 2024
Bazel 7.0.1 has a bug where lockfiles can cause a bogus error about
cyclic dependencies. Removing the lockfile or disabling the lockfiles
prevents it from triggering.

See bazelbuild/bazel#20942
@fmeum
Copy link
Collaborator

fmeum commented Jan 19, 2024

The cycle:

REPOSITORY_MAPPING: Key{repoName=@@, rootModuleShouldSeeWorkspaceRepos=true}
PACKAGE: external
EXTERNAL_PACKAGE: com.google.devtools.build.lib.skyframe.ExternalPackageFunction$$Lambda$263/0x0000000840200c40@2812f338
WORKSPACE_FILE: [/home/fhenneke/git/rules_python]/[WORKSPACE], 11
BZL_LOAD: KeyForWorkspace{label=//python:pip.bzl, isBuildPrelude=false}
BZL_LOAD: KeyForWorkspace{label=//python/pip_install:requirements.bzl, isBuildPrelude=false}
BZL_LOAD: KeyForWorkspace{label=//python:defs.bzl, isBuildPrelude=false}
BZL_LOAD: KeyForWorkspace{label=//python:py_import.bzl, isBuildPrelude=false}
BZL_LOAD: KeyForWorkspace{label=//python:py_info.bzl, isBuildPrelude=false}
BZL_LOAD: KeyForWorkspace{label=@@_main~internal_deps~rules_python_internal//:rules_python_config.bzl, isBuildPrelude=false}
CONTAINING_PACKAGE_LOOKUP: CONTAINING_PACKAGE_LOOKUP:@@_main~internal_deps~rules_python_internal//
PACKAGE_LOOKUP: PACKAGE_LOOKUP:@@_main~internal_deps~rules_python_internal//
REPOSITORY_DIRECTORY: REPOSITORY_DIRECTORY:@@_main~internal_deps~rules_python_internal
BZLMOD_REPO_RULE: BZLMOD_REPO_RULE:@@_main~internal_deps~rules_python_internal
SINGLE_EXTENSION_EVAL: SINGLE_EXTENSION_EVAL:ModuleExtensionId{bzlFileLabel=//python/private/bzlmod:internal_deps.bzl, extensionName=internal_deps, isolationKey=Optional.empty}

This suggests the workaround of touch WORKSPACE.bzlmod, but I don't yet know why this is happening.

@fmeum
Copy link
Collaborator

fmeum commented Jan 19, 2024

A linear bisect on my machine, where I delete MODULE.bazel.lock before every build and run the build twice points to 476e183. It's always the second build that fails, which is the first to use the lockfile. ce993c4 only touches JVM rules, so it's rather unlikely to be the cause here (although not impossible, it touches the "bootstrap" part of Bzlmod).

Edit: This makes sense as the cause: Validating the up-to-dateness of main repo mapping entries requests the full repo mapping, which causes a cycle if WORKSPACE evaluation, which is needed for the full repo mapping, also depends on a module extension with recorded repo mapping entries. The internal deps extension has such entries:

        "recordedRepoMappingEntries": [
          [
            "",
            "bazel_tools",
            "bazel_tools"
          ]
        ]

I'm not sure how to fix this. The chunked nature of WORKSPACE means that we only learn its repo mapping step by step and we certainly don't want to have a special repo mapping key for each chunk.

@fmeum
Copy link
Collaborator

fmeum commented Jan 19, 2024

Maybe we could pretend that repo mappings aren't used in WORKSPACE and restrict the recording and verification to the main repo mapping without WORKSPACE repos?

@meteorcloudy
Copy link
Member

SGTM, repos from WORKSPACE should not have any effect on Bzlmod resolution anyway.

github-merge-queue bot pushed a commit to bazelbuild/rules_python that referenced this issue Jan 20, 2024
…1705)

Bazel 7.0.1 has a bug where lockfiles can cause a bogus error about
cyclic dependencies. Removing the lockfile or disabling the lockfiles
prevents it from triggering. We aren't
using lockfiles anyways.

This should make CI happy, which uses 7.0.1 in some configurations.
Local development
isn't impacted because .bazelversion specifies 7.0.0

See bazelbuild/bazel#20942
@Wyverald
Copy link
Member

Amazing detective work, @fmeum !

Maybe we could pretend that repo mappings aren't used in WORKSPACE and restrict the recording and verification to the main repo mapping without WORKSPACE repos?

This is a bit easier said than done -- at "recording" time, we don't actually know whether a repo mapping entry is from WORKSPACE or Bzlmod. Verification time, sure -- that's easy; we could just request RepositoryMappingValue.KEY_FOR_ROOT_MODULE_WITHOUT_WORKSPACE_REPOS whenever the main repo appears in the list.

The good thing is, I think that's exactly what we should do. This cycle only appears when we're running an extension in the main repo, which exactly matches this case. I'll try to see if doing this fixes the issue.

@Wyverald
Copy link
Member

Hmm. I still wonder why this doesn't happen with HEAD Bazel, but solving that particular mystery is lower priority.

@Wyverald
Copy link
Member

Ah, I think I figured out why this confused bazelisk --bisect (and my manual linear-sect...) -- since we overwrite the lockfile by default (--lockfile_mode=update), each bisect check actually produces a visible side effect. Obviously bazel clean --expunge isn't enough to reset the state. What exacerbated this was that rules_python has MODULE.bazel.lock in its .gitignore file, which means that it wasn't immediately obvious to us that the lockfile had changed during the bisect.

This also explains why it wasn't failing with HEAD Bazel: it actual is failing with HEAD Bazel if you make sure to delete the lockfile first and then build twice.

In happier news, the trick I described works. Will send a PR soon.

Wyverald added a commit that referenced this issue Jan 22, 2024
…extension eval

See code comment and linked issue for more information.

Fixes #20942.
@Wyverald
Copy link
Member

@bazel-io fork 7.0.2

@Wyverald Wyverald added this to the 6.5.0 release blockers milestone Jan 23, 2024
@Wyverald Wyverald removed this from the 6.5.0 release blockers milestone Jan 23, 2024
bazel-io pushed a commit to bazel-io/bazel that referenced this issue Jan 23, 2024
…extension eval

See code comment and linked issue for more information.

Fixes bazelbuild#20942.

Closes bazelbuild#20982.

PiperOrigin-RevId: 600856392
Change-Id: I5b8a0ed3a38e37ab51ffb49b19a59f2e161b9a33
@iancha1992
Copy link
Member

@bazel-io fork 7.1.0

iancha1992 pushed a commit that referenced this issue Jan 23, 2024
…ries in extension eval (#21003)

See code comment and linked issue for more information.

Fixes #20942.

Closes #20982.

Commit
21508b1

PiperOrigin-RevId: 600856392
Change-Id: I5b8a0ed3a38e37ab51ffb49b19a59f2e161b9a33

---------

Co-authored-by: Xdng Yng <[email protected]>
copybara-service bot pushed a commit that referenced this issue Jan 24, 2024
Work towards #20942

On the referenced bug, this prints:
```
ERROR: Circular definition of repositories generated by module extensions and/or .bzl files:
.-> repository mapping of @@
|   WORKSPACE file
|   //python:pip.bzl
|   //python/pip_install:requirements.bzl
|   //python:defs.bzl
|   //python:py_import.bzl
|   //python:py_info.bzl
|   @@_main~internal_deps~rules_python_internal//:rules_python_config.bzl
|   @@_main~internal_deps~rules_python_internal
|   extension 'internal_deps' defined in //python/private/bzlmod:internal_deps.bzl
`-- repository mapping of @@
ERROR: Error computing the main repository mapping: cycles detected during computation of main repo mapping
```

Closes #20958.

PiperOrigin-RevId: 601183514
Change-Id: If321005b1f6918117d0f5b5763ae2bda4d8e34f1
bazel-io pushed a commit to bazel-io/bazel that referenced this issue Jan 24, 2024
Work towards bazelbuild#20942

On the referenced bug, this prints:
```
ERROR: Circular definition of repositories generated by module extensions and/or .bzl files:
.-> repository mapping of @@
|   WORKSPACE file
|   //python:pip.bzl
|   //python/pip_install:requirements.bzl
|   //python:defs.bzl
|   //python:py_import.bzl
|   //python:py_info.bzl
|   @@_main~internal_deps~rules_python_internal//:rules_python_config.bzl
|   @@_main~internal_deps~rules_python_internal
|   extension 'internal_deps' defined in //python/private/bzlmod:internal_deps.bzl
`-- repository mapping of @@
ERROR: Error computing the main repository mapping: cycles detected during computation of main repo mapping
```

Closes bazelbuild#20958.

PiperOrigin-RevId: 601183514
Change-Id: If321005b1f6918117d0f5b5763ae2bda4d8e34f1
github-merge-queue bot pushed a commit that referenced this issue Jan 24, 2024
…porter (#21013)

Work towards #20942

On the referenced bug, this prints:
```
ERROR: Circular definition of repositories generated by module extensions and/or .bzl files:
.-> repository mapping of @@
|   WORKSPACE file
|   //python:pip.bzl
|   //python/pip_install:requirements.bzl
|   //python:defs.bzl
|   //python:py_import.bzl
|   //python:py_info.bzl
|   @@_main~internal_deps~rules_python_internal//:rules_python_config.bzl
|   @@_main~internal_deps~rules_python_internal
|   extension 'internal_deps' defined in //python/private/bzlmod:internal_deps.bzl
`-- repository mapping of @@
ERROR: Error computing the main repository mapping: cycles detected during computation of main repo mapping
```

Closes #20958.

Commit
fab6414

PiperOrigin-RevId: 601183514
Change-Id: If321005b1f6918117d0f5b5763ae2bda4d8e34f1

Co-authored-by: Fabian Meumertzheim <[email protected]>
@iancha1992
Copy link
Member

A fix for this issue has been included in Bazel 7.0.2 RC1. Please test out the release candidate and report any issues as soon as possible. Thanks!

Wyverald added a commit that referenced this issue Jan 25, 2024
…extension eval

See code comment and linked issue for more information.

Fixes #20942.

Closes #20982.

PiperOrigin-RevId: 600856392
Change-Id: I5b8a0ed3a38e37ab51ffb49b19a59f2e161b9a33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Bzlmod Bzlmod-specific PRs, issues, and feature requests P1 I'll work on this now. (Assignee required) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants