bazelbuild / bazelbuild/bazel

Figure out how native.existing_rule() and native.existing_rules() should work with lazy expansion of symbolic macros

Open
#25,539 0 comments 0 reactions 1 assignee Claimed by @tetromino View on GitHub
P2 team-Loading-API type: feature request
Dominant language
Java
Stars
25.8k
Forks
4.6k
Avg merge
2d 20h
Merged PRs (30d)
72

Description

Sub-issue of https://github.com/bazelbuild/bazel/issues/23852

@brandjon FYI

It seems fairly clear what to do with native.existing_rules() in a finalizer. But what should Bazel do if lazy expansion of symbolic macros is enabled, and we encounter native.existing_rule() or native.existing_rules() while lazily evaluating the root package piece, i.e. not in a finalizer macro?

Some possibilites:

- Don't allow it, force users to migrate to finalizers.
- Pros: simplest
- Con: interrupts users and forces them to do things (such as expensive repo migrations). Users might decide to just not adopt lazy expansion.
- Workaround: add a flag to grandfather list of packages in which lazy macro expansion is disabled? Or pre-scan BUILD file transitive loads (we could be clever and follow only legacy macro loads) for any use of native.existing_rule/s()?
- Re-parse the BUILD file using a Package.Builder and use a full Package.
- Pros: simple-ish? (Maybe really not so simple, considering skyframe logic needed.)
- Con 1: inefficient - we throw away potentially a lot of work; native.existing_rule/s() is typically called in epilogue legacy macros nearly at the end of a BUILD file, and copying parse state from PackagePiece.ForBuildFile.Builder to Package.Builder would be technically complex.
- Con 2: ugly semantics for PackageProvider.getPackagePieceForBuildFile() (if we decide to add such a method) since PackageProvider cannot know in advance whether a particular package ID will have package pieces available for it or only full packages.
- Workaround: same as for first option
- Multiple PackagePiece.ForBuildFile instances, one per segment of BUILD file separated by calls to native.existing_rule/s(). Each piece depends both on the lhs piece *and* all the ForMacro pieces stemming from the lhs. The rightmost piece is the canonical one.
- Pros: temptingly elegant
- Con: in practice, for many affected packages, the resulting tangled dependency graph will have almost zero opportunity for change pruning but would cost more memory than a full eager Package.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.