diff options
Diffstat (limited to 'src')
4 files changed, 161 insertions, 13 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/Runfiles.java b/src/main/java/com/google/devtools/build/lib/analysis/Runfiles.java index c18bf60997..d1f1434a39 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/Runfiles.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/Runfiles.java @@ -20,6 +20,7 @@ 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.analysis.RuleConfiguredTarget.Mode; +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.collect.nestedset.Order; @@ -458,23 +459,99 @@ public final class Runfiles { // Copy manifest map to another manifest map, prepending the workspace name to every path. // E.g. for workspace "myworkspace", the runfile entry "mylib.so"->"/path/to/mylib.so" becomes // "myworkspace/mylib.so"->"/path/to/mylib.so". - Map<PathFragment, Artifact> rootManifest = new HashMap<>(); - for (Map.Entry<PathFragment, Artifact> entry : manifest.entrySet()) { - checker.put(rootManifest, suffix.getRelative(entry.getKey()), entry.getValue()); - } + ManifestBuilder builder = new ManifestBuilder(suffix, legacyExternalRunfiles); + builder.addUnderWorkspace(manifest, checker); // Finally add symlinks relative to the root of the runfiles tree, on top of everything else. // This operation is always checked for conflicts, to match historical behavior. if (conflictPolicy == ConflictPolicy.IGNORE) { checker = new ConflictChecker(ConflictPolicy.WARN, eventHandler, location); } - for (Map.Entry<PathFragment, Artifact> entry : getRootSymlinksAsMap(checker).entrySet()) { - PathFragment mappedPath = entry.getKey(); - Artifact mappedArtifact = entry.getValue(); - checker.put(rootManifest, mappedPath, mappedArtifact); + builder.add(getRootSymlinksAsMap(checker), checker); + return builder.build(); + } + + /** + * Helper class to handle munging the paths of external artifacts. + */ + @VisibleForTesting + static final class ManifestBuilder { + // Manifest of paths to artifacts. Path fragments are relative to the .runfiles directory. + private final Map<PathFragment, Artifact> manifest; + private final PathFragment workspaceName; + private final boolean legacyExternalRunfiles; + // Whether we saw the local workspace name in the runfiles. If legacyExternalRunfiles is true, + // then this is true, as anything under external/ will also have a runfile under the local + // workspace. + private boolean sawWorkspaceName; + + public ManifestBuilder( + PathFragment workspaceName, boolean legacyExternalRunfiles) { + this.manifest = new HashMap<>(); + this.workspaceName = workspaceName; + this.legacyExternalRunfiles = legacyExternalRunfiles; + this.sawWorkspaceName = legacyExternalRunfiles; } - return rootManifest; + /** + * Adds a map under the workspaceName. + */ + public void addUnderWorkspace( + Map<PathFragment, Artifact> inputManifest, ConflictChecker checker) { + for (Map.Entry<PathFragment, Artifact> entry : inputManifest.entrySet()) { + PathFragment path = entry.getKey(); + if (isUnderWorkspace(path)) { + sawWorkspaceName = true; + checker.put(manifest, workspaceName.getRelative(path), entry.getValue()); + } else { + if (legacyExternalRunfiles) { + checker.put(manifest, workspaceName.getRelative(path), entry.getValue()); + } + // Always add the non-legacy .runfiles/repo/whatever path. + checker.put(manifest, getExternalPath(path), entry.getValue()); + } + } + } + + /** + * Adds a map to the root directory. + */ + public void add(Map<PathFragment, Artifact> inputManifest, ConflictChecker checker) { + for (Map.Entry<PathFragment, Artifact> entry : inputManifest.entrySet()) { + checker.put(manifest, checkForWorkspace(entry.getKey()), entry.getValue()); + } + } + + /** + * Returns the manifest, adding the workspaceName directory if it is not already present. + */ + public Map<PathFragment, Artifact> build() { + if (!sawWorkspaceName) { + // If we haven't seen it and we have seen other files, add the workspace name directory. + // It might not be there if all of the runfiles are from other repos (and then running from + // x.runfiles/ws will fail, because ws won't exist). We can't tell Runfiles to create a + // directory, so instead this creates a hidden file inside the desired directory. + manifest.put(workspaceName.getRelative(".runfile"), null); + } + return manifest; + } + + private PathFragment getExternalPath(PathFragment path) { + return checkForWorkspace(path.relativeTo(Label.EXTERNAL_PACKAGE_NAME)); + } + + private PathFragment checkForWorkspace(PathFragment path) { + sawWorkspaceName = sawWorkspaceName || path.getSegment(0).equals(workspaceName); + return path; + } + + private static boolean isUnderWorkspace(PathFragment path) { + return !path.startsWith(Label.EXTERNAL_PACKAGE_NAME); + } + } + + boolean getLegacyExternalRunfiles() { + return legacyExternalRunfiles; } /** diff --git a/src/main/java/com/google/devtools/build/lib/analysis/SourceManifestAction.java b/src/main/java/com/google/devtools/build/lib/analysis/SourceManifestAction.java index 6d726e760d..73e8c40b7f 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/SourceManifestAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/SourceManifestAction.java @@ -192,6 +192,7 @@ public final class SourceManifestAction extends AbstractFileWriteAction { protected String computeKey() { Fingerprint f = new Fingerprint(); f.addString(GUID); + f.addBoolean(runfiles.getLegacyExternalRunfiles()); Map<PathFragment, Artifact> symlinks = runfiles.getSymlinksAsMap(null); f.addInt(symlinks.size()); for (Map.Entry<PathFragment, Artifact> symlink : symlinks.entrySet()) { diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java index b62824708c..3f8c6be31f 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java @@ -710,7 +710,7 @@ public final class BuildConfiguration { @Option(name = "legacy_external_runfiles", defaultValue = "true", - category = "undocumented", + category = "strategy", help = "If true, build runfiles symlink forests for external repositories under " + ".runfiles/wsname/external/repo (in addition to .runfiles/repo).") public boolean legacyExternalRunfiles; diff --git a/src/test/java/com/google/devtools/build/lib/analysis/RunfilesTest.java b/src/test/java/com/google/devtools/build/lib/analysis/RunfilesTest.java index 7b806f371d..1f83f28005 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/RunfilesTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/RunfilesTest.java @@ -16,10 +16,12 @@ package com.google.devtools.build.lib.analysis; import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertEquals; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; import com.google.common.collect.Maps; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.Root; +import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.events.EventKind; import com.google.devtools.build.lib.testutil.FoundationTestCase; import com.google.devtools.build.lib.vfs.PathFragment; @@ -41,7 +43,7 @@ public class RunfilesTest extends FoundationTestCase { private void checkWarning() { assertContainsEvent("obscured by a -> /workspace/a"); assertEquals("Runfiles.filterListForObscuringSymlinks should have warned once", - 1, eventCollector.count()); + 1, eventCollector.count()); assertEquals(EventKind.WARNING, Iterables.getOnlyElement(eventCollector).getKind()); } @@ -83,7 +85,7 @@ public class RunfilesTest extends FoundationTestCase { root); obscuringMap.put(pathA, artifactA); obscuringMap.put(new PathFragment("a/b"), new Artifact(new PathFragment("c/b"), - root)); + root)); assertThat(Runfiles.filterListForObscuringSymlinks(null, null, obscuringMap).entrySet()) .containsExactly(Maps.immutableEntry(pathA, artifactA)).inOrder(); } @@ -117,7 +119,7 @@ public class RunfilesTest extends FoundationTestCase { root); obscuringMap.put(pathBC, artifactBC); assertThat(Runfiles.filterListForObscuringSymlinks(reporter, null, obscuringMap) - .entrySet()).containsExactly(Maps.immutableEntry(pathA, artifactA), + .entrySet()).containsExactly(Maps.immutableEntry(pathA, artifactA), Maps.immutableEntry(pathBC, artifactBC)); assertNoEvents(); } @@ -316,4 +318,72 @@ public class RunfilesTest extends FoundationTestCase { Runfiles r4 = new Runfiles.Builder("TESTING").merge(r2).merge(r1).build(); assertEquals(Runfiles.ConflictPolicy.ERROR, r4.getConflictPolicy()); } + + @Test + public void testLegacyRunfilesStructure() { + Root root = Root.asSourceRoot(scratch.resolve("/workspace")); + PathFragment workspaceName = new PathFragment("wsname"); + PathFragment pathB = new PathFragment("external/repo/b"); + Artifact artifactB = new Artifact(pathB, root); + + Runfiles.ManifestBuilder builder = new Runfiles.ManifestBuilder(workspaceName, true); + + Map<PathFragment, Artifact> inputManifest = Maps.newHashMap(); + inputManifest.put(pathB, artifactB); + Runfiles.ConflictChecker checker = new Runfiles.ConflictChecker( + Runfiles.ConflictPolicy.WARN, reporter, null); + builder.addUnderWorkspace(inputManifest, checker); + + assertThat(builder.build().entrySet()).containsExactly( + Maps.immutableEntry(workspaceName.getRelative(pathB), artifactB), + Maps.immutableEntry(new PathFragment("repo/b"), artifactB)); + assertNoEvents(); + } + + @Test + public void testRunfileAdded() { + Root root = Root.asSourceRoot(scratch.resolve("/workspace")); + PathFragment workspaceName = new PathFragment("wsname"); + PathFragment pathB = new PathFragment("external/repo/b"); + Artifact artifactB = new Artifact(pathB, root); + + Runfiles.ManifestBuilder builder = new Runfiles.ManifestBuilder(workspaceName, false); + + Map<PathFragment, Artifact> inputManifest = ImmutableMap.<PathFragment, Artifact>builder() + .put(pathB, artifactB) + .build(); + Runfiles.ConflictChecker checker = new Runfiles.ConflictChecker( + Runfiles.ConflictPolicy.WARN, reporter, null); + builder.addUnderWorkspace(inputManifest, checker); + + assertThat(builder.build().entrySet()).containsExactly( + Maps.immutableEntry(workspaceName.getRelative(".runfile"), null), + Maps.immutableEntry(new PathFragment("repo/b"), artifactB)); + assertNoEvents(); + } + + // TODO(kchodorow): remove this once the default workspace name is always set. + @Test + public void testConflictWithExternal() { + Root root = Root.asSourceRoot(scratch.resolve("/workspace")); + PathFragment pathB = new PathFragment("repo/b"); + PathFragment externalPathB = Label.EXTERNAL_PACKAGE_NAME.getRelative(pathB); + Artifact artifactB = new Artifact(pathB, root); + Artifact artifactExternalB = new Artifact(externalPathB, root); + + Runfiles.ManifestBuilder builder = new Runfiles.ManifestBuilder( + PathFragment.EMPTY_FRAGMENT, false); + + Map<PathFragment, Artifact> inputManifest = ImmutableMap.<PathFragment, Artifact>builder() + .put(pathB, artifactB) + .put(externalPathB, artifactExternalB) + .build(); + Runfiles.ConflictChecker checker = new Runfiles.ConflictChecker( + Runfiles.ConflictPolicy.WARN, reporter, null); + builder.addUnderWorkspace(inputManifest, checker); + + assertThat(builder.build().entrySet()).containsExactly( + Maps.immutableEntry(new PathFragment("repo/b"), artifactExternalB)); + checkConflictWarning(); + } } |