diff options
Diffstat (limited to 'src/main/java/com/google/devtools/build')
38 files changed, 429 insertions, 169 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/actions/Artifact.java b/src/main/java/com/google/devtools/build/lib/actions/Artifact.java index f28683b451..9a132faa08 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/Artifact.java +++ b/src/main/java/com/google/devtools/build/lib/actions/Artifact.java @@ -498,6 +498,21 @@ public class Artifact } /** + * For targets in external repositories, this returns the path the artifact live at in the + * runfiles tree. For local targets, it returns the rootRelativePath. + */ + public final PathFragment getRunfilesPath() { + PathFragment relativePath = rootRelativePath; + if (relativePath.segmentCount() > 1 + && relativePath.getSegment(0).equals(Label.EXTERNAL_PATH_PREFIX)) { + // Turn external/repo/foo into ../repo/foo. + relativePath = relativePath.relativeTo(Label.EXTERNAL_PATH_PREFIX); + relativePath = new PathFragment("..").getRelative(relativePath); + } + return relativePath; + } + + /** * Returns this.getExecPath().getPathString(). */ @Override 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 5c14ec2dda..425a3eea30 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; @@ -40,7 +41,6 @@ import java.io.IOException; import java.io.InputStreamReader; import java.util.Collections; import java.util.HashMap; -import java.util.Iterator; import java.util.LinkedHashMap; import java.util.Map; import java.util.Map.Entry; @@ -139,6 +139,11 @@ public final class Runfiles { public Artifact getArtifact() { return artifact; } + + @Override + public String toString() { + return path + " -> " + artifact.getRunfilesPath(); + } } // It is important to declare this *after* the DUMMY_SYMLINK_EXPANDER to avoid NPEs @@ -151,7 +156,7 @@ public final class Runfiles { * * <p>This is either set to the workspace name, or is empty. */ - private final String suffix; + private final PathFragment suffix; /** * The artifacts that should *always* be present in the runfiles directory. These are @@ -203,7 +208,7 @@ public final class Runfiles { * * <p>If no EventHandler is available, all values are treated as IGNORE. */ - public static enum ConflictPolicy { + public enum ConflictPolicy { IGNORE, WARN, ERROR, @@ -262,13 +267,16 @@ public final class Runfiles { */ private final NestedSet<PruningManifest> pruningManifests; - private Runfiles(String suffix, + private final boolean legacyRepositoryStructure; + + private Runfiles(PathFragment suffix, NestedSet<Artifact> artifacts, NestedSet<SymlinkEntry> symlinks, NestedSet<SymlinkEntry> rootSymlinks, NestedSet<PruningManifest> pruningManifests, EmptyFilesSupplier emptyFilesSupplier, - ConflictPolicy conflictPolicy) { + ConflictPolicy conflictPolicy, + boolean legacyRepositoryStructure) { this.suffix = suffix; this.unconditionalArtifacts = Preconditions.checkNotNull(artifacts); this.symlinks = Preconditions.checkNotNull(symlinks); @@ -276,12 +284,13 @@ public final class Runfiles { this.pruningManifests = Preconditions.checkNotNull(pruningManifests); this.emptyFilesSupplier = Preconditions.checkNotNull(emptyFilesSupplier); this.conflictPolicy = conflictPolicy; + this.legacyRepositoryStructure = legacyRepositoryStructure; } /** * Returns the runfiles' suffix. */ - public String getSuffix() { + public PathFragment getSuffix() { return suffix; } @@ -352,54 +361,10 @@ public final class Runfiles { /** * Returns the symlinks as a map from path fragment to artifact. - * - * @param checker If not null, check for conflicts using this checker. */ - public Map<PathFragment, Artifact> getSymlinksAsMap(@Nullable ConflictChecker checker) { - return entriesToMap(symlinks, checker); - } - - /** - * @param eventHandler Used for throwing an error if we have an obscuring runlink. - * May be null, in which case obscuring symlinks are silently discarded. - * @param location Location for reporter. Ignored if reporter is null. - * @param workingManifest Manifest to be checked for obscuring symlinks. - * @return map of source file names mapped to their location on disk. - */ - @VisibleForTesting - static Map<PathFragment, Artifact> filterListForObscuringSymlinks( - EventHandler eventHandler, Location location, Map<PathFragment, Artifact> workingManifest) { - Map<PathFragment, Artifact> newManifest = new HashMap<>(); - - outer: - for (Iterator<Entry<PathFragment, Artifact>> i = workingManifest.entrySet().iterator(); - i.hasNext(); ) { - Entry<PathFragment, Artifact> entry = i.next(); - PathFragment source = entry.getKey(); - Artifact symlink = entry.getValue(); - // drop nested entries; warn if this changes anything - int n = source.segmentCount(); - for (int j = 1; j < n; ++j) { - PathFragment prefix = source.subFragment(0, n - j); - Artifact ancestor = workingManifest.get(prefix); - if (ancestor != null) { - // This is an obscuring symlink, so just drop it and move on if there's no reporter. - if (eventHandler == null) { - continue outer; - } - PathFragment suffix = source.subFragment(n - j, n); - Path viaAncestor = ancestor.getPath().getRelative(suffix); - Path expected = symlink.getPath(); - if (!viaAncestor.equals(expected)) { - eventHandler.handle(Event.warn(location, "runfiles symlink " + source + " -> " - + expected + " obscured by " + prefix + " -> " + ancestor.getPath())); - } - continue outer; - } - } - newManifest.put(entry.getKey(), entry.getValue()); - } - return newManifest; + public Map<PathFragment, Artifact> getSymlinksAsMap() { + return new ManifestBuilder(ConflictChecker.IGNORE_CHECKER, suffix, legacyRepositoryStructure) + .putSymlinks(symlinks).build(); } /** @@ -414,11 +379,14 @@ public final class Runfiles { */ public Map<PathFragment, Artifact> getRunfilesInputs(EventHandler eventHandler, Location location) throws IOException { - ConflictChecker checker = new ConflictChecker(conflictPolicy, eventHandler, location); - Map<PathFragment, Artifact> manifest = getSymlinksAsMap(checker); + ManifestBuilder builder = new ManifestBuilder( + new ConflictChecker(conflictPolicy, eventHandler, location), + suffix, + legacyRepositoryStructure); + builder.putSymlinks(symlinks); // Add unconditional artifacts (committed to inclusion on construction of runfiles). for (Artifact artifact : getUnconditionalArtifactsWithoutMiddlemen()) { - checker.put(manifest, artifact.getRootRelativePath(), artifact); + builder.put(RunfilesPath.resolve(artifact.getRootRelativePath(), suffix), artifact); } // Add conditional artifacts (only included if they appear in a pruning manifest). @@ -434,39 +402,26 @@ public final class Runfiles { while ((line = reader.readLine()) != null) { Artifact artifact = allowedRunfiles.get(line); if (artifact != null) { - checker.put(manifest, artifact.getRootRelativePath(), artifact); + builder.put(RunfilesPath.resolve(artifact.getRootRelativePath(), suffix), artifact); } } } } - manifest = filterListForObscuringSymlinks(eventHandler, location, manifest); + builder.filterListForObscuringSymlinks(eventHandler, location); // TODO(bazel-team): Create /dev/null-like Artifact to avoid nulls? - for (PathFragment extraPath : emptyFilesSupplier.getExtraPaths(manifest.keySet())) { - checker.put(manifest, extraPath, null); - } - - // 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". - PathFragment suffixPath = new PathFragment(suffix); - Map<PathFragment, Artifact> rootManifest = new HashMap<>(); - for (Map.Entry<PathFragment, Artifact> entry : manifest.entrySet()) { - checker.put(rootManifest, suffixPath.getRelative(entry.getKey()), entry.getValue()); + for (PathFragment extraPath : emptyFilesSupplier.getExtraPaths(builder.getPaths())) { + builder.put(RunfilesPath.alreadyResolved(extraPath, suffix), null); } // 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.resetConflictPolicy( + new ConflictChecker(ConflictPolicy.WARN, eventHandler, location)); } - - return rootManifest; + builder.putRootSymlinks(rootSymlinks); + return builder.build(); } /** @@ -482,7 +437,9 @@ public final class Runfiles { * @param checker If not null, check for conflicts using this checker. */ public Map<PathFragment, Artifact> getRootSymlinksAsMap(@Nullable ConflictChecker checker) { - return entriesToMap(rootSymlinks, checker); + return new ManifestBuilder(checker, suffix, legacyRepositoryStructure) + .putRootSymlinks(rootSymlinks) + .buildWithoutDummyFile(); } /** @@ -490,14 +447,15 @@ public final class Runfiles { * account. */ public Map<PathFragment, Artifact> asMapWithoutRootSymlinks() { - Map<PathFragment, Artifact> result = entriesToMap(symlinks, null); + ManifestBuilder builder = new ManifestBuilder( + ConflictChecker.IGNORE_CHECKER, suffix, legacyRepositoryStructure).putSymlinks(symlinks); // If multiple artifacts have the same root-relative path, the last one in the list will win. // That is because the runfiles tree cannot contain the same artifact for different // configurations, because it only uses root-relative paths. for (Artifact artifact : Iterables.filter(unconditionalArtifacts, Artifact.MIDDLEMAN_FILTER)) { - result.put(artifact.getRootRelativePath(), artifact); + builder.put(RunfilesPath.resolve(artifact.getRootRelativePath(), suffix), artifact); } - return result; + return builder.build(); } /** @@ -542,24 +500,6 @@ public final class Runfiles { pruningManifests.isEmpty(); } - /** - * Flatten a sequence of entries into a single map. - * - * @param entrySet Sequence of entries to add. - * @param checker If not null, check for conflicts with this checker, otherwise silently allow - * entries to overwrite previous entries. - * @return Map<PathFragment, Artifact> Map of runfile entries. - */ - private static Map<PathFragment, Artifact> entriesToMap( - Iterable<SymlinkEntry> entrySet, @Nullable ConflictChecker checker) { - checker = (checker != null) ? checker : ConflictChecker.IGNORE_CHECKER; - Map<PathFragment, Artifact> map = new LinkedHashMap<>(); - for (SymlinkEntry entry : entrySet) { - checker.put(map, entry.getPath(), entry.getArtifact()); - } - return map; - } - /** Returns currently policy for conflicting symlink entries. */ public ConflictPolicy getConflictPolicy() { return this.conflictPolicy; @@ -572,6 +512,189 @@ public final class Runfiles { } /** + * Helper class to make sure that every path added to runfiles is relative to the root of the + * runfiles tree. + */ + @VisibleForTesting + static class RunfilesPath { + + private final PathFragment path; + private final boolean external; + + public static RunfilesPath resolve(PathFragment path, PathFragment workspaceName) { + return new RunfilesPath(makeRelativeToRunfilesDir(path, workspaceName), workspaceName); + } + + public static RunfilesPath alreadyResolved(PathFragment path, PathFragment workspaceName) { + return new RunfilesPath(path, workspaceName); + } + + private RunfilesPath(PathFragment path, PathFragment workspaceName) { + this.path = path; + this.external = path.segmentCount() > 1 && !path.startsWith(workspaceName); + } + + public PathFragment getPath() { + return path; + } + + /** + * Returns if this file is from an external repository. + */ + public boolean isExternal() { + return external; + } + + /** + * This takes an execution-root-relative path and turns it into a runfiles-relative path. For + * paths in the current repository, it prefixes them with the workspace name. For paths in + * external repositories, it turns the execution root path (external/repo-name/foo) into a + * runfiles path (repo-name/foo). + */ + private static PathFragment makeRelativeToRunfilesDir( + PathFragment path, PathFragment mainWorkspace) { + if (path.getSegment(0).equals(Label.EXTERNAL_PATH_PREFIX)) { + path = path.relativeTo(Label.EXTERNAL_PACKAGE_NAME); + } else { + path = mainWorkspace.getRelative(path); + } + return path; + } + } + + /** + * A builder to handle the logic of creating a manifest mapping. + */ + @VisibleForTesting + static final class ManifestBuilder { + private final PathFragment workspaceName; + private final boolean legacyRunfilesStructure; + + private Map<PathFragment, Artifact> map; + private ConflictChecker checker; + private boolean sawWorkspaceName; + + ManifestBuilder( + ConflictChecker checker, PathFragment workspaceName, boolean legacyRepositoryStructure) { + this.workspaceName = workspaceName; + this.legacyRunfilesStructure = legacyRepositoryStructure; + this.map = new LinkedHashMap<>(); + this.checker = checker == null ? ConflictChecker.IGNORE_CHECKER : checker; + this.sawWorkspaceName = false; + } + + public void resetConflictPolicy(ConflictChecker checker) { + this.checker = checker; + } + + public ManifestBuilder put(RunfilesPath runfilesPath, Artifact artifact) { + checker.check(map, runfilesPath, artifact); + PathFragment path = runfilesPath.getPath(); + if (path.startsWith(workspaceName)) { + sawWorkspaceName = true; + } + if (runfilesPath.isExternal() && legacyRunfilesStructure) { + // Store runfiles at both .runfiles/wsname/external/foo and .runfiles/foo, to allow people + // time to migrate to the second form. + map.put( + workspaceName.getRelative(Label.EXTERNAL_PACKAGE_NAME).getRelative(path), artifact); + } + map.put(path, artifact); + return this; + } + + /** + * Flatten a sequence of entries into a single map. + * + * @param symlinks Sequence of entries to add. + */ + public ManifestBuilder putSymlinks(NestedSet<SymlinkEntry> symlinks) { + for (SymlinkEntry entry : symlinks) { + put(RunfilesPath.resolve(entry.getPath(), workspaceName), entry.getArtifact()); + } + return this; + } + + /** + * Flatten a sequence of entries into a single map. Symlink entries are relative to .runfiles, + * not .runfiles/wsname, so it's assumed that external workspace entries are already resolved. + * + * @param symlinks Sequence of entries to add. + */ + public ManifestBuilder putRootSymlinks(NestedSet<SymlinkEntry> symlinks) { + for (SymlinkEntry entry : symlinks) { + put(RunfilesPath.alreadyResolved(entry.getPath(), workspaceName), entry.getArtifact()); + } + return this; + } + + /** + * This destroys the existing listing and replaces it with one that has no obscuring symlinks. + * + * @param eventHandler Used for throwing an error if we have an obscuring runlink. + * May be null, in which case obscuring symlinks are silently discarded. + * @param location Location for reporter. Ignored if reporter is null. + */ + @VisibleForTesting + ManifestBuilder filterListForObscuringSymlinks(EventHandler eventHandler, Location location) { + Map<PathFragment, Artifact> newManifest = new HashMap<>(); + + outer: + for (Entry<PathFragment, Artifact> entry : map.entrySet()) { + PathFragment source = entry.getKey(); + Artifact symlink = entry.getValue(); + // drop nested entries; warn if this changes anything + int n = source.segmentCount(); + for (int j = 1; j < n; ++j) { + PathFragment prefix = source.subFragment(0, n - j); + Artifact ancestor = map.get(prefix); + if (ancestor != null) { + // This is an obscuring symlink, so just drop it and move on if there's no reporter. + if (eventHandler == null) { + continue outer; + } + PathFragment suffixPath = source.subFragment(n - j, n); + Path viaAncestor = ancestor.getPath().getRelative(suffixPath); + Path expected = symlink.getPath(); + if (!viaAncestor.equals(expected)) { + eventHandler.handle(Event.warn(location, "runfiles symlink " + source + " -> " + + expected + " obscured by " + prefix + " -> " + ancestor.getPath())); + } + continue outer; + } + } + newManifest.put(entry.getKey(), entry.getValue()); + } + + map = newManifest; + return this; + } + + public Set<PathFragment> getPaths() { + return map.keySet(); + } + + /** + * Returns the map without checking if the main repository's directory needs to be added to + * the runfiles tree. + */ + public Map<PathFragment, Artifact> buildWithoutDummyFile() { + return map; + } + + public Map<PathFragment, Artifact> build() { + if (!sawWorkspaceName && !map.isEmpty()) { + // 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. + map.put(workspaceName.getRelative(".runfile"), null); + } + return map; + } + } + + /** * Checks for conflicts between entries in a runfiles tree while putting them in a map. */ public static final class ConflictChecker { @@ -606,11 +729,11 @@ public final class Runfiles { /** * Add an entry to a Map of symlinks, optionally reporting conflicts. * - * @param map Manifest of runfile entries. - * @param path Path fragment to use as key in map. + * @param runfilesPath Path relative to the .runfiles directory, used as key in map. * @param artifact Artifact to store in map. This may be null to indicate an empty file. */ - public void put(Map<PathFragment, Artifact> map, PathFragment path, Artifact artifact) { + void check(Map<PathFragment, Artifact> map, RunfilesPath runfilesPath, Artifact artifact) { + PathFragment path = runfilesPath.getPath(); if (policy != ConflictPolicy.IGNORE && map.containsKey(path)) { // Previous and new entry might have value of null Artifact previous = map.get(path); @@ -626,7 +749,6 @@ public final class Runfiles { eventHandler.handle(new Event(eventKind, location, message)); } } - map.put(path, artifact); } } @@ -636,7 +758,13 @@ public final class Runfiles { public static final class Builder { /** This is set to the workspace name */ - private String suffix; + private PathFragment suffix; + /** + * If external runfiles should be under .runfiles/wsname/external/repo (in addition to + * .runfiles/repo). + * TODO(kchodorow): remove this once the old form is deprecated. + */ + private final boolean legacyRepositoryStructure; /** * This must be COMPILE_ORDER because {@link #asMapWithoutRootSymlinks} overwrites earlier @@ -659,15 +787,29 @@ public final class Runfiles { * Only used for Runfiles.EMPTY. */ private Builder() { - this.suffix = ""; + this.suffix = PathFragment.EMPTY_FRAGMENT; + this.legacyRepositoryStructure = false; } /** - * Creates a builder with the given suffix. + * Creates a builder with the given suffix. Transitional constructor so that new rules don't + * accidentally depend on the legacy repository structure, until that option is removed. + * * @param workspace is the string specified in workspace() in the WORKSPACE file. */ public Builder(String workspace) { - this.suffix = workspace; + this(workspace, false); + } + + /** + * Creates a builder with the given suffix. + * @param workspace is the string specified in workspace() in the WORKSPACE file. + * @param legacyRepositoryStructure if the wsname/external/repo symlinks should also be + * created. + */ + public Builder(String workspace, boolean legacyRepositoryStructure) { + this.suffix = new PathFragment(workspace); + this.legacyRepositoryStructure = legacyRepositoryStructure; } /** @@ -676,7 +818,7 @@ public final class Runfiles { public Runfiles build() { return new Runfiles(suffix, artifactsBuilder.build(), symlinksBuilder.build(), rootSymlinksBuilder.build(), pruningManifestsBuilder.build(), - emptyFilesSupplier, conflictPolicy); + emptyFilesSupplier, conflictPolicy, legacyRepositoryStructure); } /** diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java b/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java index 07282a2bd0..513bc869d6 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java @@ -97,7 +97,8 @@ public class RunfilesSupport { && TargetUtils.isTestRule(ruleContext.getRule())) { TransitiveInfoCollection runUnderTarget = ruleContext.getPrerequisite(":run_under", Mode.DATA); - runfiles = new Runfiles.Builder(ruleContext.getWorkspaceName()) + runfiles = new Runfiles.Builder( + ruleContext.getWorkspaceName(), ruleContext.getConfiguration().legacyExternalRunfiles()) .merge(getRunfiles(runUnderTarget)) .merge(runfiles) .build(); @@ -233,6 +234,13 @@ public class RunfilesSupport { } /** + * Returns the name of the workspace that the build is occurring in. + */ + public PathFragment getWorkspaceName() { + return runfiles.getSuffix(); + } + + /** * Returns the middleman artifact that depends on getExecutable(), * getRunfilesManifest(), and getRunfilesSymlinkTargets(). Anything which * needs to actually run the executable should depend on this. 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 f1e633967f..513fdc4e21 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,17 +192,21 @@ public final class SourceManifestAction extends AbstractFileWriteAction { protected String computeKey() { Fingerprint f = new Fingerprint(); f.addString(GUID); - Map<PathFragment, Artifact> symlinks = runfiles.getSymlinksAsMap(null); + Map<PathFragment, Artifact> symlinks = runfiles.getSymlinksAsMap(); f.addInt(symlinks.size()); for (Map.Entry<PathFragment, Artifact> symlink : symlinks.entrySet()) { f.addPath(symlink.getKey()); - f.addPath(symlink.getValue().getPath()); + if (symlink.getValue() != null) { + f.addPath(symlink.getValue().getPath()); + } } Map<PathFragment, Artifact> rootSymlinks = runfiles.getRootSymlinksAsMap(null); f.addInt(rootSymlinks.size()); for (Map.Entry<PathFragment, Artifact> rootSymlink : rootSymlinks.entrySet()) { f.addPath(rootSymlink.getKey()); - f.addPath(rootSymlink.getValue().getPath()); + if (rootSymlink.getValue() != null) { + f.addPath(rootSymlink.getValue().getPath()); + } } for (Artifact artifact : runfiles.getArtifactsWithoutMiddlemen()) { @@ -294,16 +298,18 @@ public final class SourceManifestAction extends AbstractFileWriteAction { private final Artifact output; private final Runfiles.Builder runfilesBuilder; - public Builder(String prefix, ManifestType manifestType, ActionOwner owner, Artifact output) { - this.runfilesBuilder = new Runfiles.Builder(prefix); + public Builder( + String prefix, ManifestType manifestType, ActionOwner owner, Artifact output, + boolean legacyExternalRunfiles) { + this.runfilesBuilder = new Runfiles.Builder(prefix, legacyExternalRunfiles); manifestWriter = manifestType; this.owner = owner; this.output = output; } - @VisibleForTesting + @VisibleForTesting // Only used for testing. Builder(String prefix, ManifestWriter manifestWriter, ActionOwner owner, Artifact output) { - this.runfilesBuilder = new Runfiles.Builder(prefix); + this.runfilesBuilder = new Runfiles.Builder(prefix, false); this.manifestWriter = manifestWriter; this.owner = owner; this.output = output; 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 fd7671d06e..67e34abefc 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 @@ -708,6 +708,13 @@ public final class BuildConfiguration { + "If false, write only manifests when possible.") public boolean buildRunfiles; + @Option(name = "legacy_external_runfiles", + defaultValue = "true", + 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; + @Option(name = "test_arg", allowMultiple = true, defaultValue = "", @@ -2180,6 +2187,13 @@ public final class BuildConfiguration { return options.buildRunfiles; } + /** + * Returns if we are building external runfiles symlinks using the old-style structure. + */ + public boolean legacyExternalRunfiles() { + return options.legacyExternalRunfiles; + } + public boolean getCheckFilesetDependenciesRecursively() { return options.checkFilesetDependenciesRecursively; } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java index ef0bca705c..216f013081 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java @@ -245,7 +245,7 @@ public class BazelRuleClassProvider { .addBuildInfoFactory(new ObjcBuildInfoFactory()) .setConfigurationCollectionFactory(new BazelConfigurationCollection()) .setPrelude("//tools/build_rules:prelude_bazel") - .setRunfilesPrefix("") + .setRunfilesPrefix("__main__") .setToolsRepository("@bazel_tools") .setPrerequisiteValidator(new BazelPrerequisiteValidator()) .setSkylarkAccessibleJavaClasses(skylarkBuiltinJavaObects); diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRule.java index 119eb7838c..1448aee2ce 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRule.java @@ -149,7 +149,10 @@ public class GenRule implements RuleConfiguredTargetFactory { // No need to visit the dependencies of a genrule. They cross from the target into the host // configuration, because the dependencies of a genrule are always built for the host // configuration. - new Runfiles.Builder(ruleContext.getWorkspaceName()).addTransitiveArtifacts(filesToBuild) + new Runfiles.Builder( + ruleContext.getWorkspaceName(), + ruleContext.getConfiguration().legacyExternalRunfiles()) + .addTransitiveArtifacts(filesToBuild) .build()); return new RuleConfiguredTargetBuilder(ruleContext) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java index fae681b68e..4e8513a1e1 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java @@ -204,7 +204,7 @@ public class BazelJavaSemantics implements JavaSemantics { buffer.append(delimiter); } buffer.append("${RUNPATH}"); - buffer.append(artifact.getRootRelativePath().getPathString()); + buffer.append(artifact.getRunfilesPath().getPathString()); } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java index 5b72cbd473..f82c9fef76 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java @@ -72,7 +72,11 @@ public class BazelPythonSemantics implements PythonSemantics { @Override public List<PathFragment> getImports(RuleContext ruleContext) { List<PathFragment> result = new ArrayList<>(); - PathFragment packageFragment = ruleContext.getLabel().getPackageIdentifier().getPathFragment(); + PathFragment packageFragment = ruleContext.getLabel().getPackageIdentifier().getRunfilesPath(); + // Python scripts start with x.runfiles/ as the module space, so everything must be manually + // adjusted to be relative to the workspace name. + packageFragment = new PathFragment(ruleContext.getWorkspaceName()) + .getRelative(packageFragment); for (String importsAttr : ruleContext.attributes().get("imports", Type.STRING_LIST)) { importsAttr = ruleContext.expandMakeVariables("includes", importsAttr); if (importsAttr.startsWith("/")) { @@ -83,7 +87,7 @@ public class BazelPythonSemantics implements PythonSemantics { PathFragment importsPath = packageFragment.getRelative(importsAttr).normalize(); if (!importsPath.isNormalized()) { ruleContext.attributeError("imports", - "Path references a path above the execution root."); + "Path " + importsAttr + " references a path above the execution root"); } result.add(importsPath); } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/stub_template.txt b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/stub_template.txt index bdce83eebe..29e373c2ac 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/stub_template.txt +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/stub_template.txt @@ -73,18 +73,11 @@ def Main(): sys.argv[0]) python_imports = '%imports%' - module_space_with_workspace_name = module_space - if '%workspace_name%' != '': - module_space_with_workspace_name = os.path.join(module_space, '%workspace_name%') - - python_path_entries = CreatePythonPathEntries( - python_imports, module_space_with_workspace_name) - - external_dir = os.path.join(module_space_with_workspace_name, 'external') - if os.path.isdir(external_dir): - external_entries = [os.path.join(external_dir, d) for d in os.listdir(external_dir)] - repositories = [d for d in external_entries if os.path.isdir(d)] - python_path_entries += repositories + python_path_entries = CreatePythonPathEntries(python_imports, module_space) + + repo_dirs = [os.path.join(module_space, d) for d in os.listdir(module_space)] + repositories = [d for d in repo_dirs if os.path.isdir(d)] + python_path_entries += repositories old_python_path = os.environ.get('PYTHONPATH') separator = ';' if IsWindows() else ':' diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/ShBinary.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/ShBinary.java index 5bd542eb64..cd2a48ee06 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/ShBinary.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/ShBinary.java @@ -56,7 +56,8 @@ public class ShBinary implements RuleConfiguredTargetFactory { .add(src) .add(symlink) .build(); - Runfiles runfiles = new Runfiles.Builder(ruleContext.getWorkspaceName()) + Runfiles runfiles = new Runfiles.Builder( + ruleContext.getWorkspaceName(), ruleContext.getConfiguration().legacyExternalRunfiles()) .addTransitiveArtifacts(filesToBuild) .addRunfiles(ruleContext, RunfilesProvider.DEFAULT_RUNFILES) .build(); diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/ShLibrary.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/ShLibrary.java index 496917775b..9e94d4591a 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/ShLibrary.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/ShLibrary.java @@ -36,7 +36,8 @@ public class ShLibrary implements RuleConfiguredTargetFactory { .addAll(ruleContext.getPrerequisiteArtifacts("deps", Mode.TARGET).list()) .addAll(ruleContext.getPrerequisiteArtifacts("data", Mode.DATA).list()) .build(); - Runfiles runfiles = new Runfiles.Builder(ruleContext.getWorkspaceName()) + Runfiles runfiles = new Runfiles.Builder( + ruleContext.getWorkspaceName(), ruleContext.getConfiguration().legacyExternalRunfiles()) .addTransitiveArtifacts(filesToBuild) .build(); return new RuleConfiguredTargetBuilder(ruleContext) diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java b/src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java index 021845cbc6..2fa4494192 100644 --- a/src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java +++ b/src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java @@ -129,6 +129,14 @@ public final class PackageIdentifier implements Comparable<PackageIdentifier>, S return repository.getPathFragment().getRelative(pkgName); } + /** + * Returns the runfiles path for this repository (relative to the x.runfiles/main-repo/ + * directory). + */ + public PathFragment getRunfilesPath() { + return getRepository().getRunfilesPath().getRelative(getPackageFragment()); + } + public PackageIdentifier makeAbsolute() { if (!repository.isDefault()) { return this; diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/RepositoryName.java b/src/main/java/com/google/devtools/build/lib/cmdline/RepositoryName.java index 9a4e5a7ca1..6b89a6ee7a 100644 --- a/src/main/java/com/google/devtools/build/lib/cmdline/RepositoryName.java +++ b/src/main/java/com/google/devtools/build/lib/cmdline/RepositoryName.java @@ -202,6 +202,16 @@ public final class RepositoryName implements Serializable { } /** + * Returns the runfiles path for this repository (relative to the x.runfiles/main-repo/ + * directory). If we don't know the name of this repo (i.e., it is in the main repository), + * return an empty path fragment. + */ + public PathFragment getRunfilesPath() { + return isDefault() || isMain() + ? PathFragment.EMPTY_FRAGMENT : new PathFragment("..").getRelative(strippedName()); + } + + /** * Returns the repository name, with leading "{@literal @}" (or "" for the default repository). */ @Override diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleConfiguredTargetBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleConfiguredTargetBuilder.java index db0f9709f6..263fc49468 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleConfiguredTargetBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleConfiguredTargetBuilder.java @@ -334,7 +334,9 @@ public final class SkylarkRuleConfiguredTargetBuilder { if (executable == null) { return runfiles; } - return new Runfiles.Builder(ruleContext.getWorkspaceName()).addArtifact(executable) + return new Runfiles.Builder( + ruleContext.getWorkspaceName(), ruleContext.getConfiguration().legacyExternalRunfiles()) + .addArtifact(executable) .merge(runfiles).build(); } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleImplementationFunctions.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleImplementationFunctions.java index a0e00ff62d..44f637f3e7 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleImplementationFunctions.java +++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleImplementationFunctions.java @@ -563,7 +563,9 @@ public class SkylarkRuleImplementationFunctions { Boolean collectData, Boolean collectDefault, SkylarkDict<?, ?> symlinks, SkylarkDict<?, ?> rootSymlinks, Location loc) throws EvalException, ConversionException { - Runfiles.Builder builder = new Runfiles.Builder(ctx.getRuleContext().getWorkspaceName()); + Runfiles.Builder builder = new Runfiles.Builder( + ctx.getRuleContext().getWorkspaceName(), + ctx.getConfiguration().legacyExternalRunfiles()); boolean checkConflicts = false; if (EvalUtils.toBoolean(collectData)) { builder.addRunfiles(ctx.getRuleContext(), RunfilesProvider.DATA_RUNFILES); diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java index 8c9fc5f2e0..0e668560f6 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java @@ -621,7 +621,9 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { .add( RunfilesProvider.class, RunfilesProvider.simple( - new Runfiles.Builder(ruleContext.getWorkspaceName()) + new Runfiles.Builder( + ruleContext.getWorkspaceName(), + ruleContext.getConfiguration().legacyExternalRunfiles()) .addRunfiles(ruleContext, RunfilesProvider.DEFAULT_RUNFILES) .addTransitiveArtifacts(filesToBuild) .build())) diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java index a1bd85521c..80606ce237 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java @@ -702,7 +702,8 @@ public class AndroidCommon { private Runfiles getRunfiles() { // TODO(bazel-team): why return any Runfiles in the neverlink case? if (asNeverLink) { - return new Runfiles.Builder(ruleContext.getWorkspaceName()) + return new Runfiles.Builder( + ruleContext.getWorkspaceName(), ruleContext.getConfiguration().legacyExternalRunfiles()) .addRunfiles(ruleContext, RunfilesProvider.DEFAULT_RUNFILES) .build(); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/NativeLibs.java b/src/main/java/com/google/devtools/build/lib/rules/android/NativeLibs.java index dbcf0bfa98..dd0df2245a 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/NativeLibs.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/NativeLibs.java @@ -149,7 +149,7 @@ public final class NativeLibs { Artifact inputManifest = AndroidBinary.getDxArtifact(ruleContext, "native_symlinks.manifest"); ruleContext.registerAction(new SourceManifestAction.Builder( ruleContext.getWorkspaceName(), ManifestType.SOURCE_SYMLINKS, ruleContext.getActionOwner(), - inputManifest) + inputManifest, ruleContext.getConfiguration().legacyExternalRunfiles()) .addRootSymlinks(symlinks) .build()); Artifact outputManifest = AndroidBinary.getDxArtifact(ruleContext, "native_symlinks/MANIFEST"); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java index eda0dfbc73..7b5e4a3ba7 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java @@ -97,7 +97,8 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { Iterable<Artifact> fakeLinkerInputs, boolean fake, ImmutableList<Pair<Artifact, Label>> cAndCppSources) { - Runfiles.Builder builder = new Runfiles.Builder(context.getWorkspaceName()); + Runfiles.Builder builder = new Runfiles.Builder( + context.getWorkspaceName(), context.getConfiguration().legacyExternalRunfiles()); Function<TransitiveInfoCollection, Runfiles> runfilesMapping = CppRunfilesProvider.runfilesFunction(linkStaticness != LinkStaticness.DYNAMIC); boolean linkshared = isLinkShared(context); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java index fe6428b1b7..d6a34ca5d2 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java @@ -75,7 +75,8 @@ public abstract class CcLibrary implements RuleConfiguredTargetFactory { CcLinkingOutputs ccLinkingOutputs, boolean neverLink, boolean addDynamicRuntimeInputArtifactsToRunfiles, boolean linkingStatically) { - Runfiles.Builder builder = new Runfiles.Builder(context.getWorkspaceName()); + Runfiles.Builder builder = new Runfiles.Builder( + context.getWorkspaceName(), context.getConfiguration().legacyExternalRunfiles()); // neverlink= true creates a library that will never be linked into any binary that depends on // it, but instead be loaded as an extension. So we need the dynamic library for this in the diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java index dc7f035353..6fdcc36f29 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java @@ -1037,7 +1037,8 @@ public final class CcLibraryHelper { private Runfiles collectCppRunfiles( CcLinkingOutputs ccLinkingOutputs, boolean linkingStatically) { - Runfiles.Builder builder = new Runfiles.Builder(ruleContext.getWorkspaceName()); + Runfiles.Builder builder = new Runfiles.Builder( + ruleContext.getWorkspaceName(), ruleContext.getConfiguration().legacyExternalRunfiles()); builder.addTargets(implementationDeps, RunfilesProvider.DEFAULT_RUNFILES); builder.addTargets(implementationDeps, CppRunfilesProvider.runfilesFunction(linkingStatically)); // Add the shared libraries to the runfiles. diff --git a/src/main/java/com/google/devtools/build/lib/rules/filegroup/Filegroup.java b/src/main/java/com/google/devtools/build/lib/rules/filegroup/Filegroup.java index b42a918fc8..fb4d990fcf 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/filegroup/Filegroup.java +++ b/src/main/java/com/google/devtools/build/lib/rules/filegroup/Filegroup.java @@ -56,11 +56,16 @@ public class Filegroup implements RuleConfiguredTargetFactory { InstrumentedFilesCollector.NO_METADATA_COLLECTOR, filesToBuild); RunfilesProvider runfilesProvider = RunfilesProvider.withData( - new Runfiles.Builder(ruleContext.getWorkspaceName()) + new Runfiles.Builder( + ruleContext.getWorkspaceName(), + ruleContext.getConfiguration().legacyExternalRunfiles()) .addRunfiles(ruleContext, RunfilesProvider.DEFAULT_RUNFILES) .build(), // If you're visiting a filegroup as data, then we also visit its data as data. - new Runfiles.Builder(ruleContext.getWorkspaceName()).addTransitiveArtifacts(filesToBuild) + new Runfiles.Builder( + ruleContext.getWorkspaceName(), + ruleContext.getConfiguration().legacyExternalRunfiles()) + .addTransitiveArtifacts(filesToBuild) .addDataDeps(ruleContext).build()); return new RuleConfiguredTargetBuilder(ruleContext) diff --git a/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java b/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java index 3f2cd64703..3e4bfbd349 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java +++ b/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java @@ -157,7 +157,9 @@ public class GenQuery implements RuleConfiguredTargetFactory { return new RuleConfiguredTargetBuilder(ruleContext) .setFilesToBuild(filesToBuild) .add(RunfilesProvider.class, RunfilesProvider.simple( - new Runfiles.Builder(ruleContext.getWorkspaceName()) + new Runfiles.Builder( + ruleContext.getWorkspaceName(), + ruleContext.getConfiguration().legacyExternalRunfiles()) .addTransitiveArtifacts(filesToBuild).build())) .build(); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java index 79ae9bb656..2a0560ef65 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java @@ -66,7 +66,8 @@ public class JavaBinary implements RuleConfiguredTargetFactory { public ConfiguredTarget create(RuleContext ruleContext) throws InterruptedException { final JavaCommon common = new JavaCommon(ruleContext, semantics); DeployArchiveBuilder deployArchiveBuilder = new DeployArchiveBuilder(semantics, ruleContext); - Runfiles.Builder runfilesBuilder = new Runfiles.Builder(ruleContext.getWorkspaceName()); + Runfiles.Builder runfilesBuilder = new Runfiles.Builder( + ruleContext.getWorkspaceName(), ruleContext.getConfiguration().legacyExternalRunfiles()); List<String> jvmFlags = new ArrayList<>(); JavaTargetAttributes.Builder attributesBuilder = common.initCommon(); @@ -266,7 +267,11 @@ public class JavaBinary implements RuleConfiguredTargetFactory { RunfilesProvider runfilesProvider = RunfilesProvider.withData( defaultRunfiles, - new Runfiles.Builder(ruleContext.getWorkspaceName()).merge(runfilesSupport).build()); + new Runfiles.Builder( + ruleContext.getWorkspaceName(), + ruleContext.getConfiguration().legacyExternalRunfiles()) + .merge(runfilesSupport) + .build()); ImmutableList<String> deployManifestLines = getDeployManifestLines(ruleContext, originalMainClass); diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java index 8a9cff0259..cd9378ca36 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java @@ -459,7 +459,7 @@ public class JavaCommon { if (launcher != null) { javaExecutable = launcher.getRootRelativePath(); } else { - javaExecutable = ruleContext.getFragment(Jvm.class).getJavaExecutable(); + javaExecutable = ruleContext.getFragment(Jvm.class).getRunfilesJavaExecutable(); } String pathPrefix = javaExecutable.isAbsolute() ? "" : "${JAVA_RUNFILES}/" @@ -718,7 +718,8 @@ public class JavaCommon { if (neverLink) { return Runfiles.EMPTY; } - Runfiles.Builder runfilesBuilder = new Runfiles.Builder(ruleContext.getWorkspaceName()) + Runfiles.Builder runfilesBuilder = new Runfiles.Builder( + ruleContext.getWorkspaceName(), ruleContext.getConfiguration().legacyExternalRunfiles()) .addArtifacts(javaArtifacts.getRuntimeJars()); runfilesBuilder.addRunfiles(ruleContext, RunfilesProvider.DEFAULT_RUNFILES); runfilesBuilder.add(ruleContext, JavaRunfilesProvider.TO_RUNFILES); diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaImport.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaImport.java index 46e4d815e2..b6ac48ec1a 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaImport.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaImport.java @@ -94,7 +94,9 @@ public class JavaImport implements RuleConfiguredTargetFactory { // runfiles from this target or its dependencies. Runfiles runfiles = neverLink ? Runfiles.EMPTY : - new Runfiles.Builder(ruleContext.getWorkspaceName()) + new Runfiles.Builder( + ruleContext.getWorkspaceName(), + ruleContext.getConfiguration().legacyExternalRunfiles()) // add the jars to the runfiles .addArtifacts(javaArtifacts.getRuntimeJars()) .addTargets(targets, RunfilesProvider.DEFAULT_RUNFILES) diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaLibraryHelper.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaLibraryHelper.java index e66847e422..8b65c0436f 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaLibraryHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaLibraryHelper.java @@ -371,7 +371,8 @@ public final class JavaLibraryHelper { private JavaRunfilesProvider collectJavaRunfiles( JavaCompilationArtifacts javaCompilationArtifacts) { - Runfiles runfiles = new Runfiles.Builder(ruleContext.getWorkspaceName()) + Runfiles runfiles = new Runfiles.Builder( + ruleContext.getWorkspaceName(), ruleContext.getConfiguration().legacyExternalRunfiles()) // Compiled templates as well, for API. .addArtifacts(javaCompilationArtifacts.getRuntimeJars()) .addTargets(deps, JavaRunfilesProvider.TO_RUNFILES) diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/Jvm.java b/src/main/java/com/google/devtools/build/lib/rules/java/Jvm.java index e24266db5f..edecece6aa 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/Jvm.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/Jvm.java @@ -39,6 +39,10 @@ public final class Jvm extends BuildConfiguration.Fragment { private final PathFragment jar; private final PathFragment java; + private static final String BIN_JAVAC = "bin/javac" + OsUtils.executableExtension(); + private static final String BIN_JAR = "bin/jar" + OsUtils.executableExtension(); + private static final String BIN_JAVA = "bin/java" + OsUtils.executableExtension(); + /** * Creates a Jvm instance. Either the {@code javaHome} parameter is absolute, * or the {@code jvmLabel} parameter must be non-null. This restriction might @@ -48,9 +52,9 @@ public final class Jvm extends BuildConfiguration.Fragment { Preconditions.checkArgument(javaHome.isAbsolute() ^ (jvmLabel != null)); this.javaHome = javaHome; this.jvmLabel = jvmLabel; - this.javac = getJavaHome().getRelative("bin/javac" + OsUtils.executableExtension()); - this.jar = getJavaHome().getRelative("bin/jar" + OsUtils.executableExtension()); - this.java = getJavaHome().getRelative("bin/java" + OsUtils.executableExtension()); + this.javac = getJavaHome().getRelative(BIN_JAVAC); + this.jar = getJavaHome().getRelative(BIN_JAR); + this.java = getJavaHome().getRelative(BIN_JAVA); } /** @@ -95,6 +99,17 @@ public final class Jvm extends BuildConfiguration.Fragment { return jvmLabel; } + /** + * If possible, resolves java relative to the jvmLabel's repository. Otherwise, returns the + * same thing as getJavaExecutable(). + */ + public PathFragment getRunfilesJavaExecutable() { + if (jvmLabel == null || jvmLabel.getPackageIdentifier().getRepository().isMain()) { + return getJavaExecutable(); + } + return jvmLabel.getPackageIdentifier().getRepository().getRunfilesPath().getRelative(BIN_JAVA); + } + @Override public void addGlobalMakeVariables(Builder<String, String> globalMakeEnvBuilder) { globalMakeEnvBuilder.put("JAVABASE", getJavaHome().getPathString()); diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/IosTest.java b/src/main/java/com/google/devtools/build/lib/rules/objc/IosTest.java index 06d6d778ef..f5ca1a6da7 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/IosTest.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/IosTest.java @@ -165,7 +165,9 @@ public final class IosTest implements RuleConfiguredTargetFactory { NestedSet<Artifact> filesToBuildSet = filesToBuild.build(); Runfiles.Builder runfilesBuilder = - new Runfiles.Builder(ruleContext.getWorkspaceName()) + new Runfiles.Builder( + ruleContext.getWorkspaceName(), + ruleContext.getConfiguration().legacyExternalRunfiles()) .addRunfiles(ruleContext, RunfilesProvider.DEFAULT_RUNFILES); NestedSetBuilder<Artifact> filesToBuildBuilder = NestedSetBuilder.<Artifact>stableOrder().addTransitive(filesToBuildSet); diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java index 76af1c2ce7..cb33acb6ec 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java @@ -237,9 +237,13 @@ public class ObjcRuleClasses { static RuleConfiguredTargetBuilder ruleConfiguredTarget(RuleContext ruleContext, NestedSet<Artifact> filesToBuild) { RunfilesProvider runfilesProvider = RunfilesProvider.withData( - new Runfiles.Builder(ruleContext.getWorkspaceName()) + new Runfiles.Builder( + ruleContext.getWorkspaceName(), + ruleContext.getConfiguration().legacyExternalRunfiles()) .addRunfiles(ruleContext, RunfilesProvider.DEFAULT_RUNFILES).build(), - new Runfiles.Builder(ruleContext.getWorkspaceName()) + new Runfiles.Builder( + ruleContext.getWorkspaceName(), + ruleContext.getConfiguration().legacyExternalRunfiles()) .addTransitiveArtifacts(filesToBuild).build()); return new RuleConfiguredTargetBuilder(ruleContext) diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ReleaseBundlingSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ReleaseBundlingSupport.java index 8b15f7ac6a..69ad78fc9e 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/ReleaseBundlingSupport.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ReleaseBundlingSupport.java @@ -688,7 +688,8 @@ public final class ReleaseBundlingSupport { * Returns a {@link RunfilesSupport} that uses the provided runner script as the executable. */ RunfilesSupport runfilesSupport(Artifact runnerScript) throws InterruptedException { - Runfiles runfiles = new Runfiles.Builder(ruleContext.getWorkspaceName()) + Runfiles runfiles = new Runfiles.Builder( + ruleContext.getWorkspaceName(), ruleContext.getConfiguration().legacyExternalRunfiles()) .addArtifact(releaseBundling.getIpaArtifact()) .addArtifact(runnerScript) .addArtifact(attributes.iossim()) diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCommon.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCommon.java index 42e02980e6..aa7a3e1064 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCommon.java @@ -108,7 +108,9 @@ public class ProtoCommon { final NestedSet<Artifact> transitiveProtoSources, RuleContext ruleContext) { return RunfilesProvider.withData( Runfiles.EMPTY, - new Runfiles.Builder(ruleContext.getWorkspaceName()) + new Runfiles.Builder( + ruleContext.getWorkspaceName(), + ruleContext.getConfiguration().legacyExternalRunfiles()) // TODO(bazel-team): addArtifacts is deprecated, but addTransitive fails // due to nested set ordering restrictions. Figure this out. .addArtifacts(transitiveProtoSources) diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PyBinary.java b/src/main/java/com/google/devtools/build/lib/rules/python/PyBinary.java index ae048a0a03..f4e68f26d5 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/python/PyBinary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/python/PyBinary.java @@ -75,7 +75,8 @@ public abstract class PyBinary implements RuleConfiguredTargetFactory { semantics.createExecutable(ruleContext, common, ccLinkParamsStore, imports); Runfiles commonRunfiles = collectCommonRunfiles(ruleContext, common, semantics); - Runfiles.Builder defaultRunfilesBuilder = new Runfiles.Builder(ruleContext.getWorkspaceName()) + Runfiles.Builder defaultRunfilesBuilder = new Runfiles.Builder( + ruleContext.getWorkspaceName(), ruleContext.getConfiguration().legacyExternalRunfiles()) .merge(commonRunfiles); semantics.collectDefaultRunfilesForBinary(ruleContext, defaultRunfilesBuilder); Runfiles defaultRunfiles = defaultRunfilesBuilder.build(); @@ -90,7 +91,8 @@ public abstract class PyBinary implements RuleConfiguredTargetFactory { // Only include common runfiles and middleman. Default runfiles added by semantics are // excluded. The middleman is necessary to ensure the runfiles trees are generated for all // dependency binaries. - Runfiles dataRunfiles = new Runfiles.Builder(ruleContext.getWorkspaceName()) + Runfiles dataRunfiles = new Runfiles.Builder( + ruleContext.getWorkspaceName(), ruleContext.getConfiguration().legacyExternalRunfiles()) .merge(commonRunfiles) .addArtifact(runfilesSupport.getRunfilesMiddleman()) .build(); @@ -112,7 +114,8 @@ public abstract class PyBinary implements RuleConfiguredTargetFactory { private static Runfiles collectCommonRunfiles(RuleContext ruleContext, PyCommon common, PythonSemantics semantics) { - Runfiles.Builder builder = new Runfiles.Builder(ruleContext.getWorkspaceName()); + Runfiles.Builder builder = new Runfiles.Builder( + ruleContext.getWorkspaceName(), ruleContext.getConfiguration().legacyExternalRunfiles()); builder.addArtifact(common.getExecutable()); if (common.getConvertedFiles() != null) { builder.addSymlinks(common.getConvertedFiles()); diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java b/src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java index c375701f14..50aebce23b 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java @@ -405,8 +405,8 @@ public final class PyCommon { } else { ruleContext.attributeError("srcs", buildMultipleMainMatchesErrorText(explicitMain, mainSourceName, - mainArtifact.getRootRelativePath().toString(), - outItem.getRootRelativePath().toString())); + mainArtifact.getRunfilesPath().toString(), + outItem.getRunfilesPath().toString())); } } } @@ -417,7 +417,7 @@ public final class PyCommon { } PathFragment workspaceName = new PathFragment(ruleContext.getRule().getWorkspaceName()); - return workspaceName.getRelative(mainArtifact.getRootRelativePath()).getPathString(); + return workspaceName.getRelative(mainArtifact.getRunfilesPath()).getPathString(); } public Artifact getExecutable() { diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PyLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/python/PyLibrary.java index 95f4c8364e..eedef4f38d 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/python/PyLibrary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/python/PyLibrary.java @@ -74,7 +74,8 @@ public abstract class PyLibrary implements RuleConfiguredTargetFactory { return null; } - Runfiles.Builder runfilesBuilder = new Runfiles.Builder(ruleContext.getWorkspaceName()); + Runfiles.Builder runfilesBuilder = new Runfiles.Builder( + ruleContext.getWorkspaceName(), ruleContext.getConfiguration().legacyExternalRunfiles()); if (common.getConvertedFiles() != null) { runfilesBuilder.addSymlinks(common.getConvertedFiles()); } else { diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/TestActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/test/TestActionBuilder.java index 73235cdac2..f8875255d6 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/test/TestActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/test/TestActionBuilder.java @@ -88,7 +88,7 @@ public final class TestActionBuilder { // heuristically sharding is currently experimental. Also, we do detect // false-positive cases and return an error. return runfilesSupport.getRunfilesSymlinkNames().contains( - new PathFragment("tools/test_sharding_compliant")); + runfilesSupport.getWorkspaceName().getRelative("tools/test_sharding_compliant")); } /** diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/TestSuite.java b/src/main/java/com/google/devtools/build/lib/rules/test/TestSuite.java index 4b477cf70e..ed333a0058 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/test/TestSuite.java +++ b/src/main/java/com/google/devtools/build/lib/rules/test/TestSuite.java @@ -74,7 +74,8 @@ public class TestSuite implements RuleConfiguredTargetFactory { directTestsAndSuitesBuilder.add(dep); } - Runfiles runfiles = new Runfiles.Builder(ruleContext.getWorkspaceName()) + Runfiles runfiles = new Runfiles.Builder( + ruleContext.getWorkspaceName(), ruleContext.getConfiguration().legacyExternalRunfiles()) .addTargets(directTestsAndSuitesBuilder, RunfilesProvider.DATA_RUNFILES) .build(); |