diff options
Diffstat (limited to 'src')
4 files changed, 129 insertions, 48 deletions
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 8caec89db0..d48aa7316c 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 @@ -28,6 +28,7 @@ import com.google.devtools.build.lib.events.StoredEventHandler; import com.google.devtools.build.lib.packages.Package.Builder; import com.google.devtools.build.lib.packages.Package.LegacyBuilder; import com.google.devtools.build.lib.packages.PackageFactory.EnvironmentExtension; +import com.google.devtools.build.lib.skylarkinterface.SkylarkSignature; import com.google.devtools.build.lib.syntax.BaseFunction; import com.google.devtools.build.lib.syntax.BuildFileAST; import com.google.devtools.build.lib.syntax.BuiltinFunction; @@ -40,11 +41,15 @@ import com.google.devtools.build.lib.syntax.FunctionSignature; import com.google.devtools.build.lib.syntax.Mutability; import com.google.devtools.build.lib.syntax.ParserInputSource; import com.google.devtools.build.lib.syntax.Runtime; +import com.google.devtools.build.lib.syntax.SkylarkList; +import com.google.devtools.build.lib.syntax.SkylarkSignatureProcessor; import com.google.devtools.build.lib.vfs.Path; import java.io.File; import java.io.IOException; import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import javax.annotation.Nullable; @@ -161,23 +166,38 @@ public class WorkspaceFactory { localReporter.clear(); } - // TODO(bazel-team): use @SkylarkSignature annotations on a BuiltinFunction.Factory - // for signature + documentation of this and other functions in this file. - private static BuiltinFunction newWorkspaceNameFunction() { - return new BuiltinFunction( - "workspace", FunctionSignature.namedOnly("name"), BuiltinFunction.USE_AST_ENV) { - public Object invoke(String name, FuncallExpression ast, Environment env) - throws EvalException { - String errorMessage = LabelValidator.validateTargetName(name); - if (errorMessage != null) { - throw new EvalException(ast.getLocation(), errorMessage); + @SkylarkSignature(name = "workspace", objectType = Object.class, returnType = SkylarkList.class, + doc = "Sets the name for this workspace. Workspace names should be a Java-package-style " + + "description of the project, using underscores as separators, e.g., " + + "github.com/bazelbuild/bazel should use com_github_bazelbuild_bazel. Names must start " + + "with a letter and can only contain letters, numbers, and underscores.", + mandatoryPositionals = { + @SkylarkSignature.Param(name = "name", type = String.class, + doc = "the name of the workspace.")}, + documented = true, useAst = true, useEnvironment = true) + private static final BuiltinFunction.Factory newWorkspaceFunction = + new BuiltinFunction.Factory("workspace") { + public BuiltinFunction create() { + return new BuiltinFunction( + "workspace", FunctionSignature.namedOnly("name"), BuiltinFunction.USE_AST_ENV) { + public Object invoke(String name, FuncallExpression ast, Environment env) + throws EvalException { + Pattern legalWorkspaceName = Pattern.compile("^\\p{Alpha}\\w*$"); + Matcher matcher = legalWorkspaceName.matcher(name); + if (!matcher.matches()) { + throw new EvalException( + ast.getLocation(), name + " is not a legal workspace name"); + } + String errorMessage = LabelValidator.validateTargetName(name); + if (errorMessage != null) { + throw new EvalException(ast.getLocation(), errorMessage); + } + PackageFactory.getContext(env, ast).pkgBuilder.setWorkspaceName(name); + return NONE; + } + }; } - - PackageFactory.getContext(env, ast).pkgBuilder.setWorkspaceName(name); - return NONE; - } - }; - } + }; private static BuiltinFunction newBindFunction(final RuleFactory ruleFactory) { return new BuiltinFunction( @@ -256,7 +276,7 @@ public class WorkspaceFactory { private void addWorkspaceFunctions(Environment workspaceEnv, StoredEventHandler localReporter) { try { - workspaceEnv.update("workspace", newWorkspaceNameFunction()); + workspaceEnv.setup("workspace", newWorkspaceFunction.apply()); for (Map.Entry<String, BaseFunction> function : workspaceFunctions.entrySet()) { workspaceEnv.update(function.getKey(), function.getValue()); } @@ -296,4 +316,8 @@ public class WorkspaceFactory { public static ClassObject newNativeModule(RuleClassProvider ruleClassProvider) { return newNativeModule(createWorkspaceFunctions(ruleClassProvider)); } + + static { + SkylarkSignatureProcessor.configureSkylarkFunctions(WorkspaceFactory.class); + } } 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 13be11547d..d2d8b7c78e 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 @@ -16,6 +16,7 @@ package com.google.devtools.build.lib.packages; import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.fail; import com.google.common.collect.ImmutableList; @@ -28,7 +29,6 @@ import com.google.devtools.build.lib.testutil.Scratch; import com.google.devtools.build.lib.testutil.TestRuleClassProvider; import com.google.devtools.build.lib.vfs.Path; -import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -42,38 +42,95 @@ import java.util.List; @RunWith(JUnit4.class) public class WorkspaceFactoryTest { - private Scratch scratch; - private Path root; + @Test + public void testLoadError() throws Exception { + // WS with a syntax error: '//a' should end with .bzl. + WorkspaceFactoryHelper helper = parse("load('//a', 'a')"); + helper.assertLexingExceptionThrown(); + assertThat(helper.getLexerError()) + .contains("The label must reference a file with extension '.bzl'"); + } - @Before - public void setUpFileSystem() throws Exception { - scratch = new Scratch("/"); - root = scratch.dir("/workspace"); + @Test + public void testWorkspaceName() throws Exception { + WorkspaceFactoryHelper helper = parse("workspace(name = 'my_ws')"); + assertEquals("my_ws", helper.getPackage().getWorkspaceName()); } @Test - public void testLoadError() throws Exception { - // WS with a syntax error: '//a' should end with .bzl. - Path workspaceFilePath = scratch.file("/workspace/WORKSPACE", "load('//a', 'a')"); - LegacyBuilder builder = Package.newExternalPackageBuilder(workspaceFilePath, ""); - WorkspaceFactory factory = - new WorkspaceFactory( - builder, - TestRuleClassProvider.getRuleClassProvider(), - ImmutableList.<PackageFactory.EnvironmentExtension>of(), - Mutability.create("test"), - root, - root); - StoredEventHandler localReporter = new StoredEventHandler(); - try { - factory.parse(ParserInputSource.create(workspaceFilePath), localReporter); - fail("Parsing " + workspaceFilePath + " should have failed"); - } catch (IOException e) { - assertThat(e.getMessage()).contains("Failed to parse " + workspaceFilePath); + public void testWorkspaceStartsWithNumber() throws Exception { + WorkspaceFactoryHelper helper = parse("workspace(name = '123abc')"); + assertThat(helper.getParserError()).contains("123abc is not a legal workspace name"); + } + + @Test + public void testWorkspaceWithIllegalCharacters() throws Exception { + WorkspaceFactoryHelper helper = parse("workspace(name = 'a.b.c')"); + assertThat(helper.getParserError()).contains("a.b.c is not a legal workspace name"); + } + + private WorkspaceFactoryHelper parse(String... args) { + return new WorkspaceFactoryHelper(args); + } + + /** + * Parses a WORKSPACE file with the given content. + */ + private class WorkspaceFactoryHelper { + private final LegacyBuilder builder; + private final WorkspaceFactory factory; + private final Exception exception; + private final ImmutableList<Event> events; + + public WorkspaceFactoryHelper(String... args) { + Path root = null; + Path workspaceFilePath = null; + try { + Scratch scratch = new Scratch("/"); + root = scratch.dir("/workspace"); + workspaceFilePath = scratch.file("/workspace/WORKSPACE", args); + } catch (IOException e) { + fail("Shouldn't happen: " + e.getMessage()); + } + StoredEventHandler eventHandler = new StoredEventHandler(); + builder = Package.newExternalPackageBuilder(workspaceFilePath, ""); + this.factory = new WorkspaceFactory( + builder, + TestRuleClassProvider.getRuleClassProvider(), + ImmutableList.<PackageFactory.EnvironmentExtension>of(), + Mutability.create("test"), + root, + root); + Exception exception = null; + try { + factory.parse(ParserInputSource.create(workspaceFilePath), eventHandler); + } catch (IOException e) { + exception = e; + } catch (InterruptedException e) { + fail("Shouldn't happen: " + e.getMessage()); + } + this.events = eventHandler.getEvents(); + this.exception = exception; + } + + public Package getPackage() { + return builder.build(); + } + + public void assertLexingExceptionThrown() { + assertNotNull(exception); + assertThat(exception.getMessage()).contains("Failed to parse /workspace/WORKSPACE"); + } + + public String getLexerError() { + assertEquals(1, events.size()); + return events.get(0).getMessage(); + } + + public String getParserError() { + List<Event> events = builder.getEvents(); + assertThat(events.size()).isGreaterThan(0); + return events.get(0).getMessage(); } - List<Event> events = localReporter.getEvents(); - assertEquals("Incorrect size for " + events, 1, events.size()); - assertThat(events.get(0).getMessage()) - .contains("The label must reference a file with extension '.bzl'"); } } 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 71c3cf296f..3f7c0d56d3 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 @@ -163,7 +163,7 @@ public class WorkspaceFileFunctionTest extends BuildViewTestCase { (PackageValue) skyFunc.compute(PackageValue.workspaceKey(workspacePath), getEnv()); Package pkg = value.getPackage(); assertTrue(pkg.containsErrors()); - MoreAsserts.assertContainsEvent(pkg.getEvents(), "target names may not contain '$'"); + MoreAsserts.assertContainsEvent(pkg.getEvents(), "foo$ is not a legal workspace name"); } @Test diff --git a/src/test/shell/bazel/runfiles_test.sh b/src/test/shell/bazel/runfiles_test.sh index de9e951fbc..afc2c0d4b5 100755 --- a/src/test/shell/bazel/runfiles_test.sh +++ b/src/test/shell/bazel/runfiles_test.sh @@ -25,7 +25,7 @@ source $(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/test-setup.sh \ # workspace() is specified in the WORKSPACE file. function test_runfiles() { - name=blorp/malorp + name=blorp_malorp cat > WORKSPACE <<EOF workspace(name = "$name") |