Add the repository name as a parameter to the output path functions
This doesn't do anything yet, it's in preparation for the execroot rearranging
change. The execroot will have one bazel-out per repo, so it'll look like:
execroot/
repo1/
bazel-out/
local-fastbuild/
bin/
repo2/
bazel-out/
local-fastbuild/
bin/
genfiles/
repo3/
bazel-out/
local-fastbuild/
testlogs/
and so on. Thus, any output path (getBinDirectory() & friends) needs to know
what the repo name is. This changes so many places in the code I thought it
would be good to do separately, then just flip the functionality in the
execroot-rearranging commit.
While I was poking around, I changed all of the refs I could from getPackageRelativeArtifact() to getBin/GenfilesArtifact(), so that 1) rule implementation don't have to know as much about roots and 2) they'll be more isolated from other output dir changes.
`bazel info` and similar just return roots for the main repository.
The only "change" is passing around a target label in the Java rules.
Continues work on #1262.
--
MOS_MIGRATED_REVID=129985336
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/CompilationHelper.java b/src/main/java/com/google/devtools/build/lib/analysis/CompilationHelper.java
index dbf9a57..8a1fddc 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/CompilationHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/CompilationHelper.java
@@ -55,7 +55,8 @@
MiddlemanFactory factory = env.getMiddlemanFactory();
return ImmutableList.of(factory.createMiddlemanAllowMultiple(
env, actionOwner, ruleContext.getPackageDirectory(), purpose, filesToBuild,
- ruleContext.getConfiguration().getMiddlemanDirectory()));
+ ruleContext.getConfiguration().getMiddlemanDirectory(
+ ruleContext.getRule().getRepository())));
}
// TODO(bazel-team): remove this duplicated code after the ConfiguredTarget migration
@@ -87,6 +88,7 @@
Iterable<Artifact> artifacts = dep.getProvider(FileProvider.class).getFilesToBuild();
return ImmutableList.of(
factory.createMiddlemanAllowMultiple(env, actionOwner, ruleContext.getPackageDirectory(),
- purpose, artifacts, ruleContext.getConfiguration().getMiddlemanDirectory()));
+ purpose, artifacts, ruleContext.getConfiguration().getMiddlemanDirectory(
+ ruleContext.getRule().getRepository())));
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
index dc91ecb..ac47679 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
@@ -139,8 +139,8 @@
boolean isFileset, ArtifactFactory artifactFactory) {
Rule rule = outputFile.getAssociatedRule();
Root root = rule.hasBinaryOutput()
- ? configuration.getBinDirectory()
- : configuration.getGenfilesDirectory();
+ ? configuration.getBinDirectory(rule.getRepository())
+ : configuration.getGenfilesDirectory(rule.getRepository());
ArtifactOwner owner =
new ConfiguredTargetKey(rule.getLabel(), configuration.getArtifactOwnerConfiguration());
PathFragment rootRelativePath =
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/PseudoAction.java b/src/main/java/com/google/devtools/build/lib/analysis/PseudoAction.java
index e0eb712..eb2115a 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/PseudoAction.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/PseudoAction.java
@@ -84,6 +84,7 @@
public static Artifact getDummyOutput(RuleContext ruleContext) {
return ruleContext.getPackageRelativeArtifact(
ruleContext.getLabel().getName() + ".extra_action_dummy",
- ruleContext.getConfiguration().getGenfilesDirectory());
+ ruleContext.getConfiguration().getGenfilesDirectory(
+ ruleContext.getRule().getRepository()));
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
index 1bd77b5..d4cea81 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
@@ -103,7 +103,6 @@
*/
public final class RuleContext extends TargetContext
implements ActionConstructionContext, ActionRegistry, RuleErrorConsumer {
-
/**
* The configured version of FilesetEntry.
*/
@@ -520,8 +519,8 @@
*/
public Root getBinOrGenfilesDirectory() {
return rule.hasBinaryOutput()
- ? getConfiguration().getBinDirectory()
- : getConfiguration().getGenfilesDirectory();
+ ? getConfiguration().getBinDirectory(rule.getRepository())
+ : getConfiguration().getGenfilesDirectory(rule.getRepository());
}
/**
@@ -537,8 +536,12 @@
* guaranteeing that it never clashes with artifacts created by rules in other packages.
*/
public Artifact getBinArtifact(String relative) {
+ return getBinArtifact(new PathFragment(relative));
+ }
+
+ public Artifact getBinArtifact(PathFragment relative) {
return getPackageRelativeArtifact(
- new PathFragment(relative), getConfiguration().getBinDirectory());
+ relative, getConfiguration().getBinDirectory(rule.getRepository()));
}
/**
@@ -546,8 +549,12 @@
* guaranteeing that it never clashes with artifacts created by rules in other packages.
*/
public Artifact getGenfilesArtifact(String relative) {
+ return getGenfilesArtifact(new PathFragment(relative));
+ }
+
+ public Artifact getGenfilesArtifact(PathFragment relative) {
return getPackageRelativeArtifact(
- new PathFragment(relative), getConfiguration().getGenfilesDirectory());
+ relative, getConfiguration().getGenfilesDirectory(rule.getRepository()));
}
/**
@@ -1311,7 +1318,7 @@
*/
public final Artifact getRelatedArtifact(PathFragment pathFragment, String extension) {
PathFragment file = FileSystemUtils.replaceExtension(pathFragment, extension);
- return getDerivedArtifact(file, getConfiguration().getBinDirectory());
+ return getDerivedArtifact(file, getConfiguration().getBinDirectory(rule.getRepository()));
}
/**
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 fe06afb..c6b6e04 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
@@ -175,7 +175,7 @@
String basename = relativePath.getBaseName();
PathFragment inputManifestPath = relativePath.replaceName(basename + ".runfiles_manifest");
return context.getDerivedArtifact(inputManifestPath,
- context.getConfiguration().getBinDirectory());
+ context.getConfiguration().getBinDirectory(context.getRule().getRepository()));
}
/**
@@ -261,7 +261,8 @@
Iterable<Artifact> allRunfilesArtifacts) {
return context.getAnalysisEnvironment().getMiddlemanFactory().createRunfilesMiddleman(
context.getActionOwner(), owningExecutable, allRunfilesArtifacts,
- context.getConfiguration().getMiddlemanDirectory(), "runfiles_artifacts");
+ context.getConfiguration().getMiddlemanDirectory(context.getRule().getRepository()),
+ "runfiles_artifacts");
}
private Artifact createRunfilesMiddleman(ActionConstructionContext context,
@@ -269,7 +270,8 @@
return context.getAnalysisEnvironment().getMiddlemanFactory().createRunfilesMiddleman(
context.getActionOwner(), owningExecutable,
ImmutableList.of(artifactsMiddleman, outputManifest),
- context.getConfiguration().getMiddlemanDirectory(), "runfiles");
+ context.getConfiguration().getMiddlemanDirectory(context.getRule().getRepository()),
+ "runfiles");
}
/**
@@ -300,7 +302,7 @@
BuildConfiguration config = context.getConfiguration();
Artifact outputManifest = context.getDerivedArtifact(
- outputManifestPath, config.getBinDirectory());
+ outputManifestPath, config.getBinDirectory(context.getRule().getRepository()));
context
.getAnalysisEnvironment()
.registerAction(
@@ -328,7 +330,8 @@
PathFragment sourcesManifestPath = executablePath.getParentDirectory().getChild(
executablePath.getBaseName() + ".runfiles.SOURCES");
Artifact sourceOnlyManifest = context.getDerivedArtifact(
- sourcesManifestPath, context.getConfiguration().getBinDirectory());
+ sourcesManifestPath,
+ context.getConfiguration().getBinDirectory(context.getRule().getRepository()));
context.getAnalysisEnvironment().registerAction(SourceManifestAction.forRunfiles(
ManifestType.SOURCES_ONLY, context.getActionOwner(), sourceOnlyManifest, runfiles));
return sourceOnlyManifest;
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/FileWriteAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/FileWriteAction.java
index 23c1484..1c7043a 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/FileWriteAction.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/FileWriteAction.java
@@ -136,7 +136,8 @@
public static Artifact createFile(RuleContext ruleContext,
String fileName, CharSequence contents, boolean executable) {
Artifact scriptFileArtifact = ruleContext.getPackageRelativeArtifact(
- fileName, ruleContext.getConfiguration().getGenfilesDirectory());
+ fileName, ruleContext.getConfiguration().getGenfilesDirectory(
+ ruleContext.getRule().getRepository()));
ruleContext.registerAction(new FileWriteAction(
ruleContext.getActionOwner(), scriptFileArtifact, contents, executable));
return scriptFileArtifact;
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/ParamFileHelper.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/ParamFileHelper.java
index 6d4b767..20834f9 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/ParamFileHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/ParamFileHelper.java
@@ -22,6 +22,7 @@
import com.google.devtools.build.lib.actions.ParameterFile;
import com.google.devtools.build.lib.analysis.AnalysisEnvironment;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
+import com.google.devtools.build.lib.util.Preconditions;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.List;
import javax.annotation.Nullable;
@@ -55,7 +56,7 @@
* @param analysisEnvironment the analysis environment
* @param outputs outputs of the action (used to construct a filename for the params file)
*/
- public static Artifact getParamsFileMaybe(
+ static Artifact getParamsFileMaybe(
List<String> executableArgs,
@Nullable Iterable<String> arguments,
@Nullable CommandLine commandLine,
@@ -72,10 +73,10 @@
return null;
}
- PathFragment paramFilePath =
- ParameterFile.derivePath(Iterables.getFirst(outputs, null).getRootRelativePath());
-
- return analysisEnvironment.getDerivedArtifact(paramFilePath, configuration.getBinDirectory());
+ Artifact output = Iterables.getFirst(outputs, null);
+ Preconditions.checkNotNull(output);
+ PathFragment paramFilePath = ParameterFile.derivePath(output.getRootRelativePath());
+ return analysisEnvironment.getDerivedArtifact(paramFilePath, output.getRoot());
}
/**
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 f97ea94..fe2f807 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
@@ -39,6 +39,7 @@
import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection.Transitions;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
+import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.packages.Attribute;
@@ -444,12 +445,12 @@
}
@Option(
- name = "define",
- converter = Converters.AssignmentConverter.class,
- defaultValue = "",
- category = "semantics",
- allowMultiple = true,
- help = "Each --define option specifies an assignment for a build variable."
+ name = "define",
+ converter = Converters.AssignmentConverter.class,
+ defaultValue = "",
+ category = "semantics",
+ allowMultiple = true,
+ help = "Each --define option specifies an assignment for a build variable."
)
public List<Map.Entry<String, String>> commandLineBuildVariables;
@@ -557,7 +558,7 @@
defaultValue = "fastbuild",
category = "semantics", // Should this be "flags"?
help = "Specify the mode the binary will be built in. "
- + "Values: 'fastbuild', 'dbg', 'opt'.")
+ + "Values: 'fastbuild', 'dbg', 'opt'.")
public CompilationMode compilationMode;
/**
@@ -590,7 +591,7 @@
+ "will be read from the Bazel client environment, or by the name=value pair. "
+ "This option can be used multiple times to specify several variables. "
+ "Used only by the 'bazel test' command."
- )
+ )
public List<Map.Entry<String, String>> testEnvironment;
@Option(name = "collect_code_coverage",
@@ -600,7 +601,7 @@
+ "possible) and will collect coverage information during tests. Only targets that "
+ " match --instrumentation_filter will be affected. Usually this option should "
+ " not be specified directly - 'bazel coverage' command should be used instead."
- )
+ )
public boolean collectCodeCoverage;
@Option(name = "microcoverage",
@@ -611,7 +612,7 @@
+ "--instrumentation_filter will be affected. Usually this option should not be "
+ "specified directly - 'blaze coverage --microcoverage' command should be used "
+ "instead."
- )
+ )
public boolean collectMicroCoverage;
@Option(name = "coverage_support",
@@ -704,7 +705,7 @@
+ "executable. Can be used multiple times to specify several arguments. "
+ "If multiple tests are executed, each of them will receive identical arguments. "
+ "Used only by the 'bazel test' command."
- )
+ )
public List<String> testArguments;
@Option(name = "test_filter",
@@ -727,31 +728,31 @@
public boolean checkFilesetDependenciesRecursively;
@Option(
- name = "experimental_skyframe_native_filesets",
- defaultValue = "false",
- category = "experimental",
- help =
- "If true, Blaze will use the skyframe-native implementation of the Fileset rule."
- + " This offers improved performance in incremental builds of Filesets as well as"
- + " correct incremental behavior, but is not yet stable. The default is false,"
- + " meaning Blaze uses the legacy impelementation of Fileset."
+ name = "experimental_skyframe_native_filesets",
+ defaultValue = "false",
+ category = "experimental",
+ help =
+ "If true, Blaze will use the skyframe-native implementation of the Fileset rule."
+ + " This offers improved performance in incremental builds of Filesets as well as"
+ + " correct incremental behavior, but is not yet stable. The default is false,"
+ + " meaning Blaze uses the legacy impelementation of Fileset."
)
public boolean skyframeNativeFileset;
@Option(
- name = "run_under",
- category = "run",
- defaultValue = "null",
- converter = RunUnderConverter.class,
- help =
- "Prefix to insert in front of command before running. "
- + "Examples:\n"
- + "\t--run_under=valgrind\n"
- + "\t--run_under=strace\n"
- + "\t--run_under='strace -c'\n"
- + "\t--run_under='valgrind --quiet --num-callers=20'\n"
- + "\t--run_under=//package:target\n"
- + "\t--run_under='//package:target --options'\n"
+ name = "run_under",
+ category = "run",
+ defaultValue = "null",
+ converter = RunUnderConverter.class,
+ help =
+ "Prefix to insert in front of command before running. "
+ + "Examples:\n"
+ + "\t--run_under=valgrind\n"
+ + "\t--run_under=strace\n"
+ + "\t--run_under='strace -c'\n"
+ + "\t--run_under='valgrind --quiet --num-callers=20'\n"
+ + "\t--run_under=//package:target\n"
+ + "\t--run_under='//package:target --options'\n"
)
public RunUnder runUnder;
@@ -789,13 +790,13 @@
public boolean checkLicenses;
@Option(
- name = "enforce_constraints",
- defaultValue = "true",
- category = "undocumented",
- help =
- "Checks the environments each target is compatible with and reports errors if any "
- + "target has dependencies that don't support the same environments",
- oldName = "experimental_enforce_constraints"
+ name = "enforce_constraints",
+ defaultValue = "true",
+ category = "undocumented",
+ help =
+ "Checks the environments each target is compatible with and reports errors if any "
+ + "target has dependencies that don't support the same environments",
+ oldName = "experimental_enforce_constraints"
)
public boolean enforceConstraints;
@@ -849,18 +850,18 @@
public boolean useDynamicConfigurations;
@Option(
- name = "experimental_enable_runfiles",
- defaultValue = "auto",
- category = "undocumented",
- help = "Enable runfiles; off on Windows, on on other platforms"
+ name = "experimental_enable_runfiles",
+ defaultValue = "auto",
+ category = "undocumented",
+ help = "Enable runfiles; off on Windows, on on other platforms"
)
public TriState enableRunfiles;
@Option(
- name = "build_python_zip",
- defaultValue = "auto",
- category = "undocumented",
- help = "Build python executable zip; on on Windows, off on other platforms"
+ name = "build_python_zip",
+ defaultValue = "auto",
+ category = "undocumented",
+ help = "Build python executable zip; on on Windows, off on other platforms"
)
public TriState buildPythonZip;
@@ -1153,8 +1154,8 @@
== TestActionBuilder.TestShardingStrategy.EXPERIMENTAL_HEURISTIC) {
reporter.handle(Event.warn(
"Heuristic sharding is intended as a one-off experimentation tool for determing the "
- + "benefit from sharding certain tests. Please don't keep this option in your "
- + ".blazerc or continuous build"));
+ + "benefit from sharding certain tests. Please don't keep this option in your "
+ + ".blazerc or continuous build"));
}
if (options.useDynamicConfigurations && !options.useDistinctHostConfiguration) {
@@ -1256,8 +1257,8 @@
globalMakeEnvBuilder.put("COMPILATION_MODE", options.compilationMode.toString());
globalMakeEnvBuilder.put("BINMODE", "-"
+ ((options.compilationMode == CompilationMode.FASTBUILD)
- ? "dbg"
- : options.compilationMode.toString()));
+ ? "dbg"
+ : options.compilationMode.toString()));
/*
* Attention! Document these in the build-encyclopedia
*/
@@ -1351,7 +1352,7 @@
if (lateBoundDefaults.containsKey(option.name())) {
value = lateBoundDefaults.get(option.name());
} else if (!option.defaultValue().equals("null")) {
- // See {@link Option#defaultValue} for an explanation of default "null" strings.
+ // See {@link Option#defaultValue} for an explanation of default "null" strings.
value = option.defaultValue();
}
}
@@ -1469,10 +1470,10 @@
*/
public interface TransitionApplier {
/**
- * Creates a new instance of this transition applier bound to the specified source
- * configuration.
- */
- TransitionApplier create(BuildConfiguration config);
+ * Creates a new instance of this transition applier bound to the specified source
+ * configuration.
+ */
+ TransitionApplier create(BuildConfiguration config);
/**
* Accepts the given configuration transition. The implementation decides how to turn
@@ -1810,7 +1811,7 @@
*/
@VisibleForTesting
static Map<String, String> getMapping(List<String> variables,
- Map<String, String> environment) {
+ Map<String, String> environment) {
Map<String, String> result = new HashMap<>();
for (String var : variables) {
if (environment.containsKey(var)) {
@@ -1868,7 +1869,7 @@
/**
* Returns the output directory for this build configuration.
*/
- public Root getOutputDirectory() {
+ public Root getOutputDirectory(RepositoryName repositoryName) {
return outputRoots.outputDirectory;
}
@@ -1882,6 +1883,18 @@
}
/**
+ * TODO(kchodorow): This (and the other get*Directory functions) won't work with external
+ * repositories without changes to how ArtifactFactory resolves derived roots. This is not an
+ * issue right now because it only effects Blaze's include scanning (internal) and Bazel's
+ * repositories (external) but will need to be fixed.
+ * TODO(kchodorow): Use the repository name to derive the bin directory.
+ */
+ @SuppressWarnings("unused")
+ public Root getBinDirectory(RepositoryName repositoryName) {
+ return getBinDirectory();
+ }
+
+ /**
* Returns a relative path to the bin directory at execution time.
*/
public PathFragment getBinFragment() {
@@ -1890,8 +1903,10 @@
/**
* Returns the include directory for this build configuration.
+ * TODO(kchodorow): Use the repository name to derive the include directory.
*/
- public Root getIncludeDirectory() {
+ @SuppressWarnings("unused")
+ public Root getIncludeDirectory(RepositoryName repositoryName) {
return outputRoots.includeDirectory;
}
@@ -1904,19 +1919,29 @@
return outputRoots.genfilesDirectory;
}
+ // TODO(kchodorow): Use the repository name to derive the genfiles directory.
+ @SuppressWarnings("unused")
+ public Root getGenfilesDirectory(RepositoryName repositoryName) {
+ return getGenfilesDirectory();
+ }
+
/**
* Returns the directory where coverage-related artifacts and metadata files
* should be stored. This includes for example uninstrumented class files
* needed for Jacoco's coverage reporting tools.
+ * TODO(kchodorow): Use the repository name to derive the coverage directory.
*/
- public Root getCoverageMetadataDirectory() {
+ @SuppressWarnings("unused")
+ public Root getCoverageMetadataDirectory(RepositoryName repositoryName) {
return outputRoots.coverageMetadataDirectory;
}
/**
* Returns the testlogs directory for this build configuration.
+ * TODO(kchodorow): Use the repository name to derive the test directory.
*/
- public Root getTestLogsDirectory() {
+ @SuppressWarnings("unused")
+ public Root getTestLogsDirectory(RepositoryName repositoryName) {
return outputRoots.testLogsDirectory;
}
@@ -1942,8 +1967,10 @@
/**
* Returns the internal directory (used for middlemen) for this build configuration.
+ * TODO(kchodorow): Use the repository name to derive the middleman directory.
*/
- public Root getMiddlemanDirectory() {
+ @SuppressWarnings("unused")
+ public Root getMiddlemanDirectory(RepositoryName repositoryName) {
return outputRoots.middlemanDirectory;
}
@@ -1977,12 +2004,11 @@
}
@SkylarkCallable(
- name = "default_shell_env",
- structField = true,
- doc =
- "A dictionary representing the local shell environment. It maps variables "
- + "to their values (strings). The local shell environment contains settings that are "
- + "machine specific, therefore its use should be avoided in rules meant to be hermetic."
+ name = "default_shell_env",
+ structField = true,
+ doc = "A dictionary representing the local shell environment. It maps variables "
+ + "to their values (strings). The local shell environment contains settings that are "
+ + "machine specific, therefore its use should be avoided in rules meant to be hermetic."
)
public ImmutableMap<String, String> getLocalShellEnvironment() {
return localShellEnvironment;
@@ -2263,9 +2289,9 @@
// Configuration-specific roots.
roots.add(getBinDirectory());
roots.add(getGenfilesDirectory());
- roots.add(getIncludeDirectory());
- roots.add(getMiddlemanDirectory());
- roots.add(getTestLogsDirectory());
+ roots.add(getIncludeDirectory(RepositoryName.MAIN));
+ roots.add(getMiddlemanDirectory(RepositoryName.MAIN));
+ roots.add(getTestLogsDirectory(RepositoryName.MAIN));
return ImmutableList.copyOf(roots);
}
