Skip to content

Commit

Permalink
Allow dot ('.') in workspace names.
Browse files Browse the repository at this point in the history 
    RELNOTES: Dot ('.') is now allowed in workspace names. See bazelbuild/bazel#11837.
    PiperOrigin-RevId: 327160423
  • Loading branch information
Luca Di Grazia committed Sep 4, 2022
1 parent d230ac0 commit 514d9f4
Show file tree
Hide file tree
Showing 2 changed files with 189 additions and 103 deletions.
18 changes: 9 additions & 9 deletions ...zelbuild.bazel/src/main/java/com/google/devtools/build/lib/packages/WorkspaceGlobals.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

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

import static net.starlark.java.eval.Starlark.NONE;
import static com.google.devtools.build.lib.syntax.Starlark.NONE;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
Expand All @@ -30,6 +30,13 @@
import com.google.devtools.build.lib.packages.Package.NameConflictException;
import com.google.devtools.build.lib.packages.RuleFactory.InvalidRuleException;
import com.google.devtools.build.lib.starlarkbuildapi.WorkspaceGlobalsApi;
import com.google.devtools.build.lib.syntax.Dict;
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.syntax.Module;
import com.google.devtools.build.lib.syntax.NoneType;
import com.google.devtools.build.lib.syntax.Sequence;
import com.google.devtools.build.lib.syntax.Starlark;
import com.google.devtools.build.lib.syntax.StarlarkThread;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.List;
import java.util.Map;
Expand All @@ -38,13 +45,6 @@
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import javax.annotation.Nullable;
import net.starlark.java.eval.Dict;
import net.starlark.java.eval.EvalException;
import net.starlark.java.eval.Module;
import net.starlark.java.eval.NoneType;
import net.starlark.java.eval.Sequence;
import net.starlark.java.eval.Starlark;
import net.starlark.java.eval.StarlarkThread;

/** A collection of global Starlark build API functions that apply to WORKSPACE files. */
public class WorkspaceGlobals implements WorkspaceGlobalsApi {
Expand Down Expand Up @@ -232,7 +232,7 @@ private static RepositoryName getRepositoryName(@Nullable Label label) {
}

// registeration happened in a loaded bzl file
return label.getRepository();
return label.getPackageIdentifier().getRepository();
}

private static ImmutableList<String> renamePatterns(
Expand Down
274 changes: 180 additions & 94 deletions ...uild.bazel/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,122 +15,208 @@
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;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.StoredEventHandler;
import com.google.devtools.build.lib.packages.Package.LegacyBuilder;
import com.google.devtools.build.lib.syntax.Mutability;
import com.google.devtools.build.lib.syntax.ParserInputSource;
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 com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.testutil.Scratch;
import com.google.devtools.build.lib.vfs.Root;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;

import java.io.IOException;
import java.util.List;

/**
* Tests for WorkspaceFactory.
*/
@RunWith(JUnit4.class)
public class WorkspaceFactoryTest {

@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'");
private Scratch scratch = new Scratch();
private WorkspaceFactoryTestHelper helper;
private Root root;

@Before
public void setUp() throws Exception {
root = Root.fromPath(scratch.dir(""));
helper = new WorkspaceFactoryTestHelper(root);
}

@Test
public void testWorkspaceName() throws Exception {
WorkspaceFactoryHelper helper = parse("workspace(name = 'my_ws')");
assertEquals("my_ws", helper.getPackage().getWorkspaceName());
helper.parse("workspace(name = 'my_ws')");
assertThat(helper.getPackage().getWorkspaceName()).isEqualTo("my_ws");
}

@Test
public void testWorkspaceStartsWithNumber() throws Exception {
WorkspaceFactoryHelper helper = parse("workspace(name = '123abc')");
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();
}
helper.parse("workspace(name = 'a+b+c')");
assertThat(helper.getParserError()).contains("a+b+c is not a legal workspace name");
}

@Test
public void testIllegalRepoName() throws Exception {
helper.parse("local_repository(", " name = 'foo/bar',", " path = '/foo/bar',", ")");
assertThat(helper.getParserError()).contains(
"local_repository rule //external:foo/bar's name field must be a legal workspace name");
}

@Test
public void testIllegalWorkspaceFunctionPosition() throws Exception {
helper = new WorkspaceFactoryTestHelper(/*allowOverride=*/ false, root);
helper.parse("workspace(name = 'foo')");
assertThat(helper.getParserError()).contains(
"workspace() function should be used only at the top of the WORKSPACE file");
}

@Test
public void testRegisterExecutionPlatforms() throws Exception {
helper.parse("register_execution_platforms('//platform:ep1')");
assertThat(helper.getPackage().getRegisteredExecutionPlatforms())
.containsExactly("//platform:ep1");
}

@Test
public void testRegisterExecutionPlatforms_multipleLabels() throws Exception {
helper.parse("register_execution_platforms(", " '//platform:ep1',", " '//platform:ep2')");
assertThat(helper.getPackage().getRegisteredExecutionPlatforms())
.containsExactly("//platform:ep1", "//platform:ep2");
}

@Test
public void testRegisterExecutionPlatforms_multipleCalls() throws Exception {
helper.parse(
"register_execution_platforms('//platform:ep1')",
"register_execution_platforms('//platform:ep2')",
"register_execution_platforms('//platform/...')");
assertThat(helper.getPackage().getRegisteredExecutionPlatforms())
.containsExactly("//platform:ep1", "//platform:ep2", "//platform/...");
}

@Test
public void testRegisterToolchains() throws Exception {
helper.parse("register_toolchains('//toolchain:tc1')");
assertThat(helper.getPackage().getRegisteredToolchains()).containsExactly("//toolchain:tc1");
}

@Test
public void testRegisterToolchains_multipleLabels() throws Exception {
helper.parse("register_toolchains(", " '//toolchain:tc1',", " '//toolchain:tc2')");
assertThat(helper.getPackage().getRegisteredToolchains())
.containsExactly("//toolchain:tc1", "//toolchain:tc2");
}

@Test
public void testRegisterToolchains_multipleCalls() throws Exception {
helper.parse(
"register_toolchains('//toolchain:tc1')",
"register_toolchains('//toolchain:tc2')",
"register_toolchains('//toolchain/...')");
assertThat(helper.getPackage().getRegisteredToolchains())
.containsExactly("//toolchain:tc1", "//toolchain:tc2", "//toolchain/...");
}

@Test
public void testWorkspaceMappings() throws Exception {
helper.parse(
"workspace(name = 'bar')",
"local_repository(",
" name = 'foo',",
" path = '/foo',",
" repo_mapping = {'@x' : '@y'},",
")");
assertMapping(helper, "@foo", "@x", "@y");
}

@Test
public void testMultipleRepositoriesWithMappings() throws Exception {
helper.parse(
"local_repository(",
" name = 'foo',",
" path = '/foo',",
" repo_mapping = {'@x' : '@y'},",
")",
"local_repository(",
" name = 'bar',",
" path = '/bar',",
" repo_mapping = {'@a' : '@b'},",
")");
assertMapping(helper, "@foo", "@x", "@y");
assertMapping(helper, "@bar", "@a", "@b");
}

@Test
public void testMultipleMappings() throws Exception {
helper.parse(
"local_repository(",
" name = 'foo',",
" path = '/foo',",
" repo_mapping = {'@a' : '@b', '@c' : '@d', '@e' : '@f'},",
")");
assertMapping(helper, "@foo", "@a", "@b");
assertMapping(helper, "@foo", "@c", "@d");
assertMapping(helper, "@foo", "@e", "@f");
}

@Test
public void testEmptyMappings() throws Exception {
helper.parse(
"local_repository(",
" name = 'foo',",
" path = '/foo',",
" repo_mapping = {},",
")");
assertThat(helper.getPackage().getRepositoryMapping(RepositoryName.create("@foo"))).isEmpty();
}

@Test
public void testRepoMappingNotAStringStringDict() throws Exception {
helper.parse("local_repository(name='foo', path='/foo', repo_mapping=1)");
assertThat(helper.getParserError()).contains("got int for 'repo_mapping', want dict");

helper.parse("local_repository(name='foo', path='/foo', repo_mapping='hello')");
assertThat(helper.getParserError()).contains("got string for 'repo_mapping', want dict");

helper.parse("local_repository(name='foo', path='/foo', repo_mapping={1: 1})");
assertThat(helper.getParserError())
.contains("got dict<int, int> for 'repo_mapping', want dict<string, string>");
}

@Test
public void testImplicitMainRepoRename() throws Exception {
helper.parse("workspace(name = 'foo')");
assertMapping(helper, "@", "@foo", "@");
}

@Test
public void testEmptyRepositoryHasEmptyMap() throws Exception {
helper.parse("");
assertThat(helper.getPackage().getRepositoryMapping(RepositoryName.create("@"))).isEmpty();
}

@Test
public void testOverrideImplicitMainRepoRename() throws Exception {
helper.parse(
"workspace(name = 'bar')",
"local_repository(",
" name = 'foo',",
" path = '/foo',",
" repo_mapping = {'@x' : '@y', '@bar' : '@newname'},",
")");
assertMapping(helper, "@foo", "@x", "@y");
assertMapping(helper, "@foo", "@bar", "@newname");
}

private void assertMapping(
WorkspaceFactoryTestHelper helper, String repo, String local, String global)
throws Exception {
RepositoryName localRepoName = RepositoryName.create(local);
RepositoryName globalRepoName = RepositoryName.create(global);
assertThat(helper.getPackage().getRepositoryMapping(RepositoryName.create(repo)))
.containsEntry(localRepoName, globalRepoName);
}

}

0 comments on commit 514d9f4

Please sign in to comment.