Skip to content

Add exports for core Python logic that's bundled with Bazel #202

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

Merged
merged 4 commits into from
Jul 27, 2019

Conversation

brandjon
Copy link
Contributor

@brandjon brandjon commented Jul 19, 2019

This series of commits adds exports to rules_python for Bazel-packaged Python rule logic. Most importantly, this includes the native rules and providers like py_binary and PyInfo, but it also includes Starlark-defined constructs that are distributed in @bazel_tools.

Work toward bazelbuild/bazel#8893. These exports constitute the new API we wish to migrate users to for that change.

Blocked on:

@brandjon brandjon requested review from iirina and oquenchil July 19, 2019 15:53
@brandjon brandjon self-assigned this Jul 19, 2019
Copy link
Contributor

@oquenchil oquenchil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@brandjon
Copy link
Contributor Author

@clintharrison: I got an email of your post but can't see a comment here, did you delete it or is github being weird?

In response to your point about warning when a deprecated API (python.bzl) is used: While (in my opinion) there's not necessarily a clear consensus about whether to use or proscribe print() for deprecation warnings, I personally try to avoid it. The philosophy is to instead make warnings into failures. This may block a user from upgrading to a new version of the repo until they migrate, but that's more acceptable for repo rules than it is for Bazel itself (since users have more flexibility about not upgrading).

In addition, we can sometimes make it a soft failure that can be bypassed during a transition period (a la Bazel's --incompatible_* flags). In this case it might be done by having the macro inject an extra dependency on a target that fails unless a certain flag is passed on the command line (implemented using Starlark-based configurations / build settings).

@clintharrison
Copy link

clintharrison commented Jul 25, 2019

@clintharrison: I got an email of your post but can't see a comment here, did you delete it or is github being weird?

In response to your point about warning when a deprecated API (python.bzl) is used: While (in my opinion) there's not necessarily a clear consensus about whether to use or proscribe print() for deprecation warnings, I personally try to avoid it. The philosophy is to instead make warnings into failures. This may block a user from upgrading to a new version of the repo until they migrate, but that's more acceptable for repo rules than it is for Bazel itself (since users have more flexibility about not upgrading).

In addition, we can sometimes make it a soft failure that can be bypassed during a transition period (a la Bazel's --incompatible_* flags). In this case it might be done by having the macro inject an extra dependency on a target that fails unless a certain flag is passed on the command line (implemented using Starlark-based configurations / build settings).

Sorry, I intended on reposting it as a full comment, rather than an inline one, but you beat me to the reply :)

Thanks for explaining the reasoning there -- I think that approach makes a lot of sense!

brandjon added 4 commits July 26, 2019 23:21
The "core" Python rules are the rules that traditionally have been bundled with
Bazel. This includes native rules like `py_binary`, and Starlark-defined rules
under `@bazel_tools` like `py_runtime_pair`. These should all live in
or around `@rules_python//python:defs.bzl`.

Currently we re-export the native rules here, with a magic tag to allow them to
survive the flag flip for `--incompatible_load_python_rules_from_bzl`. When
native rules are ported to Starlark their definitions will live here.
This adds export definitions for built-in symbols like `PyInfo` and
`@bazel_tools`-defined symbols like py_runtime_pair.
This vendors in the @bazel_tools//tools/python/runfiles target as
//python/runfiles. See comment in the BUILD file for why we couldn't re-export
the bundled implementation.
@brandjon brandjon merged commit 522222a into bazel-contrib:master Jul 27, 2019
@brandjon brandjon deleted the out-of-bazel branch July 27, 2019 03:35
fweikert pushed a commit to fweikert/rules_python that referenced this pull request Aug 7, 2019
…ntrib#202)

* Introduce defs.bzl as the official home of the core Python rules

The "core" Python rules are the rules that traditionally have been bundled with
Bazel. This includes native rules like `py_binary`, and Starlark-defined rules
under `@bazel_tools` like `py_runtime_pair`. These should all live in
or around `@rules_python//python:defs.bzl`.

Currently we re-export the native rules here, with a magic tag to allow them to
survive the flag flip for `--incompatible_load_python_rules_from_bzl`. When
native rules are ported to Starlark their definitions will live here.

* Add re-exports for Starlark-defined symbols

This adds export definitions for built-in symbols like `PyInfo` and
`@bazel_tools`-defined symbols like py_runtime_pair.

* Vendor in runfiles library

This vendors in the @bazel_tools//tools/python/runfiles target as
//python/runfiles. See comment in the BUILD file for why we couldn't re-export
the bundled implementation.

* Fix README to prefer defs.bzl over python.bzl
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants