-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Adding Starlark dependencies to the package //external #14630
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is certainly much cleaner than the other approach, I don't see any real issues. Is there enough information in it for your purposes, though?
src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java
Outdated
Show resolved
Hide resolved
Thanks for asking. Yes, the query result of this approach is a superset of the query result of the other approach. I am trying to use |
This is an alternative approach to fix bazelbuild#14280. It adds transitive closure of Starlark dependencies to `//external` package when loading `WORKSPACE` file, so it can be processed in the same way as `BUILD` files during query execution. Comparing to the approach taken in bazelbuild#14497, this approach is less intrusive, but not able to distinguish the extension files needed by `//external:foo` and `//external:bar`, meaning `buildfiles(//external:foo)` returns the same result as `buildfiles(//external:*)`. However, this behavior is consistent with other packages. For example, `buildfiles(//foo:bar)` has the same result as `buildfiles(//foo:*)`. Closes bazelbuild#14630. PiperOrigin-RevId: 424092916 (cherry picked from commit a6c3f23)
This is an alternative approach to fix bazelbuild#14280. It adds transitive closure of Starlark dependencies to `//external` package when loading `WORKSPACE` file, so it can be processed in the same way as `BUILD` files during query execution. Comparing to the approach taken in bazelbuild#14497, this approach is less intrusive, but not able to distinguish the extension files needed by `//external:foo` and `//external:bar`, meaning `buildfiles(//external:foo)` returns the same result as `buildfiles(//external:*)`. However, this behavior is consistent with other packages. For example, `buildfiles(//foo:bar)` has the same result as `buildfiles(//foo:*)`. Closes bazelbuild#14630. PiperOrigin-RevId: 424092916 (cherry picked from commit a6c3f23)
* Adding Starlark dependencies to the package //external This is an alternative approach to fix #14280. It adds transitive closure of Starlark dependencies to `//external` package when loading `WORKSPACE` file, so it can be processed in the same way as `BUILD` files during query execution. Comparing to the approach taken in #14497, this approach is less intrusive, but not able to distinguish the extension files needed by `//external:foo` and `//external:bar`, meaning `buildfiles(//external:foo)` returns the same result as `buildfiles(//external:*)`. However, this behavior is consistent with other packages. For example, `buildfiles(//foo:bar)` has the same result as `buildfiles(//foo:*)`. Closes #14630. PiperOrigin-RevId: 424092916 (cherry picked from commit a6c3f23) * Avoid bazel_module_test timeout on macos https://buildkite.com/bazel/bazel-bazel/builds/17785#9a1fb564-c6f9-4e69-ac8f-87c422a002b0 By setting the test size to "large". RELNOTES:None PiperOrigin-RevId: 409114345 (cherry picked from commit 1d99391) Co-authored-by: Zhongpeng Lin <[email protected]> Co-authored-by: pcloudy <[email protected]>
This is an alternative approach to fix #14280. It adds transitive closure of Starlark dependencies to
//external
package when loadingWORKSPACE
file, so it can be processed in the same way asBUILD
files during query execution. Comparing to the approach taken in #14497, this approach is less intrusive, but not able to distinguish the extension files needed by//external:foo
and//external:bar
, meaningbuildfiles(//external:foo)
returns the same result asbuildfiles(//external:*)
. However, this behavior is consistent with other packages. For example,buildfiles(//foo:bar)
has the same result asbuildfiles(//foo:*)
.