Propagate skylark flags to WORKSPACE and repo rules
RELNOTES: Skylark semantics flags now affect WORKSPACE files and repository rules.
PiperOrigin-RevId: 173130286
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryFunction.java
index 6f70886..027d901 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryFunction.java
@@ -22,10 +22,12 @@
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue;
import com.google.devtools.build.lib.rules.repository.RepositoryFunction;
+import com.google.devtools.build.lib.skyframe.PrecomputedValue;
import com.google.devtools.build.lib.syntax.BaseFunction;
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.syntax.Mutability;
import com.google.devtools.build.lib.syntax.Runtime;
+import com.google.devtools.build.lib.syntax.SkylarkSemantics;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.skyframe.SkyFunction.Environment;
@@ -54,11 +56,14 @@
if (declareEnvironmentDependencies(markerData, env, getEnviron(rule)) == null) {
return null;
}
+ SkylarkSemantics skylarkSemantics = PrecomputedValue.SKYLARK_SEMANTICS.get(env);
+ if (skylarkSemantics == null) {
+ return null;
+ }
try (Mutability mutability = Mutability.create("skylark repository")) {
- // This Skylark environment ignores command line flags.
com.google.devtools.build.lib.syntax.Environment buildEnv =
com.google.devtools.build.lib.syntax.Environment.builder(mutability)
- .useDefaultSemantics()
+ .setSemantics(skylarkSemantics)
.setEventHandler(env.getListener())
.build();
SkylarkRepositoryContext skylarkRepositoryContext =
diff --git a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
index 0a9d68b..88ef31b 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
@@ -48,6 +48,7 @@
import com.google.devtools.build.lib.syntax.Runtime;
import com.google.devtools.build.lib.syntax.Runtime.NoneType;
import com.google.devtools.build.lib.syntax.SkylarkList;
+import com.google.devtools.build.lib.syntax.SkylarkSemantics;
import com.google.devtools.build.lib.syntax.SkylarkSignatureProcessor;
import com.google.devtools.build.lib.vfs.Path;
import java.io.File;
@@ -141,15 +142,19 @@
}
/**
- * Parses the given WORKSPACE file without resolving skylark imports.
+ * Parses the given WORKSPACE file without resolving Skylark imports, using the default Skylark
+ * semantics.
*/
public void parse(ParserInputSource source)
throws BuildFileContainsErrorsException, InterruptedException {
- parse(source, null);
+ parse(source, SkylarkSemantics.DEFAULT_SEMANTICS, null);
}
@VisibleForTesting
- public void parse(ParserInputSource source, @Nullable StoredEventHandler localReporter)
+ public void parse(
+ ParserInputSource source,
+ SkylarkSemantics skylarkSemantics,
+ @Nullable StoredEventHandler localReporter)
throws BuildFileContainsErrorsException, InterruptedException {
// This method is split in 2 so WorkspaceFileFunction can call the two parts separately and
// do the Skylark load imports in between. We can't load skylark imports from
@@ -163,7 +168,7 @@
throw new BuildFileContainsErrorsException(
Label.EXTERNAL_PACKAGE_IDENTIFIER, "Failed to parse " + source.getPath());
}
- execute(buildFileAST, null, localReporter);
+ execute(buildFileAST, null, skylarkSemantics, localReporter);
}
@@ -171,15 +176,21 @@
* Actually runs through the AST, calling the functions in the WORKSPACE file and adding rules
* to the //external package.
*/
- public void execute(BuildFileAST ast, Map<String, Extension> importedExtensions)
+ public void execute(
+ BuildFileAST ast,
+ Map<String, Extension> importedExtensions,
+ SkylarkSemantics skylarkSemantics)
throws InterruptedException {
Preconditions.checkNotNull(ast);
Preconditions.checkNotNull(importedExtensions);
- execute(ast, importedExtensions, new StoredEventHandler());
+ execute(ast, importedExtensions, skylarkSemantics, new StoredEventHandler());
}
- private void execute(BuildFileAST ast, @Nullable Map<String, Extension> importedExtensions,
+ private void execute(
+ BuildFileAST ast,
+ @Nullable Map<String, Extension> importedExtensions,
+ SkylarkSemantics skylarkSemantics,
StoredEventHandler localReporter)
throws InterruptedException {
if (importedExtensions != null) {
@@ -191,8 +202,7 @@
}
Environment workspaceEnv =
Environment.builder(mutability)
- // Note that this Skylark environment ignores command line flags.
- .useDefaultSemantics()
+ .setSemantics(skylarkSemantics)
.setGlobals(BazelLibrary.GLOBALS)
.setEventHandler(localReporter)
.setImportedExtensions(importMap)
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java
index 71f2c98..3f0f3f8 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java
@@ -27,6 +27,7 @@
import com.google.devtools.build.lib.syntax.BuildFileAST;
import com.google.devtools.build.lib.syntax.Environment.Extension;
import com.google.devtools.build.lib.syntax.Mutability;
+import com.google.devtools.build.lib.syntax.SkylarkSemantics;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.RootedPath;
import com.google.devtools.build.skyframe.SkyFunction;
@@ -66,6 +67,10 @@
if (workspaceASTValue == null) {
return null;
}
+ SkylarkSemantics skylarkSemantics = PrecomputedValue.SKYLARK_SEMANTICS.get(env);
+ if (skylarkSemantics == null) {
+ return null;
+ }
Path repoWorkspace = workspaceRoot.getRoot().getRelative(workspaceRoot.getRelativePath());
Package.Builder builder = packageFactory.newExternalPackageBuilder(
@@ -112,7 +117,7 @@
if (importResult == null) {
return null;
}
- parser.execute(ast, importResult.importMap);
+ parser.execute(ast, importResult.importMap, skylarkSemantics);
} catch (NoSuchPackageException e) {
throw new WorkspaceFileFunctionException(e, Transience.PERSISTENT);
} catch (NameConflictException e) {
diff --git a/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTest.java b/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTest.java
index 59455ec..e92d3af 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTest.java
@@ -24,6 +24,7 @@
import com.google.devtools.build.lib.packages.Package.Builder;
import com.google.devtools.build.lib.syntax.Mutability;
import com.google.devtools.build.lib.syntax.ParserInputSource;
+import com.google.devtools.build.lib.syntax.SkylarkSemantics;
import com.google.devtools.build.lib.testutil.Scratch;
import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
import com.google.devtools.build.lib.vfs.Path;
@@ -149,7 +150,10 @@
root);
Exception exception = null;
try {
- factory.parse(ParserInputSource.create(workspaceFilePath), eventHandler);
+ factory.parse(
+ ParserInputSource.create(workspaceFilePath),
+ SkylarkSemantics.DEFAULT_SEMANTICS,
+ eventHandler);
} catch (BuildFileContainsErrorsException e) {
exception = e;
} catch (IOException | InterruptedException e) {
diff --git a/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java b/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java
index 57e517e..6bf0519 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java
@@ -37,6 +37,7 @@
import com.google.devtools.build.lib.skyframe.SkyFunctions;
import com.google.devtools.build.lib.skyframe.WorkspaceASTFunction;
import com.google.devtools.build.lib.skyframe.WorkspaceFileFunction;
+import com.google.devtools.build.lib.syntax.SkylarkSemantics;
import com.google.devtools.build.lib.testutil.FoundationTestCase;
import com.google.devtools.build.lib.testutil.TestConstants;
import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
@@ -125,6 +126,7 @@
.put(RepositoryName.createFromValidStrippedName("foo"), overrideDirectory.asFragment())
.build());
PrecomputedValue.PATH_PACKAGE_LOCATOR.set(differencer, pkgLocator.get());
+ PrecomputedValue.SKYLARK_SEMANTICS.set(differencer, SkylarkSemantics.DEFAULT_SEMANTICS);
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupFunctionTest.java
index 3c662db..19b0c95 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupFunctionTest.java
@@ -37,6 +37,7 @@
import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFileAction;
import com.google.devtools.build.lib.skyframe.PackageLookupFunction.CrossRepositoryLabelViolationStrategy;
import com.google.devtools.build.lib.skyframe.PackageLookupValue.BuildFileName;
+import com.google.devtools.build.lib.syntax.SkylarkSemantics;
import com.google.devtools.build.lib.testutil.FoundationTestCase;
import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor;
import com.google.devtools.build.lib.vfs.PathFragment;
@@ -137,9 +138,10 @@
evaluator = new InMemoryMemoizingEvaluator(skyFunctions, differencer);
driver = new SequentialBuildDriver(evaluator);
PrecomputedValue.BUILD_ID.set(differencer, UUID.randomUUID());
- PrecomputedValue.PATH_PACKAGE_LOCATOR.set(differencer, pkgLocator.get());
PrecomputedValue.BLACKLISTED_PACKAGE_PREFIXES_FILE.set(differencer,
PathFragment.EMPTY_FRAGMENT);
+ PrecomputedValue.PATH_PACKAGE_LOCATOR.set(differencer, pkgLocator.get());
+ PrecomputedValue.SKYLARK_SEMANTICS.set(differencer, SkylarkSemantics.DEFAULT_SEMANTICS);
RepositoryDelegatorFunction.REPOSITORY_OVERRIDES.set(
differencer, ImmutableMap.<RepositoryName, PathFragment>of());
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java
index 93bb5ae..846e6d3 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java
@@ -42,6 +42,7 @@
import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFileAction;
import com.google.devtools.build.lib.skyframe.PackageLookupFunction.CrossRepositoryLabelViolationStrategy;
import com.google.devtools.build.lib.skyframe.PackageLookupValue.BuildFileName;
+import com.google.devtools.build.lib.syntax.SkylarkSemantics;
import com.google.devtools.build.lib.testutil.ManualClock;
import com.google.devtools.build.lib.testutil.TestConstants;
import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
@@ -169,6 +170,7 @@
PrecomputedValue.PATH_PACKAGE_LOCATOR.set(differencer, pkgLocator);
RepositoryDelegatorFunction.REPOSITORY_OVERRIDES.set(
differencer, ImmutableMap.<RepositoryName, PathFragment>of());
+ PrecomputedValue.SKYLARK_SEMANTICS.set(differencer, SkylarkSemantics.DEFAULT_SEMANTICS);
return new SequentialBuildDriver(evaluator);
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/LocalRepositoryLookupFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/LocalRepositoryLookupFunctionTest.java
index ca81506..7c4e309 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/LocalRepositoryLookupFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/LocalRepositoryLookupFunctionTest.java
@@ -31,6 +31,7 @@
import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFileAction;
import com.google.devtools.build.lib.skyframe.PackageLookupFunction.CrossRepositoryLabelViolationStrategy;
import com.google.devtools.build.lib.skyframe.PackageLookupValue.BuildFileName;
+import com.google.devtools.build.lib.syntax.SkylarkSemantics;
import com.google.devtools.build.lib.testutil.FoundationTestCase;
import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
@@ -114,6 +115,7 @@
evaluator = new InMemoryMemoizingEvaluator(skyFunctions, differencer);
driver = new SequentialBuildDriver(evaluator);
PrecomputedValue.PATH_PACKAGE_LOCATOR.set(differencer, pkgLocator.get());
+ PrecomputedValue.SKYLARK_SEMANTICS.set(differencer, SkylarkSemantics.DEFAULT_SEMANTICS);
}
private SkyKey createKey(RootedPath directory) {
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/PackageLookupFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/PackageLookupFunctionTest.java
index e24dca8..27eef90 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/PackageLookupFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/PackageLookupFunctionTest.java
@@ -42,6 +42,7 @@
import com.google.devtools.build.lib.skyframe.PackageLookupValue.BuildFileName;
import com.google.devtools.build.lib.skyframe.PackageLookupValue.ErrorReason;
import com.google.devtools.build.lib.skyframe.PackageLookupValue.IncorrectRepositoryReferencePackageLookupValue;
+import com.google.devtools.build.lib.syntax.SkylarkSemantics;
import com.google.devtools.build.lib.testutil.FoundationTestCase;
import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
@@ -148,9 +149,10 @@
evaluator = new InMemoryMemoizingEvaluator(skyFunctions, differencer);
driver = new SequentialBuildDriver(evaluator);
PrecomputedValue.BUILD_ID.set(differencer, UUID.randomUUID());
- PrecomputedValue.PATH_PACKAGE_LOCATOR.set(differencer, pkgLocator.get());
PrecomputedValue.BLACKLISTED_PACKAGE_PREFIXES_FILE.set(
differencer, PathFragment.EMPTY_FRAGMENT);
+ PrecomputedValue.PATH_PACKAGE_LOCATOR.set(differencer, pkgLocator.get());
+ PrecomputedValue.SKYLARK_SEMANTICS.set(differencer, SkylarkSemantics.DEFAULT_SEMANTICS);
RepositoryDelegatorFunction.REPOSITORY_OVERRIDES.set(
differencer, ImmutableMap.<RepositoryName, PathFragment>of());
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java
index 61a6e98..d5e37ce 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java
@@ -25,6 +25,7 @@
import com.google.devtools.build.lib.packages.PackageFactory;
import com.google.devtools.build.lib.packages.PackageFactory.EnvironmentExtension;
import com.google.devtools.build.lib.packages.Rule;
+import com.google.devtools.build.lib.syntax.SkylarkSemantics;
import com.google.devtools.build.lib.testutil.MoreAsserts;
import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
import com.google.devtools.build.lib.vfs.Path;
@@ -166,6 +167,19 @@
return astSkyFunc.compute(key, getEnv());
}
});
+ Mockito.when(env.getValue(Matchers.argThat(new SkyKeyMatchers(SkyFunctions.PRECOMPUTED))))
+ .then(
+ new Answer<SkyValue>() {
+ @Override
+ public SkyValue answer(InvocationOnMock invocation) throws Throwable {
+ SkyKey key = (SkyKey) invocation.getArguments()[0];
+ if (key.equals(PrecomputedValue.SKYLARK_SEMANTICS.getKeyForTesting())) {
+ return new PrecomputedValue(SkylarkSemantics.DEFAULT_SEMANTICS);
+ } else {
+ return null;
+ }
+ }
+ });
return env;
}
diff --git a/src/test/shell/bazel/skylark_repository_test.sh b/src/test/shell/bazel/skylark_repository_test.sh
index c1d9608..72c08bb 100755
--- a/src/test/shell/bazel/skylark_repository_test.sh
+++ b/src/test/shell/bazel/skylark_repository_test.sh
@@ -24,6 +24,8 @@
source "${CURRENT_DIR}/remote_helpers.sh" \
|| { echo "remote_helpers.sh not found!" >&2; exit 1; }
+SKYLARK_FLAG_MARKER="<== skylark flag test ==>"
+
# Basic test.
function test_macro_local_repository() {
create_new_workspace
@@ -334,6 +336,26 @@
cat > BUILD
}
+function test_skylark_flags_affect_repository_rule() {
+ setup_skylark_repository
+
+ cat >test.bzl <<EOF
+def _impl(repository_ctx):
+ print("In repo rule: ")
+ # Symlink so a repository is created
+ repository_ctx.symlink(repository_ctx.path("$repo2"), repository_ctx.path(""))
+
+repo = repository_rule(implementation=_impl, local=True)
+EOF
+
+ # Build with the special testing flag that appends a marker string to all
+ # print() calls.
+ bazel build @foo//:bar --internal_skylark_flag_test_canary >& $TEST_log \
+ || fail "Expected build to succeed"
+ expect_log "In repo rule: $SKYLARK_FLAG_MARKER" \
+ "Skylark flags are not propagating to repository rules"
+}
+
function test_skylark_repository_which_and_execute() {
setup_skylark_repository
diff --git a/src/test/shell/bazel/workspace_test.sh b/src/test/shell/bazel/workspace_test.sh
index 5b4b7de..bd00105 100755
--- a/src/test/shell/bazel/workspace_test.sh
+++ b/src/test/shell/bazel/workspace_test.sh
@@ -21,6 +21,8 @@
export JAVA_RUNFILES=$BAZEL_RUNFILES
+SKYLARK_FLAG_MARKER="<== skylark flag test ==>"
+
function setup_repo() {
mkdir -p $1
touch $1/WORKSPACE
@@ -173,6 +175,30 @@
[ ! -L bazel-x ] || fail "bazel-x should have been removed"
}
+function test_skylark_flags_affect_workspace() {
+ cat > WORKSPACE <<EOF
+load("//:macro.bzl", "macro")
+print("In workspace: ")
+macro()
+EOF
+ cat > macro.bzl <<EOF
+def macro():
+ print("In workspace macro: ")
+EOF
+ cat > BUILD <<'EOF'
+genrule(name = "x", cmd = "echo hi > $@", outs = ["x.out"], srcs = [])
+EOF
+
+ # Build with the special testing flag that appends a marker string to all
+ # print() calls.
+ bazel build //:x --internal_skylark_flag_test_canary &>"$TEST_log" \
+ || fail "Expected build to succeed"
+ expect_log "In workspace: $SKYLARK_FLAG_MARKER" \
+ "Skylark flags are not propagating to workspace evaluation"
+ expect_log "In workspace macro: $SKYLARK_FLAG_MARKER" \
+ "Skylark flags are not propagating to workspace macro evaluation"
+}
+
function test_workspace_name() {
mkdir -p foo
mkdir -p bar
