Skip to content

Commit

Permalink
[7.1.0] Fix bazel fetch by replacing query with cquery for underlyi…
Browse files Browse the repository at this point in the history
…ng implementation (#21567)

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

Backported commits:

-
70f7c80
-
de9d1f5
-
926b2c4
-
81e1333
-
dfbb3ce

---------

Co-authored-by: Googler <jhorvitz@google.com>
Co-authored-by: Googler <shahan@google.com>
Co-authored-by: Googler <jcater@google.com>
Co-authored-by: Googler <noreply@google.com>
Co-authored-by: salma-samy <salmasamy@google.com>
  • Loading branch information
6 people authored Mar 5, 2024
1 parent 1d871ce commit bbd608c
Show file tree
Hide file tree
Showing 153 changed files with 1,351 additions and 605 deletions.
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/build/lib/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/profiler/memory:allocationtracker",
"//src/main/java/com/google/devtools/build/lib/query2",
"//src/main/java/com/google/devtools/build/lib/query2:aquery-utils",
"//src/main/java/com/google/devtools/build/lib/query2/common:cquery-node",
"//src/main/java/com/google/devtools/build/lib/query2/engine",
"//src/main/java/com/google/devtools/build/lib/query2/query/output",
"//src/main/java/com/google/devtools/build/lib/runtime/events",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
import com.google.devtools.build.lib.skyframe.serialization.ObjectCodec;
import com.google.devtools.build.lib.skyframe.serialization.SerializationContext;
import com.google.devtools.build.lib.skyframe.serialization.SerializationException;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.VisibleForSerialization;
import com.google.devtools.build.lib.skyframe.serialization.VisibleForSerialization;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.SerializationConstant;
import com.google.devtools.build.lib.starlarkbuildapi.FileApi;
import com.google.devtools.build.lib.util.FileType;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import com.google.devtools.build.lib.actions.Artifact.SourceArtifact;
import com.google.devtools.build.lib.cmdline.LabelConstants;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.skyframe.serialization.VisibleForSerialization;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.starlarkbuildapi.FileRootApi;
import com.google.devtools.build.lib.vfs.Path;
Expand Down Expand Up @@ -164,7 +165,7 @@ public static ArtifactRoot asDerivedRoot(
return INTERNER.intern(new ArtifactRoot(Root.fromPath(root), execPath, rootType));
}

@AutoCodec.VisibleForSerialization
@VisibleForSerialization
@AutoCodec.Instantiator
static ArtifactRoot createForSerialization(
Root rootForSerialization, PathFragment execPath, RootType rootType) {
Expand Down
3 changes: 3 additions & 0 deletions src/main/java/com/google/devtools/build/lib/actions/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/skyframe:sane_analysis_exception",
"//src/main/java/com/google/devtools/build/lib/skyframe:skyframe_aware_action",
"//src/main/java/com/google/devtools/build/lib/skyframe:tree_artifact_value",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization:visible-for-serialization",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec:serialization-constant",
"//src/main/java/com/google/devtools/build/lib/starlarkbuildapi",
Expand Down Expand Up @@ -319,6 +320,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/skyframe:detailed_exceptions",
"//src/main/java/com/google/devtools/build/lib/skyframe:sky_functions",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization:visible-for-serialization",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec:serialization-constant",
"//src/main/java/com/google/devtools/build/lib/starlarkbuildapi",
Expand Down Expand Up @@ -399,6 +401,7 @@ java_library(
":has_digest",
"//src/main/java/com/google/devtools/build/lib/concurrent",
"//src/main/java/com/google/devtools/build/lib/io:inconsistent_filesystem_exception",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization:visible-for-serialization",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec:serialization-constant",
"//src/main/java/com/google/devtools/build/lib/util",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
import com.google.devtools.build.lib.skyframe.serialization.VisibleForSerialization;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.RootedPath;
Expand Down Expand Up @@ -150,7 +151,7 @@ public static Key key(RootedPath rootedPath) {
}

/** Key type for FileValue. */
@AutoCodec.VisibleForSerialization
@VisibleForSerialization
@AutoCodec
public static class Key extends AbstractSkyKey<RootedPath> {
private static final SkyKeyInterner<Key> interner = SkyKey.newInterner();
Expand All @@ -159,7 +160,7 @@ private Key(RootedPath arg) {
super(arg);
}

@AutoCodec.VisibleForSerialization
@VisibleForSerialization
@AutoCodec.Instantiator
static Key create(RootedPath arg) {
return interner.intern(new Key(arg));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
import com.google.auto.value.extension.memoized.Memoized;
import com.google.common.collect.ImmutableSortedSet;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.skyframe.serialization.VisibleForSerialization;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.Instantiator;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.VisibleForSerialization;
import com.google.devtools.build.lib.util.Fingerprint;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
Expand Down
7 changes: 7 additions & 0 deletions src/main/java/com/google/devtools/build/lib/analysis/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/profiler",
"//src/main/java/com/google/devtools/build/lib/profiler:google-auto-profiler-utils",
"//src/main/java/com/google/devtools/build/lib/profiler/memory:current_rule_tracker",
"//src/main/java/com/google/devtools/build/lib/query2/common:cquery-node",
"//src/main/java/com/google/devtools/build/lib/shell",
"//src/main/java/com/google/devtools/build/lib/skyframe:aspect_key_creator",
"//src/main/java/com/google/devtools/build/lib/skyframe:build_info_collection_value",
Expand All @@ -416,6 +417,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/skyframe:starlark_builtins_value",
"//src/main/java/com/google/devtools/build/lib/skyframe:workspace_status_value",
"//src/main/java/com/google/devtools/build/lib/skyframe/config",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization:visible-for-serialization",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec:serialization-constant",
"//src/main/java/com/google/devtools/build/lib/starlarkbuildapi",
Expand Down Expand Up @@ -691,6 +693,7 @@ java_library(
":transitive_info_collection",
"//src/main/java/com/google/devtools/build/lib/actions:action_lookup_key",
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/query2/common:cquery-node",
"//src/main/java/com/google/devtools/build/lib/skyframe/config",
"//src/main/java/net/starlark/java/eval",
"//third_party:guava",
Expand Down Expand Up @@ -1225,6 +1228,7 @@ java_library(
":transitive_info_provider_map",
"//src/main/java/com/google/devtools/build/lib/collect",
"//src/main/java/com/google/devtools/build/lib/packages",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization:visible-for-serialization",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
"//third_party:guava",
"//third_party:jsr305",
Expand Down Expand Up @@ -1380,6 +1384,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/collect/nestedset",
"//src/main/java/com/google/devtools/build/lib/concurrent",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization:visible-for-serialization",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec:serialization-constant",
"//src/main/java/com/google/devtools/build/lib/util",
Expand Down Expand Up @@ -2491,12 +2496,14 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/collect/nestedset",
"//src/main/java/com/google/devtools/build/lib/concurrent",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization:visible-for-serialization",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
"//src/main/java/com/google/devtools/build/lib/starlarkbuildapi",
"//src/main/java/com/google/devtools/build/lib/util",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
"//src/main/java/net/starlark/java/eval",
"//src/main/java/net/starlark/java/syntax",
"//third_party:error_prone_annotations",
"//third_party:guava",
"//third_party:jsr305",
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
import com.google.devtools.build.lib.packages.TestSize;
import com.google.devtools.build.lib.packages.Type;
import com.google.devtools.build.lib.packages.Type.ConversionException;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.skyframe.serialization.VisibleForSerialization;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.SerializationConstant;
import com.google.devtools.build.lib.util.FileTypeSet;
import javax.annotation.Nullable;
Expand All @@ -63,7 +63,7 @@ public class BaseRuleClasses {

private BaseRuleClasses() {}

@SerializationConstant @AutoCodec.VisibleForSerialization
@SerializationConstant @VisibleForSerialization
static final Attribute.ComputedDefault testonlyDefault =
new Attribute.ComputedDefault() {
@Override
Expand All @@ -77,7 +77,7 @@ public boolean resolvableWithRawAttributes() {
}
};

@SerializationConstant @AutoCodec.VisibleForSerialization
@SerializationConstant @VisibleForSerialization
static final Attribute.ComputedDefault deprecationDefault =
new Attribute.ComputedDefault() {
@Override
Expand All @@ -91,7 +91,7 @@ public boolean resolvableWithRawAttributes() {
}
};

@SerializationConstant @AutoCodec.VisibleForSerialization
@SerializationConstant @VisibleForSerialization
public static final Attribute.ComputedDefault TIMEOUT_DEFAULT =
new Attribute.ComputedDefault() {
@Override
Expand All @@ -112,7 +112,7 @@ public boolean resolvableWithRawAttributes() {
}
};

@SerializationConstant @AutoCodec.VisibleForSerialization
@SerializationConstant @VisibleForSerialization
public static final Attribute.ComputedDefault packageMetadataDefault =
new Attribute.ComputedDefault() {
@Override
Expand All @@ -135,15 +135,15 @@ public boolean resolvableWithRawAttributes() {
* they only run on the target configuration and should not operate on action_listeners and
* extra_actions themselves (to avoid cycles).
*/
@SerializationConstant @AutoCodec.VisibleForSerialization @VisibleForTesting
@SerializationConstant @VisibleForSerialization @VisibleForTesting
static final LabelListLateBoundDefault<?> ACTION_LISTENER =
LabelListLateBoundDefault.fromTargetConfiguration(
BuildConfigurationValue.class,
(rule, attributes, configuration) -> configuration.getActionListeners());

public static final String DEFAULT_COVERAGE_SUPPORT_VALUE = "//tools/test:coverage_support";

@SerializationConstant @AutoCodec.VisibleForSerialization
@SerializationConstant @VisibleForSerialization
static final Resolver<TestConfiguration, Label> COVERAGE_SUPPORT_CONFIGURATION_RESOLVER =
(rule, attributes, configuration) -> configuration.getCoverageSupport();

Expand All @@ -156,7 +156,7 @@ public static LabelLateBoundDefault<TestConfiguration> coverageSupportAttribute(
public static final String DEFAULT_COVERAGE_REPORT_GENERATOR_VALUE =
"//tools/test:coverage_report_generator";

@SerializationConstant @AutoCodec.VisibleForSerialization
@SerializationConstant @VisibleForSerialization
static final Resolver<CoverageConfiguration, Label>
COVERAGE_REPORT_GENERATOR_CONFIGURATION_RESOLVER =
(rule, attributes, configuration) -> configuration.reportGenerator();
Expand All @@ -174,13 +174,13 @@ public static LabelLateBoundDefault<CoverageConfiguration> getCoverageOutputGene
CoverageConfiguration.class, null, COVERAGE_OUTPUT_GENERATOR_RESOLVER);
}

@SerializationConstant @AutoCodec.VisibleForSerialization
@SerializationConstant @VisibleForSerialization
static final Resolver<CoverageConfiguration, Label> COVERAGE_OUTPUT_GENERATOR_RESOLVER =
(rule, attributes, configuration) -> configuration.outputGenerator();

// TODO(b/65746853): provide a way to do this without passing the entire configuration
/** Implementation for the :run_under attribute. */
@SerializationConstant @AutoCodec.VisibleForSerialization
@SerializationConstant @VisibleForSerialization
public static final LabelLateBoundDefault<?> RUN_UNDER =
LabelLateBoundDefault.fromTargetConfiguration(
BuildConfigurationValue.class,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import com.google.devtools.build.lib.actions.ActionLookupKey;
import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.query2.common.CqueryNode;
import com.google.devtools.build.lib.skyframe.config.BuildConfigurationKey;
import javax.annotation.Nullable;
import net.starlark.java.eval.Dict;
Expand All @@ -35,7 +36,7 @@
* their direct dependencies, only the corresponding {@link TransitiveInfoCollection}s. Also, {@link
* ConfiguredTarget} objects should not be accessible from the action graph.
*/
public interface ConfiguredTarget extends TransitiveInfoCollection, Structure {
public interface ConfiguredTarget extends TransitiveInfoCollection, Structure, CqueryNode {

/** All <code>ConfiguredTarget</code>s have a "label" field. */
String LABEL_FIELD = "label";
Expand All @@ -44,13 +45,15 @@ public interface ConfiguredTarget extends TransitiveInfoCollection, Structure {
String FILES_FIELD = "files";

/** Returns a key that may be used to lookup this {@link ConfiguredTarget}. */
@Override
ActionLookupKey getLookupKey();

@Override
default Label getLabel() {
return getLookupKey().getLabel();
}

@Override
@Nullable
default String getConfigurationChecksum() {
return getConfigurationKey() == null ? null : getConfigurationKey().getOptions().checksum();
Expand All @@ -65,8 +68,9 @@ default String getConfigurationChecksum() {
* com.google.devtools.build.lib.analysis.configuredtargets.PackageGroupConfiguredTarget} for
* which it is always <b>null</b>.
*
* <p>If this changes, {@link AspectResolver#aspecMatchesConfiguredTarget} should be updated.
* <p>If this changes, {@link AspectResolver#aspectMatchesConfiguredTarget} should be updated.
*/
@Override
@Nullable
default BuildConfigurationKey getConfigurationKey() {
return getLookupKey().getConfigurationKey();
Expand All @@ -89,6 +93,7 @@ default BuildConfigurationKey getConfigurationKey() {
* If the configured target is an alias, return the actual target, otherwise return the current
* target. This follows alias chains.
*/
@Override
default ConfiguredTarget getActual() {
return this;
}
Expand All @@ -98,6 +103,7 @@ default ConfiguredTarget getActual() {
* label. This is not the same as {@code getActual().getLabel()}, because it does not follow alias
* chains.
*/
@Override
default Label getOriginalLabel() {
return getLabel();
}
Expand All @@ -106,10 +112,12 @@ default Label getOriginalLabel() {
* The configuration conditions that trigger this configured target's configurable attributes. For
* targets that do not support configurable attributes, this will be an empty map.
*/
@Override
default ImmutableMap<Label, ConfigMatchingProvider> getConfigConditions() {
return ImmutableMap.of();
}

@Override
default boolean isRuleConfiguredTarget() {
return false;
}
Expand All @@ -119,6 +127,7 @@ default boolean isRuleConfiguredTarget() {
*
* <p>Unwrapping is recursive if there are multiple layers.
*/
@Override
default ConfiguredTarget unwrapIfMerged() {
return this;
}
Expand All @@ -128,6 +137,7 @@ default ConfiguredTarget unwrapIfMerged() {
*
* @return a map of provider names to their values, or null if there are no providers
*/
@Override
@Nullable
default Dict<String, Object> getProvidersDictForQuery() {
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
import com.google.devtools.build.lib.server.FailureDetails.Execution;
import com.google.devtools.build.lib.server.FailureDetails.Execution.Code;
import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.skyframe.serialization.VisibleForSerialization;
import com.google.devtools.build.lib.util.DetailedExitCode;
import com.google.devtools.build.lib.util.Fingerprint;
import com.google.protobuf.Extension;
Expand All @@ -41,11 +41,10 @@
* about rules to extra_actions.
*/
public class PseudoAction<InfoType extends MessageLite> extends AbstractAction {
@AutoCodec.VisibleForSerialization protected final UUID uuid;
@VisibleForSerialization protected final UUID uuid;
private final String mnemonic;

@AutoCodec.VisibleForSerialization
protected final Extension<ExtraActionInfo, InfoType> infoExtension;
@VisibleForSerialization protected final Extension<ExtraActionInfo, InfoType> infoExtension;

private final InfoType info;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
import com.google.devtools.build.lib.events.EventKind;
import com.google.devtools.build.lib.packages.BuildType;
import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.skyframe.serialization.VisibleForSerialization;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.SerializationConstant;
import com.google.devtools.build.lib.starlarkbuildapi.RunfilesApi;
import com.google.devtools.build.lib.util.Fingerprint;
Expand Down Expand Up @@ -88,7 +88,7 @@ public void fingerprint(Fingerprint fp) {
}
}

@SerializationConstant @AutoCodec.VisibleForSerialization
@SerializationConstant @VisibleForSerialization
static final EmptyFilesSupplier DUMMY_EMPTY_FILES_SUPPLIER = new DummyEmptyFilesSupplier();

// It is important to declare this *after* the DUMMY_SYMLINK_EXPANDER to avoid NPEs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.profiler.AutoProfiler;
import com.google.devtools.build.lib.profiler.GoogleAutoProfilerUtils;
import com.google.devtools.build.lib.query2.common.CqueryNode;
import java.time.Duration;
import java.util.ArrayList;
import java.util.HashMap;
Expand Down Expand Up @@ -243,7 +244,7 @@ static ArtifactsToBuild getAllArtifactsToBuild(
*
* <p>Always returns false for hidden rules and source file targets.
*/
public static boolean shouldConsiderForDisplay(ConfiguredTarget configuredTarget) {
public static boolean shouldConsiderForDisplay(CqueryNode configuredTarget) {
// TODO(bazel-team): this is quite ugly. Add a marker provider for this check.
if (configuredTarget instanceof InputFileConfiguredTarget) {
// Suppress display of source files (because we do no work to build them).
Expand Down
Loading

0 comments on commit bbd608c

Please sign in to comment.