Skip to content

Commit

Permalink
Provide transitive digest of bzl files for each Starlark Module.
Browse files Browse the repository at this point in the history
Replace the optional label in Module with a context object, storing arbitrary
additional data. The context will still carry a label which can be used to
enhance the information provided by the repr, but also allows to store more
information, useful outside of Starlark interpreter. An example of this is the
transitive hash of the bzl files loaded by the module, which is not relevant to
Starlark itself, but is important in Bazel.

Move the concept of bzlTransitiveDigest from BazelStarlakContext (associated
with the thread) to new BazelModuleContext, which is added to the Starlark
module.

The concept of the transitive digest makes most sense at the level of the
module -- it means the `digest({bzl \in transitively_loaded(module)})`. After
this change, we rely on picking appropriate module to get the digest from
rather than the data associated with the StarlarkThread.

One of the uses of the bzlTransitiveDigest is adding a digest for RuleClass
instances. After this change, we explicitly use the hash of the module which
defines the RuleClass itself.

PiperOrigin-RevId: 311173293
  • Loading branch information
alexjski authored and copybara-github committed May 12, 2020
1 parent 60b2eaf commit 9f2cab5
Show file tree
Hide file tree
Showing 20 changed files with 147 additions and 123 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -763,16 +763,14 @@ public ImmutableMap<String, Object> getEnvironment() {
public void setStarlarkThreadContext(
StarlarkThread thread,
Label fileLabel,
byte[] transitiveDigest,
ImmutableMap<RepositoryName, RepositoryName> repoMapping) {
new BazelStarlarkContext(
BazelStarlarkContext.Phase.LOADING,
toolsRepository,
configurationFragmentMap,
repoMapping,
new SymbolGenerator<>(fileLabel),
/*analysisRuleLabel=*/ null,
transitiveDigest)
/*analysisRuleLabel=*/ null)
.storeInThread(thread);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,13 @@
import com.google.devtools.build.lib.analysis.config.TransitionFactories;
import com.google.devtools.build.lib.analysis.config.transitions.SplitTransition;
import com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.packages.Attribute;
import com.google.devtools.build.lib.packages.Attribute.AllowedValueSet;
import com.google.devtools.build.lib.packages.Attribute.ImmutableAttributeFactory;
import com.google.devtools.build.lib.packages.Attribute.SkylarkComputedDefaultTemplate;
import com.google.devtools.build.lib.packages.AttributeTransitionData;
import com.google.devtools.build.lib.packages.AttributeValueSource;
import com.google.devtools.build.lib.packages.BazelModuleContext;
import com.google.devtools.build.lib.packages.BazelStarlarkContext;
import com.google.devtools.build.lib.packages.BuildType;
import com.google.devtools.build.lib.packages.Provider;
Expand Down Expand Up @@ -138,7 +138,9 @@ private static Attribute.Builder<?> createAttribute(
builder.defaultValue(
defaultValue,
new BuildType.LabelConversionContext(
(Label) Module.ofInnermostEnclosingStarlarkFunction(thread).getLabel(),
((BazelModuleContext)
Module.ofInnermostEnclosingStarlarkFunction(thread).getClientData())
.label(),
BazelStarlarkContext.from(thread).getRepoMapping()),
DEFAULT_ARG);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
import com.google.devtools.build.lib.packages.AttributeContainer;
import com.google.devtools.build.lib.packages.AttributeMap;
import com.google.devtools.build.lib.packages.AttributeValueSource;
import com.google.devtools.build.lib.packages.BazelModuleContext;
import com.google.devtools.build.lib.packages.BazelStarlarkContext;
import com.google.devtools.build.lib.packages.BuildSetting;
import com.google.devtools.build.lib.packages.BuildType;
Expand Down Expand Up @@ -344,9 +345,11 @@ public StarlarkCallable rule(
.requiresHostConfigurationFragmentsByStarlarkBuiltinName(
Sequence.cast(hostFragments, String.class, "host_fragments"));
builder.setConfiguredTargetFunction(implementation);
// Information about the .bzl module containing the call that created the rule class.
BazelModuleContext bzlModule =
(BazelModuleContext) Module.ofInnermostEnclosingStarlarkFunction(thread).getClientData();
builder.setRuleDefinitionEnvironmentLabelAndDigest(
(Label) Module.ofInnermostEnclosingStarlarkFunction(thread).getLabel(),
bazelContext.getTransitiveDigest());
bzlModule.label(), bzlModule.bzlTransitiveDigest());

builder.addRequiredToolchains(parseToolchains(toolchains, thread));

Expand Down Expand Up @@ -848,7 +851,9 @@ public Label label(String labelString, Boolean relativeToCallerRepository, Starl
parentLabel = context.getAnalysisRuleLabel();
} else {
// This is the label of the innermost BUILD/.bzl file on the current call stack.
parentLabel = (Label) Module.ofInnermostEnclosingStarlarkFunction(thread).getLabel();
parentLabel =
((BazelModuleContext) Module.ofInnermostEnclosingStarlarkFunction(thread).getClientData())
.label();
}

try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,7 @@ public static ConfiguredTarget buildRule(
/*fragmentNameToClass=*/ null,
ruleContext.getTarget().getPackage().getRepositoryMapping(),
ruleContext.getSymbolGenerator(),
ruleContext.getLabel(),
/*transitiveDigest=*/ null)
ruleContext.getLabel())
.storeInThread(thread);

RuleClass ruleClass = ruleContext.getRule().getRuleClassObject();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,7 @@ public RepositoryDirectoryValue.Builder fetch(
/*fragmentNameToClass=*/ null,
rule.getPackage().getRepositoryMapping(),
new SymbolGenerator<>(key),
/*analysisRuleLabel=*/ null,
/*transitiveDigest=*/ null)
/*analysisRuleLabel=*/ null)
.storeInThread(thread);

StarlarkRepositoryContext starlarkRepositoryContext =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.packages.AttributeValueSource;
import com.google.devtools.build.lib.packages.BazelModuleContext;
import com.google.devtools.build.lib.packages.BazelStarlarkContext;
import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.packages.Package.NameConflictException;
Expand Down Expand Up @@ -98,9 +99,10 @@ public StarlarkCallable repositoryRule(
}
}
builder.setConfiguredTargetFunction(implementation);
BazelModuleContext bzlModule =
(BazelModuleContext) Module.ofInnermostEnclosingStarlarkFunction(thread).getClientData();
builder.setRuleDefinitionEnvironmentLabelAndDigest(
(Label) Module.ofInnermostEnclosingStarlarkFunction(thread).getLabel(),
context.getTransitiveDigest());
bzlModule.label(), bzlModule.bzlTransitiveDigest());
builder.setWorkspaceOnly();
return new RepositoryRuleFunction(builder, implementation);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// Copyright 2020 The Bazel Authors. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package com.google.devtools.build.lib.packages;

import com.google.auto.value.AutoValue;
import com.google.devtools.build.lib.cmdline.Label;

/**
* BazelModuleContext records Bazel-specific information associated with a .bzl {@link
* com.google.devtools.build.lib.syntax.Module}.
*/
@AutoValue
public abstract class BazelModuleContext {
/** Label associated with the Starlark {@link com.google.devtools.build.lib.syntax.Module}. */
public abstract Label label();

/**
* Transitive digest of the .bzl file of the {@link com.google.devtools.build.lib.syntax.Module}
* itself and all files it transitively loads.
*/
@SuppressWarnings("AutoValueImmutableFields")
public abstract byte[] bzlTransitiveDigest();

/**
* Returns a label for a {@link com.google.devtools.build.lib.syntax.Module}.
*
* <p>This is a user-facing value and we rely on this string to be a valid label for the {@link
* com.google.devtools.build.lib.syntax.Module} (and that only). Please see the documentation of
* {@link com.google.devtools.build.lib.syntax.Module#withClientData(Object)} for more details.
*/
@Override
public final String toString() {
return label().toString();
}

public static BazelModuleContext create(Label label, byte[] bzlTransitiveDigest) {
return new AutoValue_BazelModuleContext(label, bzlTransitiveDigest);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ public void storeInThread(StarlarkThread thread) {
private final ImmutableMap<RepositoryName, RepositoryName> repoMapping;
private final SymbolGenerator<?> symbolGenerator;
@Nullable private final Label analysisRuleLabel;
@Nullable private final byte[] transitiveDigest;

/**
* @param phase the phase to which this Starlark thread belongs
Expand All @@ -77,15 +76,13 @@ public BazelStarlarkContext(
@Nullable ImmutableMap<String, Class<?>> fragmentNameToClass,
ImmutableMap<RepositoryName, RepositoryName> repoMapping,
SymbolGenerator<?> symbolGenerator,
@Nullable Label analysisRuleLabel,
@Nullable byte[] transitiveDigest) {
@Nullable Label analysisRuleLabel) {
this.phase = phase;
this.toolsRepository = toolsRepository;
this.fragmentNameToClass = fragmentNameToClass;
this.repoMapping = repoMapping;
this.symbolGenerator = Preconditions.checkNotNull(symbolGenerator);
this.analysisRuleLabel = analysisRuleLabel;
this.transitiveDigest = transitiveDigest;
}

/** Returns the phase to which this Starlark thread belongs. */
Expand Down Expand Up @@ -127,16 +124,6 @@ public Label getAnalysisRuleLabel() {
return analysisRuleLabel;
}

/**
* Returns the digest of the .bzl file and those it transitively loads. Only defined for .bzl
* initialization threads. Returns a dummy value (empty array) for WORKSPACE initialization
* threads. Returns null for all other threads.
*/
@Nullable
public byte[] getTransitiveDigest() {
return transitiveDigest;
}

/**
* Checks that the Starlark thread is in the loading or the workspace phase.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -832,8 +832,7 @@ private boolean buildPackage(
/*fragmentNameToClass=*/ null,
pkgBuilder.getRepositoryMapping(),
new SymbolGenerator<>(packageId),
/*analysisRuleLabel=*/ null,
/*transitiveDigest=*/ null)
/*analysisRuleLabel=*/ null)
.storeInThread(thread);

// TODO(adonovan): save this as a field in BazelSkylarkContext.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,11 @@ public interface RuleClassProvider extends RuleDefinitionContext {
*
* @param thread StarlarkThread in which to store the context.
* @param label the label of the .bzl file
* @param transitiveDigest digest of the .bzl file and those it transitively loads
* @param repoMapping map of RepositoryNames to be remapped
*/
void setStarlarkThreadContext(
StarlarkThread thread,
Label label,
byte[] transitiveDigest,
ImmutableMap<RepositoryName, RepositoryName> repoMapping);

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,8 +184,7 @@ private void execute(
/*fragmentNameToClass=*/ null,
/*repoMapping=*/ ImmutableMap.of(),
new SymbolGenerator<>(workspaceFileKey),
/*analysisRuleLabel=*/ null,
/*transitiveDigest=*/ new byte[] {}) // dummy value used for repository rules
/*analysisRuleLabel=*/ null)
.storeInThread(thread);

Resolver.resolveFile(file, thread.getGlobals());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -235,10 +235,11 @@ private static RepositoryName getRepositoryName(@Nullable Label label) {
return label.getPackageIdentifier().getRepository();
}

private static List<String> renamePatterns(
private static ImmutableList<String> renamePatterns(
List<String> patterns, Package.Builder builder, StarlarkThread thread) {
RepositoryName myName =
getRepositoryName((Label) Module.ofInnermostEnclosingStarlarkFunction(thread).getLabel());
BazelModuleContext bzlModule =
(BazelModuleContext) Module.ofInnermostEnclosingStarlarkFunction(thread).getClientData();
RepositoryName myName = getRepositoryName((bzlModule != null ? bzlModule.label() : null));
Map<RepositoryName, RepositoryName> renaming = builder.getRepositoryMappingFor(myName);
return patterns.stream()
.map(patternEntry -> TargetPattern.renameRepository(patternEntry, renaming))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.packages.BazelModuleContext;
import com.google.devtools.build.lib.skylarkbuildapi.cpp.CcCompilationOutputsApi;
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.syntax.Module;
Expand Down Expand Up @@ -109,7 +109,8 @@ public Sequence<Artifact> getSkylarkObjectFiles(boolean usePic, StarlarkThread t
CcCommon.checkLocationWhitelisted(
thread.getSemantics(),
thread.getCallerLocation(),
((Label) Module.ofInnermostEnclosingStarlarkFunction(thread).getLabel())
((BazelModuleContext) Module.ofInnermostEnclosingStarlarkFunction(thread).getClientData())
.label()
.getPackageIdentifier()
.toString());
return StarlarkList.immutableCopyOf(getObjectFiles(usePic));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,7 @@ public ConfiguredAspect create(
/*fragmentNameToClass=*/ null,
ruleContext.getRule().getPackage().getRepositoryMapping(),
ruleContext.getSymbolGenerator(),
ruleContext.getLabel(),
/*transitiveDigest=*/ null)
ruleContext.getLabel())
.storeInThread(thread);

try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.events.ExtendedEventHandler.Postable;
import com.google.devtools.build.lib.events.StoredEventHandler;
import com.google.devtools.build.lib.packages.BazelModuleContext;
import com.google.devtools.build.lib.packages.BuildFileNotFoundException;
import com.google.devtools.build.lib.packages.PackageFactory;
import com.google.devtools.build.lib.packages.RuleClassProvider;
Expand Down Expand Up @@ -683,14 +684,15 @@ private Module executeModule(
try (Mutability mu = Mutability.create("importing", moduleLabel)) {
StarlarkThread thread =
StarlarkThread.builder(mu)
.setGlobals(Module.createForBuiltins(predeclared).withLabel(moduleLabel))
.setGlobals(
Module.createForBuiltins(predeclared)
.withClientData(BazelModuleContext.create(moduleLabel, transitiveDigest)))
.setSemantics(starlarkSemantics)
.build();
thread.setLoader(loadedModules::get);
StoredEventHandler eventHandler = new StoredEventHandler();
thread.setPrintHandler(Event.makeDebugPrintHandler(eventHandler));
ruleClassProvider.setStarlarkThreadContext(
thread, moduleLabel, transitiveDigest, repositoryMapping);
ruleClassProvider.setStarlarkThreadContext(thread, moduleLabel, repositoryMapping);
Module module = thread.getGlobals();
execAndExport(file, moduleLabel, eventHandler, thread);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ public static void assertFramesEqual(Module frame1, Module frame2) {
public static void assertModulesEqual(Module frame1, Module frame2) {
assertThat(frame1.mutability().getAnnotation())
.isEqualTo(frame2.mutability().getAnnotation());
assertThat(frame1.getLabel()).isEqualTo(frame2.getLabel());
assertThat(frame1.getClientData()).isEqualTo(frame2.getClientData());
assertThat(frame1.getTransitiveBindings())
.containsExactlyEntriesIn(frame2.getTransitiveBindings()).inOrder();
if (frame1.getParent() == null || frame2.getParent() == null) {
Expand Down
Loading

0 comments on commit 9f2cab5

Please sign in to comment.