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

Make Bazel itself depend on @rules_python #9029

Closed
brandjon opened this issue Jul 31, 2019 · 6 comments
Closed

Make Bazel itself depend on @rules_python #9029

brandjon opened this issue Jul 31, 2019 · 6 comments
Assignees
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Rules-Python Native rules for Python type: process

Comments

@brandjon
Copy link
Member

brandjon commented Jul 31, 2019

As part of the work for #9006, we're migrating Bazel's own use of native Python rules to load these rules from a bzl file. Ideally, the load statements should reference the macros in @rules_python//python:defs.bzl just like user code should. However, this isn't possible for the @bazel_tools repository, which is not allowed to have any external dependencies. Therefore, we fake it by creating //tools/python:private/defs.bzl, which implements the same magic @rules_python does, for use by Bazel only.

Of course, there are other uses of Python rules in Bazel's source tree besides those that appear in @bazel_tools. Currently some of these also reference the same //tools/python:private/defs.bzl file, while others reference a stub @rules_python repository (defined in Bazel's own WORKSPACE) that only contains a stub version of @rules_python//python:defs.bzl; see #9019.

This issue tracks potentially replacing the stub @rules_python with the real thing, and avoiding non-@bazel_tools uses of the //tools/python:private/defs.bzl path.

Counterarguments for doing the former: The stub shouldn't be hard to keep in sync with the real rules_python, and using a stub avoids extra build overhead (and finagling with the WORKSPACE). Counterarguments for doing the latter: It's not particularly harmful to refer directly to the workspace-relative path, and it allows us to easily have a path that works both in @io_bazel and @bazel_tools (in the case of the six dependency).

@brandjon brandjon added P3 We're not considering working on this, but happy to review a PR. (No assignee) type: process team-Rules-Python Native rules for Python labels Jul 31, 2019
brandjon added a commit to brandjon/bazel that referenced this issue Aug 1, 2019
This replaces all direct uses of the native Python rules underneath the
third_party/ directory with load()s of the internal wrapper macros. These
macros are needed for compatibility with
`--incompatible_load_python_rules_from_bzl`.

Some of the uses are replaced by the file under the tools/ dir, which is
already used elsewhere in Bazel. A few uses have to use a new @rules_python
repo (see also bazelbuild#9029).

Work toward bazelbuild#9006.

RELNOTES: None
brandjon added a commit to brandjon/bazel that referenced this issue Aug 1, 2019
This replaces all direct uses of the native Python rules underneath the
third_party/ directory with load()s of the internal wrapper macros. These
macros are needed for compatibility with
`--incompatible_load_python_rules_from_bzl`.

Some of the uses are replaced by the file under the tools/ dir, which is
already used elsewhere in Bazel. A few uses have to use a new @rules_python
repo (see also bazelbuild#9029).

Work toward bazelbuild#9006.

RELNOTES: None
brandjon added a commit to brandjon/bazel that referenced this issue Aug 1, 2019
This replaces all direct uses of the native Python rules underneath the
third_party/ directory with load()s of the internal wrapper macros. These
macros are needed for compatibility with
`--incompatible_load_python_rules_from_bzl`.

Some of the uses are replaced by the file under the tools/ dir, which is
already used elsewhere in Bazel. A few uses have to use a new @rules_python
repo (see also bazelbuild#9029).

Work toward bazelbuild#9006.

RELNOTES: None
brandjon added a commit that referenced this issue Aug 1, 2019
This replaces all direct uses of the native Python rules underneath the
third_party/ directory with load()s of the internal wrapper macros. These
macros are needed for compatibility with
`--incompatible_load_python_rules_from_bzl`.

Some of the uses are replaced by the file under the tools/ dir, which is
already used elsewhere in Bazel. A few uses have to use a new @rules_python
repo (see also #9029).

Work toward #9006.

RELNOTES: None
brandjon added a commit that referenced this issue Aug 1, 2019
This replaces all direct uses of the native Python rules underneath the
third_party/ directory with load()s of the internal wrapper macros. These
macros are needed for compatibility with
`--incompatible_load_python_rules_from_bzl`.

Some of the uses are replaced by the file under the tools/ dir, which is
already used elsewhere in Bazel. A few uses have to use a new @rules_python
repo (see also #9029).

Work toward #9006.

RELNOTES: None
brandjon added a commit to brandjon/bazel that referenced this issue Aug 1, 2019
This replaces all direct uses of the native Python rules underneath the
third_party/ directory with load()s of the internal wrapper macros. These
macros are needed for compatibility with
`--incompatible_load_python_rules_from_bzl`.

Some of the uses are replaced by the file under the tools/ dir, which is
already used elsewhere in Bazel. A few uses have to use a new @rules_python
repo (see also bazelbuild#9029).

Work toward bazelbuild#9006.

RELNOTES: None
brandjon added a commit to brandjon/bazel that referenced this issue Aug 1, 2019
This replaces all direct uses of the native Python rules underneath the
third_party/ directory with load()s of the internal wrapper macros. These
macros are needed for compatibility with
`--incompatible_load_python_rules_from_bzl`.

Some of the uses are replaced by the file under the tools/ dir, which is
already used elsewhere in Bazel. A few uses have to use a new @rules_python
repo (see also bazelbuild#9029).

Work toward bazelbuild#9006.

RELNOTES: None
bazel-io pushed a commit that referenced this issue Aug 1, 2019
This replaces all direct uses of the native Python rules underneath the
third_party/ directory with load()s of the internal wrapper macros. These
macros are needed for compatibility with
`--incompatible_load_python_rules_from_bzl`.

Some of the uses are replaced by the file under the tools/ dir, which is
already used elsewhere in Bazel. A few uses have to use a new @rules_python
repo (see also #9029).

Work toward #9006.

Closes #9019.

RELNOTES: None
@aiuto aiuto added P2 We'll consider working on this in future. (Assignee optional) and removed P3 We're not considering working on this, but happy to review a PR. (No assignee) labels Apr 22, 2020
@aiuto
Copy link
Contributor

aiuto commented Apr 22, 2020

Bumping this up. This is interfering with tests that depend on rules that, in turn, depend on rules_python behind a maybe(). I'm seeing errors like:

ERROR: /var/lib/buildkite-agent/.cache/bazel/_bazel_buildkite-agent/.../external/rules_pkg/BUILD:15:11: in py_library rule @rules_pkg//:archive: \
  Direct access to the native Python rules is deprecated. Please load py_library from the rules_python repository. \
  See http://github.com/bazelbuild/rules_python and https://github.com/bazelbuild/bazel/issues/9006. \
  You can temporarily bypass this error by setting --incompatible_load_python_rules_from_bzl=false.

@aiuto
Copy link
Contributor

aiuto commented Apr 22, 2020

But maybe not. Still investigating. Bazel overall may have just been using an old version of the rules_pkg and getting the built-in rules.

@brandjon
Copy link
Member Author

The error indicates that the offending target is underneath @rules_pkg. Whether that's the real rules_pkg, or some kind of vendored or mock copy packaged with Bazel, it should be possible to load the stubs from @rules_python//python:defs.bzl and use them.

@aiuto
Copy link
Contributor

aiuto commented Apr 22, 2020

Yes. rules_pkg was behind. I pulled this into #11191

At first, I just update rules_pkg, which has this as its repositories rule:

...
def rules_pkg_dependencies():
    ... 
    maybe(
        git_repository,
        name = "rules_python",
        remote = "https://github.com/bazelbuild/rules_python.git",
        commit = "4b84ad270387a7c439ebdccfd530e2339601ef27",  # (2019-08-02 or later)
    )

My first attempt was to just update rules _pkg. That fails in an unexpected way

//src/test/shell/bazel:bazel_determinism_test                            FAILED in 3 out of 3 in 11.6s

https://buildkite.com/bazel/google-bazel-presubmit/builds/33474#ff750ae9-44ae-4924-9ca4-7cba5da20462

The drilldown was not informative.
https://storage.googleapis.com/bazel-untrusted-buildkite-artifacts/b73d7dcc-11e5-46d3-99a8-57b8b5886fa1/src/test/shell/bazel/bazel_determinism_test/attempt_1.log

The current version of the PR also tries to use real rules_pkg. That fails horribly, because protobufs don't build. :-( I don't have the cycles to deal with that problem.

@abergmeier
Copy link
Contributor

abergmeier commented Apr 23, 2020

Proposed switch for python migration: #1120

@lberki lberki self-assigned this Nov 18, 2020
@sgowroji sgowroji added the stale Issues or PRs that are stale (no activity for 30 days) label Feb 15, 2023
@sgowroji
Copy link
Member

Hi there! We're doing a clean up of old issues and will be closing this one. Please reopen if you’d like to discuss anything further. We’ll respond as soon as we have the bandwidth/resources to do so.

@comius comius reopened this Feb 24, 2023
@sgowroji sgowroji removed the stale Issues or PRs that are stale (no activity for 30 days) label Feb 24, 2023
comius added a commit to comius/bazel that referenced this issue Feb 24, 2023
comius added a commit to comius/bazel that referenced this issue Mar 3, 2023
fweikert pushed a commit to fweikert/bazel that referenced this issue May 25, 2023
Initial motivation was to use py_proto_library from rules_python, but then a yak came along.

Fixes: bazelbuild#9029

Closes bazelbuild#17545.

PiperOrigin-RevId: 513834100
Change-Id: I11a99381e1169a9fb7a7a3eaa733ddd348ebac2b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Rules-Python Native rules for Python type: process
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants