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

[7.1.0] Fix fetch target (fixes #13847) #21527

Closed
wants to merge 7 commits into from
Closed

Conversation

SalmaSamy
Copy link
Contributor

@SalmaSamy SalmaSamy commented Feb 29, 2024

Context:

  • Traditional query relies on the initial loading phase of the build, this lacks the context of build configurations (flags, select() logic), leading to potentially inaccurate or over-inclusive dependency listings.

  • cquery executes after the analysis phase, where Bazel has resolved configurations and determined how options influence target definitions. This allows cquery to provide the dependencies truly needed for a build under the current settings.

Considering these differences, I'm updating fetch target logic to rely on cquery instead. This ensures that all necessary repositories are fetched for an offline build while avoiding potential over-fetching

PiperOrigin-RevId: 611455579
Change-Id: I2a954476c06182fd9eb78ad86def7bd72f04074a

Cherry-picking commit & commit was needed.

Context:
- Traditional query relies on the initial loading phase of the build, this lacks the context of build configurations (flags, select() logic), leading to potentially inaccurate or over-inclusive dependency listings.

- cquery executes after the analysis phase, where Bazel has resolved configurations and determined how options influence target definitions. This allows cquery to provide the dependencies truly needed for a build under the current settings.

Considering these differences, I'm updating fetch target logic to rely on cquery instead. This ensures that all necessary repositories are fetched for an offline build while avoiding potential over-fetching

PiperOrigin-RevId: 611455579
Change-Id: I2a954476c06182fd9eb78ad86def7bd72f04074a

# Conflicts:
#	src/main/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQueryEnvironment.java
@SalmaSamy SalmaSamy requested a review from a team as a code owner February 29, 2024 15:02
@github-actions github-actions bot added awaiting-review PR is awaiting review from an assigned reviewer team-Configurability platforms, toolchains, cquery, select(), config transitions team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. labels Feb 29, 2024
Googler and others added 5 commits February 29, 2024 16:19
This CL adds aspect nodes as explicit targets in cquery deps by making
ConfiguredTargetQueryEnvironment generic over a new interface,
CqueryNode, rather than ConfiguredTarget.

CqueryNode is implemented by AspectKey as well as ConfiguredTarget so
that both can be traversed in a deps query.

PiperOrigin-RevId: 595509373
Change-Id: I8a637cd3ed640907d2b1501bcd2b4a4507d183e7

# Conflicts:
#	src/main/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQueryEnvironment.java
#	src/main/java/com/google/devtools/build/lib/query2/cquery/ProtoOutputFormatterCallback.java
#	src/main/java/com/google/devtools/build/lib/rules/BUILD
This annotation is orthogonal to AutoCodec and creates a circular dependency in
a later change.

PiperOrigin-RevId: 579867965
Change-Id: Iff7ab004d7635c32edb9de38231b2d4c93469c6f

# Conflicts:
#	src/main/java/com/google/devtools/build/lib/rules/BUILD
#	src/main/java/com/google/devtools/build/lib/skyframe/TestCompletionValue.java
@iancha1992 iancha1992 added this to the 7.1.0 release blockers milestone Feb 29, 2024
@iancha1992
Copy link
Member

@SalmaSamy Looks like the tests are failing. Could you please take a look?

cc: @bazelbuild/triage

@meteorcloudy
Copy link
Member

Salma is OOO, I'll take a look today

@meteorcloudy
Copy link
Member

Closing due to #21567

auto-merge was automatically disabled March 5, 2024 10:51

Pull request was closed

@github-actions github-actions bot removed the awaiting-review PR is awaiting review from an assigned reviewer label Mar 5, 2024
@iancha1992 iancha1992 deleted the add-fetch-fix branch September 16, 2024 03:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Configurability platforms, toolchains, cquery, select(), config transitions team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants