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

Flag --incompatible_load_python_rules_from_bzl will break rules_python in Bazel 1.2.1 #268

Closed
bazel-flag-bot opened this issue Dec 19, 2019 · 5 comments
Assignees

Comments

@bazel-flag-bot
Copy link

Incompatible flag --incompatible_load_python_rules_from_bzl will break rules_python once Bazel 1.2.1 is released.

Please see the following CI builds for more information:

Questions? Please file an issue in https://github.com/bazelbuild/continuous-integration

Important: Please do NOT modify the issue title since that might break our tools.

@gertvdijk
Copy link

Interesting error, because this seems like Bazel wants us to create a circular dependency on itself. 😄

[...]/external/subpar/runtime/BUILD:3:1: in py_library rule @subpar//runtime:support: 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 bazelbuild/bazel#9006. You can temporarily bypass this error by setting --incompatible_load_python_rules_from_bzl=false.

@brandjon
Copy link
Member

That's odd, rules_python should be exempt from this flag (via the migration tag that only rules_python should use).

Ah, but the failure log is pretty clear. The bad targets are @subpar//runtime:support, //packaging:whltool, and //packaging:piptool. The last two are par_binary macros that call py_binary within subpar. So this appears to be entirely due to us grabbing subpar from the Bazel Federation, which pins to subpar 2.0.0, which does not include the fix for this flag.

The fix is in google/subpar@2917d27, which is not included in any subpar release. So our choices are:

  1. Cut a new subpar release just to include this fix. Then update the federation to point to it, and update our reference to the federation.

  2. Update the federation to point to a commit instead of a tagged release. Then update our reference to the federation.

  3. Update our own WORKSPACE to override subpar to the commit, without any change to subpar or the federation.

This flag won't be flipped anytime soon, and by the time we're getting ready to flip it there may be other reasons to cut a release. So I'm inclined to go with (3) at the moment, which is also the least work. We should still fix this so that people can test that their migration is successful.

Some background on the flag: Originally our plan for migrating the native Python rules to Starlark was to have everyone use the stubs, and then piece by piece add more real Starlark code to the stubs and use smaller and smaller bits of native functionality (exposed as internal symbols that only rules_python would use). Having everyone use the stubs first means that we get full coverage of the new Starlark logic as the migration proceeds, as opposed to doing all the work and finding out at the end there's some blockers.

But our current plan is to do this bit of redirection in @bazel_tools rather than rules_python, so that accessing native.py_binary would actually automatically give you a Starlark stub that's bundled with Bazel. This has the benefit that there's no version skew to worry about between Starlark-implemented logic and native logic.

We still want everyone to eventually use the stubs in rules_python. But under the new plan, there's no need to flip the flag before the rules are fully ported.

@brandjon
Copy link
Member

brandjon commented Apr 17, 2020

We should still fix this so that people can test that their migration is successful.

Actually this doesn't seem to impact users of rules_python, only its own development -- we can't do bazel test //... --incompatible_load_python_rules_from_bzl, but users who depend on @rules_python can.

I tried modifying the subpar definition in internal_deps.bzl, but there are other failures from our own dependencies, such as @rules_pkg. So this might require a federation fix.

Lowering priority since it doesn't block anyone else.

@brandjon brandjon added P3 and removed P1 labels Apr 17, 2020
@thundergolfer
Copy link

This is still a problem at commit 5d6c0a2

bazel test //... --incompatible_load_python_rules_from_bzl                                                                                                                           
Starting local Bazel server and connecting to it...
ERROR: /private/var/tmp/_bazel_jonathon/29bb5d8ab2ff174b67c235f3ce7964dd/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.

@fweikert
Copy link
Member

Bazel team has decided to not move forward with this flag: bazelbuild/bazel#9006 (comment)

As a result, no action is needed and this issue can be closed.

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

No branches or pull requests

6 participants