Skip to content

Commit

Permalink
Add a --legacy_external_runfiles option
Browse files Browse the repository at this point in the history
This isn't hooked up to anything yet, but is another piece of getting #848
rolled forward.

--
MOS_MIGRATED_REVID=120582973
  • Loading branch information
kchodorow authored and meteorcloudy committed Apr 25, 2016
1 parent 1aedd1f commit 8dba2b2
Show file tree
Hide file tree
Showing 28 changed files with 125 additions and 40 deletions.
33 changes: 29 additions & 4 deletions src/main/java/com/google/devtools/build/lib/analysis/Runfiles.java
Original file line number Diff line number Diff line change
Expand Up @@ -262,20 +262,29 @@ public Artifact getManifestFile() {
*/
private final NestedSet<PruningManifest> pruningManifests;

private Runfiles(PathFragment suffix,
/**
* If external runfiles should be created under .runfiles/wsname/external/repo as well as
* .runfiles/repo.
*/
private final boolean legacyExternalRunfiles;

private Runfiles(
PathFragment suffix,
NestedSet<Artifact> artifacts,
NestedSet<SymlinkEntry> symlinks,
NestedSet<SymlinkEntry> rootSymlinks,
NestedSet<PruningManifest> pruningManifests,
EmptyFilesSupplier emptyFilesSupplier,
ConflictPolicy conflictPolicy) {
ConflictPolicy conflictPolicy,
boolean legacyExternalRunfiles) {
this.suffix = suffix;
this.unconditionalArtifacts = Preconditions.checkNotNull(artifacts);
this.symlinks = Preconditions.checkNotNull(symlinks);
this.rootSymlinks = Preconditions.checkNotNull(rootSymlinks);
this.pruningManifests = Preconditions.checkNotNull(pruningManifests);
this.emptyFilesSupplier = Preconditions.checkNotNull(emptyFilesSupplier);
this.conflictPolicy = conflictPolicy;
this.legacyExternalRunfiles = legacyExternalRunfiles;
}

/**
Expand Down Expand Up @@ -654,19 +663,35 @@ public static final class Builder {
/** Build the Runfiles object with this policy */
private ConflictPolicy conflictPolicy = ConflictPolicy.IGNORE;

private final boolean legacyExternalRunfiles;

/**
* Only used for Runfiles.EMPTY.
*/
private Builder() {
this.suffix = PathFragment.EMPTY_FRAGMENT;
this.legacyExternalRunfiles = 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(workspace, false);
}

/**
* Creates a builder with the given suffix.
* @param workspace is the string specified in workspace() in the WORKSPACE file.
* @param legacyExternalRunfiles if the wsname/external/repo symlinks should also be
* created.
*/
public Builder(String workspace, boolean legacyExternalRunfiles) {
this.suffix = new PathFragment(workspace);
this.legacyExternalRunfiles = legacyExternalRunfiles;
}

/**
Expand All @@ -675,7 +700,7 @@ public Builder(String workspace) {
public Runfiles build() {
return new Runfiles(suffix, artifactsBuilder.build(), symlinksBuilder.build(),
rootSymlinksBuilder.build(), pruningManifestsBuilder.build(),
emptyFilesSupplier, conflictPolicy);
emptyFilesSupplier, conflictPolicy, legacyExternalRunfiles);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,8 @@ private RunfilesSupport(RuleContext ruleContext, Artifact executable, Runfiles r
&& 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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -294,16 +294,17 @@ public static final class Builder {
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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -708,6 +708,13 @@ public String getCpu() {
+ "If false, write only manifests when possible.")
public boolean buildRunfiles;

@Option(name = "legacy_external_runfiles",
defaultValue = "true",
category = "undocumented",
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 = "",
Expand Down Expand Up @@ -2180,6 +2187,13 @@ public boolean buildRunfiles() {
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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,10 @@ public ConfiguredTarget create(RuleContext ruleContext) {
// 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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ public ConfiguredTarget create(RuleContext ruleContext) {
.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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ public ConfiguredTarget create(RuleContext ruleContext) {
.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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,9 @@ private static Runfiles merge(Runfiles runfiles, Artifact executable, RuleContex
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();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -563,7 +563,9 @@ public Runfiles invoke(SkylarkRuleContext ctx, SkylarkList files, Object transit
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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -680,7 +680,9 @@ public static RuleConfiguredTargetBuilder createAndroidBinary(
.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()))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -702,7 +702,8 @@ OutputGroupProvider.HIDDEN_TOP_LEVEL, collectHiddenTopLevelArtifacts(ruleContext
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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ public Artifact createApkBuilderSymlinks(RuleContext ruleContext) {
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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,8 @@ private static Runfiles collectRunfiles(RuleContext context,
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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ private static Runfiles collectRunfiles(RuleContext context,
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1039,7 +1039,8 @@ private TransitiveLipoInfoProvider collectTransitiveLipoInfo(CcCompilationOutput

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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,16 @@ public ConfiguredTarget create(RuleContext ruleContext) {
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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,9 @@ public ConfiguredTarget create(RuleContext ruleContext) throws InterruptedExcept
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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ protected JavaBinary(JavaSemantics semantics) {
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();
Expand Down Expand Up @@ -266,7 +267,11 @@ public ConfiguredTarget create(RuleContext ruleContext) throws InterruptedExcept

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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -718,7 +718,8 @@ public static Runfiles getRunfiles(
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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,9 @@ public ConfiguredTarget create(RuleContext ruleContext) throws InterruptedExcept
// 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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,8 @@ private NestedSet<Artifact> collectTransitiveJavaSourceJars() {

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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,9 +164,10 @@ xcodeProviderBuilder, new Attribute("non_propagated_deps", Mode.TARGET))
XcodeProvider xcodeProvider = xcodeProviderBuilder.build();
NestedSet<Artifact> filesToBuildSet = filesToBuild.build();

Runfiles.Builder runfilesBuilder =
new Runfiles.Builder(ruleContext.getWorkspaceName())
.addRunfiles(ruleContext, RunfilesProvider.DEFAULT_RUNFILES);
Runfiles.Builder runfilesBuilder = new Runfiles.Builder(
ruleContext.getWorkspaceName(),
ruleContext.getConfiguration().legacyExternalRunfiles())
.addRunfiles(ruleContext, RunfilesProvider.DEFAULT_RUNFILES);
NestedSetBuilder<Artifact> filesToBuildBuilder =
NestedSetBuilder.<Artifact>stableOrder().addTransitive(filesToBuildSet);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -255,9 +255,13 @@ static SpawnAction.Builder spawnBashOnDarwinActionBuilder(String cmd) {
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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -688,7 +688,8 @@ ReleaseBundlingSupport registerGenerateRunnerScriptAction(Artifact runnerScript,
* 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())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,9 @@ public static RunfilesProvider createRunfilesProvider(
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)
Expand Down
Loading

0 comments on commit 8dba2b2

Please sign in to comment.