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

native.existing_rules in BUILD files #7496

Closed
laurentlb opened this issue Feb 21, 2019 · 5 comments
Closed

native.existing_rules in BUILD files #7496

laurentlb opened this issue Feb 21, 2019 · 5 comments
Assignees
Labels
P1 I'll work on this now. (Assignee required) type: process

Comments

@laurentlb
Copy link
Contributor

In general, when there's a global symbol in BUILD files (e.g. cc_library), we can use it from the bzl file with the native module (e.g. native.cc_library).

However:

  • The native module is (accidentally) visible in BUILD files. So native.cc_library also works in BUILD files. I think we shouldn't expose the native module anymore in BUILD files.
  • native.existing_rules and native.existing_rule are the only two functions that are not exposed as global symbols in BUILD files.

I see two possibilities:

  1. Just remove the native module from BUILD files. existing_rules will not be usable from BUILD files directly. This can be fine, as we don't want to encourage the use of this function. It was added as a workaround in macros.
  2. Remove the native module from BUILD files, expose existing_rule(s) as a global function in BUILD files. This is a bit more consistent.

Note that the existing_rule(s) functions are poorly defined, have design issues, can lead to performance and maintenance issues. So we don't really want to encourage their use.

(cc @c-parsons, @brandjon, @alandonovan)

@laurentlb laurentlb added P2 We'll consider working on this in future. (Assignee optional) type: process team-Starlark labels Feb 21, 2019
@alandonovan
Copy link
Contributor

I think 2 is better. The rules for which names appear in which environments are too complex.
I agree that anything we can do to tame the use of existing_rules is a good thing, but fundamentally, approach 1 isn't going to stop people getting their hands on existing_rules if they feel they need it.

@c-parsons
Copy link
Contributor

I agree with alan. I'm for (2).

@laurentlb
Copy link
Contributor Author

Filed #7513 to track the removal of native in BUILD files.

@laurentlb laurentlb added P1 I'll work on this now. (Assignee required) and removed P2 We'll consider working on this in future. (Assignee optional) labels Apr 5, 2019
@laurentlb
Copy link
Contributor Author

This is blocking #7513.

@alandonovan
Copy link
Contributor

BTW, it's not really documented anywhere but it seems more than a coincidence that all the functions in the native module access or update the abstract state of the package associated with the current Starlark thread, which must be a BUILD-loading thread. This serves as a good criterion for what belongs in the native module, if you're ever tempted to add more things to it.

(The package_name and repository_name functions could in principle be generalized to work for .bzl-loading threads too, since those two functions need access only to the label associated with the current thread, not an actual package, and all loading-phase threads have an associated label, used among other things to resolve load(":relative.bzl").)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 I'll work on this now. (Assignee required) type: process
Projects
None yet
Development

No branches or pull requests

4 participants