Skip to content

Commit

Permalink
Stop needlessly parsing WORKSPACE files from external repositories.
Browse files Browse the repository at this point in the history
Fixes: #11936

I have an additional test to prove the spurious error message is gone and that I still can refer to a workspace by name, in https://github.com/aiuto/repro_11936/tree/more_tests
I have not added it yet because it is not really clear what the correct behavior should be for external repos - should the WORKSPACE be valid and used, or ignored. This change brings the code in line with the documentation (ignored), but the better behavior might be different.

RELNOTES: Stop needlessly parsing WORKSPACE files from external repositories.
PiperOrigin-RevId: 327157715
  • Loading branch information
aiuto authored and copybara-github committed Aug 18, 2020
1 parent 47943be commit b970667
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,8 @@

package com.google.devtools.build.lib.rules.repository;

import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.packages.WorkspaceFileValue;
import com.google.devtools.build.lib.skyframe.RepositoryValue;
import com.google.devtools.build.lib.vfs.Root;
import com.google.devtools.build.lib.vfs.RootedPath;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyFunctionException;
import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
Expand Down Expand Up @@ -47,37 +43,6 @@ public SkyValue compute(SkyKey skyKey, Environment env)
if (!repository.repositoryExists()) {
return RepositoryValue.notFound(nameFromRule);
}
RootedPath workspaceFilePath;
try {
workspaceFilePath =
WorkspaceFileHelper.getWorkspaceRootedFile(Root.fromPath(repository.getPath()), env);
if (workspaceFilePath == null) {
return null;
}
} catch (IOException e) {
throw new RepositoryLoaderFunctionException(
new IOException(
"Could not determine workspace file (\"WORKSPACE.bazel\" or \"WORKSPACE\"): "
+ e.getMessage()),
Transience.PERSISTENT);
}
SkyKey workspaceKey = WorkspaceFileValue.key(workspaceFilePath);
WorkspaceFileValue workspacePackage = (WorkspaceFileValue) env.getValue(workspaceKey);
if (workspacePackage == null) {
return null;
}

try {
String workspaceNameStr = workspacePackage.getPackage().getWorkspaceName();
if (workspaceNameStr.isEmpty()) {
RepositoryName.create("");
} else {
RepositoryName.create("@" + workspaceNameStr);
}
} catch (LabelSyntaxException e) {
throw new IllegalStateException(e);
}

return RepositoryValue.success(nameFromRule, repository);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,24 @@ public void testDoNotSymlinkInExecroot() throws Exception {
}
}

@Test
public void testMangledExternalWorkspaceFileIsIgnored() throws Exception {
scratch.file("secondary/WORKSPACE", "garbage");
RootedPath workspace =
createWorkspaceFile(
"workspace(name = 'good')",
"local_repository(name = \"secondary\", path = \"./secondary/\")");

SkyKey key1 = WorkspaceFileValue.key(workspace, 1);
EvaluationResult<WorkspaceFileValue> result1 = eval(key1);
WorkspaceFileValue value1 = result1.get(key1);
RepositoryName good = RepositoryName.create("@good");
RepositoryName main = RepositoryName.create("@");
RepositoryName secondary = RepositoryName.create("@secondary");
assertThat(value1.getRepositoryMapping()).containsEntry(secondary, ImmutableMap.of(good, main));
assertNoEvents();
}

private static class TestManagedDirectoriesKnowledge implements ManagedDirectoriesKnowledge {
private String lastWorkspaceName;
private int cnt = 0;
Expand Down

0 comments on commit b970667

Please sign in to comment.