From 720dc5fd640de692db129777c7c7c32924627c43 Mon Sep 17 00:00:00 2001 From: Googler Date: Fri, 16 Sep 2022 03:43:51 -0700 Subject: [PATCH] Plumb transitive packages to RuleContext To fix https://github.com/bazelbuild/bazel/issues/16124, we need to collect all the repos/repo mappings relevant for a runfiles directory and write that into a manifest. One way to do that is to utilize the existing `transitivePackagesForPackageRootResolution` thing that we collect in most cases. This CL plumbs this information through to RuleContext (whereafter it can be used by RunfilesSupport to write a manifest), without any logic changes. The field `transitivePackagesForPackageRootResolution` is renamed to simply `transitivePackages` since it's now used for multiple purposes. Closes https://github.com/bazelbuild/bazel/pull/16278. PiperOrigin-RevId: 474778287 Change-Id: I7158c8adacc0b1db7f7bdda3b621d49c1fdb491c --- .../build/lib/analysis/AspectResolver.java | 3 +- .../build/lib/analysis/AspectValue.java | 12 +-- .../lib/analysis/ConfiguredObjectValue.java | 14 +++- .../lib/analysis/ConfiguredTargetFactory.java | 7 ++ .../build/lib/analysis/RuleContext.java | 22 +++++ .../IncompatibleTargetChecker.java | 15 ++-- .../build/lib/skyframe/AspectFunction.java | 76 +++++++----------- .../lib/skyframe/BuildDriverFunction.java | 11 +-- .../skyframe/ConfiguredTargetFunction.java | 80 ++++++++----------- .../NonRuleConfiguredTargetValue.java | 13 ++- .../skyframe/RuleConfiguredTargetValue.java | 15 ++-- .../build/lib/skyframe/SkyframeBuildView.java | 10 +-- .../build/lib/skyframe/SkyframeExecutor.java | 10 ++- ...onfiguredTargetTransitivePackagesTest.java | 22 ++--- 14 files changed, 151 insertions(+), 159 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AspectResolver.java b/src/main/java/com/google/devtools/build/lib/analysis/AspectResolver.java index 052918602b4c7f..b542fb7b7316ed 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/AspectResolver.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/AspectResolver.java @@ -100,8 +100,7 @@ public static OrderedSetMultimap resolveAspectDepe result.put(dep, aspectValue.getConfiguredAspect()); if (transitivePackages != null) { transitivePackages.addTransitive( - Preconditions.checkNotNull( - aspectValue.getTransitivePackagesForPackageRootResolution())); + Preconditions.checkNotNull(aspectValue.getTransitivePackages())); } } } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AspectValue.java b/src/main/java/com/google/devtools/build/lib/analysis/AspectValue.java index d9338474b9d237..258cf937dea9da 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/AspectValue.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/AspectValue.java @@ -35,20 +35,20 @@ public final class AspectValue extends BasicActionLookupValue implements RuleCon @Nullable private AspectKey key; @Nullable private ConfiguredAspect configuredAspect; // May be null either after clearing or because transitive packages are not tracked. - @Nullable private transient NestedSet transitivePackagesForPackageRootResolution; + @Nullable private transient NestedSet transitivePackages; public AspectValue( AspectKey key, Aspect aspect, Location location, ConfiguredAspect configuredAspect, - NestedSet transitivePackagesForPackageRootResolution) { + NestedSet transitivePackages) { super(configuredAspect.getActions()); this.key = key; this.aspect = Preconditions.checkNotNull(aspect, location); this.location = Preconditions.checkNotNull(location, aspect); this.configuredAspect = Preconditions.checkNotNull(configuredAspect, location); - this.transitivePackagesForPackageRootResolution = transitivePackagesForPackageRootResolution; + this.transitivePackages = transitivePackages; } public ConfiguredAspect getConfiguredAspect() { @@ -79,13 +79,13 @@ public void clear(boolean clearEverything) { key = null; configuredAspect = null; } - transitivePackagesForPackageRootResolution = null; + transitivePackages = null; } @Nullable @Override - public NestedSet getTransitivePackagesForPackageRootResolution() { - return transitivePackagesForPackageRootResolution; + public NestedSet getTransitivePackages() { + return transitivePackages; } @Override diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredObjectValue.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredObjectValue.java index b06000ad50beec..f32ec4c9ff2b7f 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredObjectValue.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredObjectValue.java @@ -26,14 +26,20 @@ public interface ConfiguredObjectValue extends NotComparableSkyValue { ProviderCollection getConfiguredObject(); /** - * Returns the set of packages transitively loaded by this value. Must only be used for - * constructing the package -> source root map needed for some builds. If the caller has not - * specified that this map needs to be constructed (via the constructor argument in {@link + * Returns the set of packages transitively loaded by this value. Must only be used for: + * + *
    + *
  • constructing the package -> source root map needed for some builds, OR + *
  • building the repo mapping manifest for runfiles + *
+ * + * If the caller has not specified that this map needs to be constructed (via the constructor + * argument in {@link * com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction#ConfiguredTargetFunction} or * {@link com.google.devtools.build.lib.skyframe.AspectFunction#AspectFunction}), calling this * will crash. */ - NestedSet getTransitivePackagesForPackageRootResolution(); + NestedSet getTransitivePackages(); /** * Clears data from this value. diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java index 31cd6108c2f318..2cfddc79043796 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java @@ -58,6 +58,7 @@ import com.google.devtools.build.lib.packages.EnvironmentGroup; import com.google.devtools.build.lib.packages.InputFile; import com.google.devtools.build.lib.packages.OutputFile; +import com.google.devtools.build.lib.packages.Package; import com.google.devtools.build.lib.packages.PackageGroup; import com.google.devtools.build.lib.packages.PackageGroupsRuleVisibility; import com.google.devtools.build.lib.packages.PackageSpecification; @@ -181,6 +182,7 @@ public ConfiguredTarget createConfiguredTarget( OrderedSetMultimap prerequisiteMap, ConfigConditions configConditions, @Nullable ToolchainCollection toolchainContexts, + @Nullable NestedSet transitivePackages, ExecGroupCollection.Builder execGroupCollectionBuilder) throws InterruptedException, ActionConflictException, InvalidExecGroupException, AnalysisFailurePropagationException { @@ -196,6 +198,7 @@ public ConfiguredTarget createConfiguredTarget( prerequisiteMap, configConditions, toolchainContexts, + transitivePackages, execGroupCollectionBuilder); } finally { CurrentRuleTracker.endConfiguredTarget(); @@ -290,6 +293,7 @@ private ConfiguredTarget createRule( OrderedSetMultimap prerequisiteMap, ConfigConditions configConditions, @Nullable ToolchainCollection toolchainContexts, + @Nullable NestedSet transitivePackages, ExecGroupCollection.Builder execGroupCollectionBuilder) throws InterruptedException, ActionConflictException, InvalidExecGroupException, AnalysisFailurePropagationException { @@ -316,6 +320,7 @@ private ConfiguredTarget createRule( ruleClassProvider.getFragmentRegistry().getUniversalFragments(), configConditions, prerequisiteMap.values())) + .setTransitivePackagesForRunfileRepoMappingManifest(transitivePackages) .build(); List> analysisFailures = @@ -506,6 +511,7 @@ public ConfiguredAspect createAspect( @Nullable ExecGroupCollection.Builder execGroupCollectionBuilder, BuildConfigurationValue aspectConfiguration, BuildConfigurationValue hostConfiguration, + @Nullable NestedSet transitivePackages, AspectKeyCreator.AspectKey aspectKey) throws InterruptedException, ActionConflictException, InvalidExecGroupException { RuleContext ruleContext = @@ -533,6 +539,7 @@ public ConfiguredAspect createAspect( ruleClassProvider.getFragmentRegistry().getUniversalFragments(), configConditions, Iterables.concat(prerequisiteMap.values(), ImmutableList.of(associatedTarget)))) + .setTransitivePackagesForRunfileRepoMappingManifest(transitivePackages) .build(); // If allowing analysis failures, targets should be created as normal as possible, and errors diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java index d7fa51447e11e9..959fc1530dec9b 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java @@ -76,6 +76,7 @@ import com.google.devtools.build.lib.packages.Info; import com.google.devtools.build.lib.packages.InputFile; import com.google.devtools.build.lib.packages.OutputFile; +import com.google.devtools.build.lib.packages.Package; import com.google.devtools.build.lib.packages.Package.ConfigSettingVisibilityPolicy; import com.google.devtools.build.lib.packages.PackageSpecification.PackageGroupContents; import com.google.devtools.build.lib.packages.RawAttributeMapper; @@ -165,6 +166,7 @@ void validate( @Nullable private final ToolchainCollection toolchainContexts; private final ExecGroupCollection execGroupCollection; @Nullable private final RequiredConfigFragmentsProvider requiredConfigFragments; + @Nullable private final NestedSet transitivePackagesForRunfileRepoMappingManifest; private final List makeVariableExpanders = new ArrayList<>(); /** Map of exec group names to ActionOwners. */ @@ -222,6 +224,8 @@ private RuleContext( this.toolchainContexts = builder.toolchainContexts; this.execGroupCollection = execGroupCollection; this.requiredConfigFragments = builder.requiredConfigFragments; + this.transitivePackagesForRunfileRepoMappingManifest = + builder.transitivePackagesForRunfileRepoMappingManifest; this.starlarkThread = createStarlarkThread(builder.mutability); // uses above state } @@ -1285,6 +1289,17 @@ public RequiredConfigFragmentsProvider getRequiredConfigFragments() { return merged == null ? requiredConfigFragments : merged.build(); } + /** + * Returns the set of transitive packages. This is only intended to be used to create the repo + * mapping manifest for the runfiles tree. Can be null if transitive packages are not tracked (see + * {@link + * com.google.devtools.build.lib.skyframe.SkyframeExecutor#shouldStoreTransitivePackagesInLoadingAndAnalysis}). + */ + @Nullable + public NestedSet getTransitivePackagesForRunfileRepoMappingManifest() { + return transitivePackagesForRunfileRepoMappingManifest; + } + private boolean isUserDefinedMakeVariable(String makeVariable) { // User-defined make values may be set either in "--define foo=bar" or in a vardef in the rule's // package. Both are equivalent for these purposes, since in both cases setting @@ -1651,6 +1666,7 @@ public static final class Builder implements RuleErrorConsumer { private ExecGroupCollection.Builder execGroupCollectionBuilder; private ImmutableMap rawExecProperties; @Nullable private RequiredConfigFragmentsProvider requiredConfigFragments; + @Nullable private NestedSet transitivePackagesForRunfileRepoMappingManifest; @VisibleForTesting public Builder( @@ -1877,6 +1893,12 @@ public Builder setRequiredConfigFragments( return this; } + @CanIgnoreReturnValue + public Builder setTransitivePackagesForRunfileRepoMappingManifest( + @Nullable NestedSet packages) { + this.transitivePackagesForRunfileRepoMappingManifest = packages; + return this; + } /** Determines and returns a map from attribute name to list of configured targets. */ private ImmutableSortedKeyListMultimap createTargetMap() { diff --git a/src/main/java/com/google/devtools/build/lib/analysis/constraints/IncompatibleTargetChecker.java b/src/main/java/com/google/devtools/build/lib/analysis/constraints/IncompatibleTargetChecker.java index 5f5a306575b8d3..d25ba108c287d3 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/constraints/IncompatibleTargetChecker.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/constraints/IncompatibleTargetChecker.java @@ -104,7 +104,7 @@ public static Optional createDirectlyIncompatibleTarg ConfigConditions configConditions, Environment env, @Nullable PlatformInfo platformInfo, - NestedSetBuilder transitivePackagesForPackageRootResolution) + NestedSetBuilder transitivePackages) throws InterruptedException { Target target = targetAndConfiguration.getTarget(); Rule rule = target.getAssociatedRule(); @@ -157,7 +157,7 @@ public static Optional createDirectlyIncompatibleTarg IncompatiblePlatformProvider.incompatibleDueToConstraints( platformInfo.label(), invalidConstraintValues), rule.getRuleClass(), - transitivePackagesForPackageRootResolution)); + transitivePackages)); } /** @@ -181,7 +181,7 @@ public static Optional createIndirectlyIncompatibleTa OrderedSetMultimap depValueMap, ConfigConditions configConditions, @Nullable PlatformInfo platformInfo, - NestedSetBuilder transitivePackagesForPackageRootResolution) { + NestedSetBuilder transitivePackages) { Target target = targetAndConfiguration.getTarget(); Rule rule = target.getAssociatedRule(); @@ -209,7 +209,7 @@ public static Optional createIndirectlyIncompatibleTa configConditions, IncompatiblePlatformProvider.incompatibleDueToTargets(platformLabel, incompatibleDeps), rule.getRuleClass(), - transitivePackagesForPackageRootResolution)); + transitivePackages)); } /** Creates an incompatible target. */ @@ -219,7 +219,7 @@ private static RuleConfiguredTargetValue createIncompatibleRuleConfiguredTarget( ConfigConditions configConditions, IncompatiblePlatformProvider incompatiblePlatformProvider, String ruleClassString, - NestedSetBuilder transitivePackagesForPackageRootResolution) { + NestedSetBuilder transitivePackages) { // Create dummy instances of the necessary data for a configured target. None of this data will // actually be used because actions associated with incompatible targets must not be evaluated. NestedSet filesToBuild = NestedSetBuilder.emptySet(Order.STABLE_ORDER); @@ -248,10 +248,7 @@ private static RuleConfiguredTargetValue createIncompatibleRuleConfiguredTarget( configConditions.asProviders(), ruleClassString); return new RuleConfiguredTargetValue( - configuredTarget, - transitivePackagesForPackageRootResolution == null - ? null - : transitivePackagesForPackageRootResolution.build()); + configuredTarget, transitivePackages == null ? null : transitivePackages.build()); } /** diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java index a0012e65c3f9d9..497ec9f71df683 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java @@ -120,24 +120,24 @@ final class AspectFunction implements SkyFunction { private final RuleClassProvider ruleClassProvider; /** * Indicates whether the set of packages transitively loaded for a given {@link AspectValue} will - * be needed for package root resolution later in the build. If not, they are not collected and - * stored. + * be needed later (see {@link + * com.google.devtools.build.lib.analysis.ConfiguredObjectValue#getTransitivePackages}). If not, + * they are not collected and stored. */ - private final boolean storeTransitivePackagesForPackageRootResolution; + private final boolean storeTransitivePackages; AspectFunction( BuildViewProvider buildViewProvider, RuleClassProvider ruleClassProvider, - boolean storeTransitivePackagesForPackageRootResolution) { + boolean storeTransitivePackages) { this.buildViewProvider = buildViewProvider; this.ruleClassProvider = ruleClassProvider; - this.storeTransitivePackagesForPackageRootResolution = - storeTransitivePackagesForPackageRootResolution; + this.storeTransitivePackages = storeTransitivePackages; } static class State implements SkyKeyComputeState { /** Null if AspectFuncton is not storing this information. */ - @Nullable NestedSetBuilder transitivePackagesForPackageRootResolution; + @Nullable NestedSetBuilder transitivePackages; NestedSetBuilder transitiveRootCauses = NestedSetBuilder.stableOrder(); @@ -145,9 +145,8 @@ static class State implements SkyKeyComputeState { ComputeDependenciesState computeDependenciesState = new ComputeDependenciesState(); - State(boolean storeTransitivePackagesForPackageRootResolution) { - this.transitivePackagesForPackageRootResolution = - storeTransitivePackagesForPackageRootResolution ? NestedSetBuilder.stableOrder() : null; + State(boolean storeTransitivePackages) { + this.transitivePackages = storeTransitivePackages ? NestedSetBuilder.stableOrder() : null; } } @@ -177,7 +176,7 @@ private InitialValues( public SkyValue compute(SkyKey skyKey, Environment env) throws AspectFunctionException, InterruptedException { AspectKey key = (AspectKey) skyKey.argument(); - State state = env.getState(() -> new State(storeTransitivePackagesForPackageRootResolution)); + State state = env.getState(() -> new State(storeTransitivePackages)); if (state.initialValues == null) { InitialValues initialValues = getInitialValues(key, env); @@ -201,9 +200,7 @@ public SkyValue compute(SkyKey skyKey, Environment env) aspect, target.getLocation(), ConfiguredAspect.forNonapplicableTarget(), - state.transitivePackagesForPackageRootResolution == null - ? null - : state.transitivePackagesForPackageRootResolution.build()); + state.transitivePackages == null ? null : state.transitivePackages.build()); } if (AliasProvider.isAlias(associatedTarget)) { @@ -244,8 +241,7 @@ public SkyValue compute(SkyKey skyKey, Environment env) aspect, target.getLocation(), ConfiguredAspect.forNonapplicableTarget(), - /*transitivePackagesForPackageRootResolution=*/ NestedSetBuilder.emptySet( - Order.STABLE_ORDER)); + /*transitivePackages=*/ NestedSetBuilder.emptySet(Order.STABLE_ORDER)); } } } @@ -308,7 +304,7 @@ public SkyValue compute(SkyKey skyKey, Environment env) ConfiguredTargetFunction.getConfigConditions( env, originalTargetAndConfiguration, - state.transitivePackagesForPackageRootResolution, + state.transitivePackages, unloadedToolchainContexts == null ? null : unloadedToolchainContexts.getTargetPlatform(), @@ -323,7 +319,7 @@ public SkyValue compute(SkyKey skyKey, Environment env) depValueMap = ConfiguredTargetFunction.computeDependencies( state.computeDependenciesState, - state.transitivePackagesForPackageRootResolution, + state.transitivePackages, state.transitiveRootCauses, env, resolver, @@ -384,7 +380,7 @@ public SkyValue compute(SkyKey skyKey, Environment env) toolchainContexts, execGroupCollectionBuilder, depValueMap, - state.transitivePackagesForPackageRootResolution); + state.transitivePackages); } catch (DependencyEvaluationException e) { // TODO(bazel-team): consolidate all env.getListener().handle() calls in this method, like in // ConfiguredTargetFunction. This encourages clear, consistent user messages (ideally without @@ -636,8 +632,8 @@ private AspectValue createAliasAspect( // the real configured target. Label aliasedLabel = aliasChain.size() > 1 ? aliasChain.get(1) : configuredTarget.getLabel(); - NestedSetBuilder transitivePackagesForPackageRootResolution = - storeTransitivePackagesForPackageRootResolution ? NestedSetBuilder.stableOrder() : null; + NestedSetBuilder transitivePackages = + storeTransitivePackages ? NestedSetBuilder.stableOrder() : null; NestedSetBuilder transitiveRootCauses = NestedSetBuilder.stableOrder(); // Compute the Dependency from originalTarget to aliasedLabel @@ -661,7 +657,7 @@ private AspectValue createAliasAspect( ConfiguredTargetFunction.getConfigConditions( env, originalTargetAndAspectConfiguration, - transitivePackagesForPackageRootResolution, + transitivePackages, unloadedToolchainContexts == null ? null : unloadedToolchainContexts.getTargetPlatform(), @@ -726,12 +722,7 @@ private AspectValue createAliasAspect( // Now that we have a Dependency, we can compute the aliased key and depend on it AspectKey actualKey = buildAliasAspectKey(originalKey, aliasedLabel, dep); return createAliasAspect( - env, - originalTarget.getTarget(), - originalKey, - aspect, - actualKey, - transitivePackagesForPackageRootResolution); + env, originalTarget.getTarget(), originalKey, aspect, actualKey, transitivePackages); } @Nullable @@ -741,7 +732,7 @@ private static AspectValue createAliasAspect( AspectKey originalKey, Aspect aspect, AspectKey depKey, - @Nullable NestedSetBuilder transitivePackagesForPackageRootResolution) + @Nullable NestedSetBuilder transitivePackages) throws InterruptedException { // Compute the AspectValue of the target the alias refers to (which can itself be either an // alias or a real target) @@ -750,12 +741,11 @@ private static AspectValue createAliasAspect( return null; } - NestedSet finalTransitivePackagesForPackageRootResolution = null; - if (transitivePackagesForPackageRootResolution != null) { - finalTransitivePackagesForPackageRootResolution = - transitivePackagesForPackageRootResolution - .addTransitive( - Preconditions.checkNotNull(real.getTransitivePackagesForPackageRootResolution())) + NestedSet finalTransitivePackages = null; + if (transitivePackages != null) { + finalTransitivePackages = + transitivePackages + .addTransitive(Preconditions.checkNotNull(real.getTransitivePackages())) .add(originalTarget.getPackage()) .build(); } @@ -764,7 +754,7 @@ private static AspectValue createAliasAspect( aspect, originalTarget.getLocation(), ConfiguredAspect.forAlias(real.getConfiguredAspect()), - finalTransitivePackagesForPackageRootResolution); + finalTransitivePackages); } @Nullable @@ -841,7 +831,7 @@ private AspectValue createAspect( @Nullable ToolchainCollection toolchainContexts, @Nullable ExecGroupCollection.Builder execGroupCollectionBuilder, OrderedSetMultimap directDeps, - @Nullable NestedSetBuilder transitivePackagesForPackageRootResolution) + @Nullable NestedSetBuilder transitivePackages) throws AspectFunctionException, InterruptedException { // Should be successfully evaluated and cached from the loading phase. StarlarkBuiltinsValue starlarkBuiltinsValue = @@ -862,12 +852,7 @@ private AspectValue createAspect( OutputFile outputFile = (OutputFile) associatedTarget.getTarget(); Label label = outputFile.getGeneratingRule().getLabel(); return createAliasAspect( - env, - associatedTarget.getTarget(), - key, - aspect, - key.withLabel(label), - transitivePackagesForPackageRootResolution); + env, associatedTarget.getTarget(), key, aspect, key.withLabel(label), transitivePackages); } else if (AspectResolver.aspectMatchesConfiguredTarget(associatedTarget, aspect)) { try { CurrentRuleTracker.beginConfiguredAspect(aspect.getAspectClass()); @@ -885,6 +870,7 @@ private AspectValue createAspect( execGroupCollectionBuilder, configuration, view.getHostConfiguration(), + transitivePackages == null ? null : transitivePackages.build(), key); } catch (MissingDepException e) { Preconditions.checkState(env.valuesMissing()); @@ -922,9 +908,7 @@ private AspectValue createAspect( aspect, associatedTarget.getTarget().getLocation(), configuredAspect, - transitivePackagesForPackageRootResolution == null - ? null - : transitivePackagesForPackageRootResolution.build()); + transitivePackages == null ? null : transitivePackages.build()); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BuildDriverFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/BuildDriverFunction.java index ecc21a246459a1..2ce6a3717f5853 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BuildDriverFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BuildDriverFunction.java @@ -241,11 +241,10 @@ private static void postTopLevelTargetAnalyzedEvent( // transitive packages for package root resolution is already cleared. In such a case, the // symlinks should have already been planted. TopLevelTargetAnalyzedEvent topLevelTargetAnalyzedEvent = - configuredTargetValue.getTransitivePackagesForPackageRootResolution() == null + configuredTargetValue.getTransitivePackages() == null ? TopLevelTargetAnalyzedEvent.createWithoutFurtherSymlinkPlanting(configuredTarget) : TopLevelTargetAnalyzedEvent.create( - configuredTarget, - configuredTargetValue.getTransitivePackagesForPackageRootResolution()); + configuredTarget, configuredTargetValue.getTransitivePackages()); env.getListener().post(topLevelTargetAnalyzedEvent); } @@ -406,12 +405,10 @@ private static void postAspectAnalyzedEvent( // transitive packages for package root resolution is already cleared. In such a case, the // symlinks should have already been planted. AspectAnalyzedEvent aspectAnalyzedEvent = - aspectValue.getTransitivePackagesForPackageRootResolution() == null + aspectValue.getTransitivePackages() == null ? AspectAnalyzedEvent.createWithoutFurtherSymlinkPlanting(aspectKey, configuredAspect) : AspectAnalyzedEvent.create( - aspectKey, - configuredAspect, - aspectValue.getTransitivePackagesForPackageRootResolution()); + aspectKey, configuredAspect, aspectValue.getTransitivePackages()); env.getListener().post(aspectAnalyzedEvent); } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java index 82d05a934689c5..7193f82b038d8a 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java @@ -147,10 +147,11 @@ public static ConfiguredValueCreationException asConfiguredValueCreationExceptio /** * Indicates whether the set of packages transitively loaded for a given {@link - * ConfiguredTargetValue} will be needed for package root resolution later in the build. If not, + * ConfiguredTargetValue} will be needed later (see {@link + * com.google.devtools.build.lib.analysis.ConfiguredObjectValue#getTransitivePackages}). If not, * they are not collected and stored. */ - private final boolean storeTransitivePackagesForPackageRootResolution; + private final boolean storeTransitivePackages; private final boolean shouldUnblockCpuWorkWhenFetchingDeps; @@ -158,14 +159,13 @@ public static ConfiguredValueCreationException asConfiguredValueCreationExceptio BuildViewProvider buildViewProvider, RuleClassProvider ruleClassProvider, AtomicReference cpuBoundSemaphore, - boolean storeTransitivePackagesForPackageRootResolution, + boolean storeTransitivePackages, boolean shouldUnblockCpuWorkWhenFetchingDeps, @Nullable ConfiguredTargetProgressReceiver configuredTargetProgress) { this.buildViewProvider = buildViewProvider; this.ruleClassProvider = ruleClassProvider; this.cpuBoundSemaphore = cpuBoundSemaphore; - this.storeTransitivePackagesForPackageRootResolution = - storeTransitivePackagesForPackageRootResolution; + this.storeTransitivePackages = storeTransitivePackages; this.shouldUnblockCpuWorkWhenFetchingDeps = shouldUnblockCpuWorkWhenFetchingDeps; this.configuredTargetProgress = configuredTargetProgress; } @@ -191,7 +191,7 @@ private void maybeReleaseSemaphore() { static class State implements SkyKeyComputeState { /** Null if ConfiguredTargetFuncton is not storing this information. */ - @Nullable NestedSetBuilder transitivePackagesForPackageRootResolution; + @Nullable NestedSetBuilder transitivePackages; NestedSetBuilder transitiveRootCauses = NestedSetBuilder.stableOrder(); @@ -199,9 +199,8 @@ static class State implements SkyKeyComputeState { ComputeDependenciesState computeDependenciesState = new ComputeDependenciesState(); - State(boolean storeTransitivePackagesForPackageRootResolution) { - this.transitivePackagesForPackageRootResolution = - storeTransitivePackagesForPackageRootResolution ? NestedSetBuilder.stableOrder() : null; + State(boolean storeTransitivePackages) { + this.transitivePackages = storeTransitivePackages ? NestedSetBuilder.stableOrder() : null; } } @@ -250,7 +249,7 @@ static class ComputeDependenciesState implements SkyKeyComputeState { @Override public SkyValue compute(SkyKey key, Environment env) throws ReportedException, UnreportedException, InterruptedException { - State state = env.getState(() -> new State(storeTransitivePackagesForPackageRootResolution)); + State state = env.getState(() -> new State(storeTransitivePackages)); if (shouldUnblockCpuWorkWhenFetchingDeps) { // Fetching blocks on other resources, so we don't want to hold on to the semaphore meanwhile. @@ -276,9 +275,7 @@ public SkyValue compute(SkyKey key, Environment env) // not prepared for it. return new NonRuleConfiguredTargetValue( new EmptyConfiguredTarget(target.getLabel(), configuredTargetKey.getConfigurationKey()), - state.transitivePackagesForPackageRootResolution == null - ? null - : state.transitivePackagesForPackageRootResolution.build()); + state.transitivePackages == null ? null : state.transitivePackages.build()); } SkyframeBuildView view = buildViewProvider.getSkyframeBuildView(); @@ -312,7 +309,7 @@ public SkyValue compute(SkyKey key, Environment env) getConfigConditions( env, targetAndConfiguration, - state.transitivePackagesForPackageRootResolution, + state.transitivePackages, platformInfo, state.transitiveRootCauses); if (env.valuesMissing()) { @@ -344,7 +341,7 @@ public SkyValue compute(SkyKey key, Environment env) configConditions, env, platformInfo, - state.transitivePackagesForPackageRootResolution); + state.transitivePackages); if (incompatibleTarget == null) { return null; } @@ -356,7 +353,7 @@ public SkyValue compute(SkyKey key, Environment env) OrderedSetMultimap depValueMap = computeDependencies( state.computeDependenciesState, - state.transitivePackagesForPackageRootResolution, + state.transitivePackages, state.transitiveRootCauses, env, resolver, @@ -390,7 +387,7 @@ public SkyValue compute(SkyKey key, Environment env) depValueMap, configConditions, platformInfo, - state.transitivePackagesForPackageRootResolution); + state.transitivePackages); if (incompatibleTarget.isPresent()) { return incompatibleTarget.get(); } @@ -423,7 +420,7 @@ public SkyValue compute(SkyKey key, Environment env) configConditions, toolchainContexts, execGroupCollectionBuilder, - state.transitivePackagesForPackageRootResolution); + state.transitivePackages); if (ans != null && configuredTargetProgress != null) { configuredTargetProgress.doneConfigureTarget(); } @@ -545,8 +542,8 @@ private static TargetAndConfiguration getTargetAndConfiguration( state.transitiveRootCauses.add( new LoadingFailedCause(label, DetailedExitCode.of(failureDetail))); } - if (state.transitivePackagesForPackageRootResolution != null) { - state.transitivePackagesForPackageRootResolution.add(pkg); + if (state.transitivePackages != null) { + state.transitivePackages.add(pkg); } state.targetAndConfiguration = new TargetAndConfiguration(target, configuration); return state.targetAndConfiguration; @@ -780,7 +777,7 @@ public static ImmutableSet