Remove traces of the "include directory" from include scanning, and clarify that it is only used by build-info actions: the native cc_inc_library rule is gone, and that was where the include directory was used in a special way. PiperOrigin-RevId: 433150940
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BlazeDirectories.java b/src/main/java/com/google/devtools/build/lib/analysis/BlazeDirectories.java index e3c3e19..959c41c 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BlazeDirectories.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/BlazeDirectories.java
@@ -46,8 +46,10 @@ */ @Immutable public final class BlazeDirectories { - // Include directory name, relative to execRoot/blaze-out/configuration. Only one segment allowed. - public static final String RELATIVE_INCLUDE_DIR = StringCanonicalizer.intern("include"); + // TODO(bazel-team): is there actually any reason to put build-info files here? Can we at least + // give the directory a better name? + // Build-info directory, relative to execRoot/blaze-out/configuration. Only one segment allowed. + public static final String RELATIVE_BUILD_INFO_DIR = StringCanonicalizer.intern("include"); @VisibleForTesting static final String DEFAULT_EXEC_ROOT = "default-exec-root"; private final ServerDirectories serverDirectories;
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 b0d823a..5e40f16 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
@@ -279,8 +279,9 @@ return getConfiguration().getBinDirectory(getLabel().getRepository()); } - public ArtifactRoot getIncludeDirectory() { - return getConfiguration().getIncludeDirectory(getLabel().getRepository()); + @SuppressWarnings("Unused") // Goal is to migrate here. + public ArtifactRoot getBuildInfoDirectory() { + return getConfiguration().getBuildInfoDirectory(getLabel().getRepository()); } public ArtifactRoot getGenfilesDirectory() {
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationValue.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationValue.java index b2f673c..4145642 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationValue.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationValue.java
@@ -346,13 +346,14 @@ } /** - * Returns the include directory for this build configuration. + * Returns the build-info directory for this build configuration, where language-specific + * generated build-info artifacts are located. * - * @deprecated Use {@code RuleContext#getIncludeDirectory} instead whenever possible. + * @deprecated Use {@code RuleContext#getBuildInfoDirectory} instead whenever possible. */ @Deprecated - public ArtifactRoot getIncludeDirectory(RepositoryName repositoryName) { - return outputDirectories.getIncludeDirectory(repositoryName); + public ArtifactRoot getBuildInfoDirectory(RepositoryName repositoryName) { + return outputDirectories.getBuildInfoDirectory(repositoryName); } /** @deprecated Use {@link #getGenfilesDirectory} instead. */
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/OutputDirectories.java b/src/main/java/com/google/devtools/build/lib/analysis/config/OutputDirectories.java index a245cb3..b4edf2c 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/OutputDirectories.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/OutputDirectories.java
@@ -85,7 +85,7 @@ MIDDLEMAN("internal"), TESTLOGS("testlogs"), COVERAGE("coverage-metadata"), - INCLUDE(BlazeDirectories.RELATIVE_INCLUDE_DIR), + BUILDINFO(BlazeDirectories.RELATIVE_BUILD_INFO_DIR), OUTPUT(""); private final String name; @@ -118,7 +118,7 @@ private final ArtifactRoot outputDirectory; private final ArtifactRoot binDirectory; - private final ArtifactRoot includeDirectory; + private final ArtifactRoot buildInfoDirectory; private final ArtifactRoot genfilesDirectory; private final ArtifactRoot coverageDirectory; private final ArtifactRoot testlogsDirectory; @@ -147,8 +147,8 @@ this.outputDirectory = OutputDirectory.OUTPUT.getRoot(outputDirName, directories, mainRepositoryName); this.binDirectory = OutputDirectory.BIN.getRoot(outputDirName, directories, mainRepositoryName); - this.includeDirectory = - OutputDirectory.INCLUDE.getRoot(outputDirName, directories, mainRepositoryName); + this.buildInfoDirectory = + OutputDirectory.BUILDINFO.getRoot(outputDirName, directories, mainRepositoryName); this.genfilesDirectory = OutputDirectory.GENFILES.getRoot(outputDirName, directories, mainRepositoryName); this.coverageDirectory = @@ -280,11 +280,11 @@ return siblingRepositoryLayout ? buildDerivedRoot("bin", repositoryName, false) : binDirectory; } - /** Returns the include directory for this build configuration. */ - ArtifactRoot getIncludeDirectory(RepositoryName repositoryName) { + /** Returns the build-info directory for this build configuration. */ + ArtifactRoot getBuildInfoDirectory(RepositoryName repositoryName) { return siblingRepositoryLayout - ? buildDerivedRoot(BlazeDirectories.RELATIVE_INCLUDE_DIR, repositoryName, false) - : includeDirectory; + ? buildDerivedRoot(BlazeDirectories.RELATIVE_BUILD_INFO_DIR, repositoryName, false) + : buildInfoDirectory; } /** Returns the genfiles directory for this build configuration. */
diff --git a/src/main/java/com/google/devtools/build/lib/includescanning/LegacyIncludeScanner.java b/src/main/java/com/google/devtools/build/lib/includescanning/LegacyIncludeScanner.java index 0afe3ced..914116c 100644 --- a/src/main/java/com/google/devtools/build/lib/includescanning/LegacyIncludeScanner.java +++ b/src/main/java/com/google/devtools/build/lib/includescanning/LegacyIncludeScanner.java
@@ -26,7 +26,6 @@ import com.google.devtools.build.lib.actions.ArtifactRoot; import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.MissingDepExecException; -import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.concurrent.AbstractQueueVisitor; import com.google.devtools.build.lib.concurrent.ErrorClassifier; @@ -182,7 +181,7 @@ continue; } } - if (onlyCheckGenerated && !isRealOutputFile(fileFragment)) { + if (onlyCheckGenerated && !isOutputFile(fileFragment)) { continue; } viewedIllegalOutput = viewedIllegalOutput || isIllegalOutputFile(fileFragment, headerData); @@ -251,7 +250,7 @@ // Construct the full framework path path/to/foo.framework. PathFragment fullFrameworkPath = includePath.getRelative(frameworkName); - if (onlyCheckGenerated && !isRealOutputFile(fullFrameworkPath)) { + if (onlyCheckGenerated && !isOutputFile(fullFrameworkPath)) { return LocateOnPathResult.createNotFound(viewedIllegalOutput); } @@ -427,7 +426,6 @@ /** Search path for searching for all includes from frameworks. */ private final ImmutableList<PathFragment> frameworkIncludePaths; - private final PathFragment includeRootFragment; private final PathFragment outputPathFragment; private final Root absoluteRoot; @@ -487,8 +485,6 @@ this.inclusionCache = new InclusionCache(); this.execRoot = execRoot; this.outputPathFragment = outputPath.relativeTo(execRoot); - this.includeRootFragment = - outputPathFragment.getRelative(BlazeDirectories.RELATIVE_INCLUDE_DIR); this.absoluteRoot = Root.absoluteRoot(execRoot.getFileSystem()); } @@ -555,7 +551,7 @@ PathFragment includeAsWritten, boolean isSource, IncludeScanningHeaderData headerData) { - if (isRealOutputFile(execPath)) { + if (isOutputFile(execPath)) { return headerData.isDeclaredHeader(execPath); } // TODO(djasper): This code path cannot be hit with isSource being false. Verify and add @@ -634,17 +630,11 @@ private boolean isIllegalOutputFile( PathFragment includeFile, IncludeScanningHeaderData headerData) { - return isRealOutputFile(includeFile) && !headerData.isDeclaredHeader(includeFile); + return isOutputFile(includeFile) && !headerData.isDeclaredHeader(includeFile); } - private boolean isRealOutputFile(PathFragment path) { - return path.startsWith(outputPathFragment) && !isIncPath(path); - } - - private boolean isIncPath(PathFragment path) { - // See CreateIncSymlinkAction and where it's used: The symlink trees - // are always rooted at locations that fit the logic here. - return path.startsWith(includeRootFragment) && !path.equals(includeRootFragment); + private boolean isOutputFile(PathFragment path) { + return path.startsWith(outputPathFragment); } /** @@ -773,7 +763,7 @@ actionExecutionContext, grepIncludes, spawnIncludeScannerSupplier.get(), - isRealOutputFile(source.getExecPath()))); + isOutputFile(source.getExecPath()))); } catch (Throwable t) { fileParseCache.remove(source); future.setException(t);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppBuildInfo.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppBuildInfo.java index a82be97..9d2b77f 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppBuildInfo.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppBuildInfo.java
@@ -91,7 +91,7 @@ NestedSet<Artifact> inputs, boolean writeVolatileInfo, boolean writeNonVolatileInfo) { - ArtifactRoot outputPath = config.getIncludeDirectory(RepositoryName.MAIN); + ArtifactRoot outputPath = config.getBuildInfoDirectory(RepositoryName.MAIN); final Artifact header = buildInfoContext.getBuildInfoArtifact( headerName,
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaBuildInfoFactory.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaBuildInfoFactory.java index 13108e0..fe85ae8 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaBuildInfoFactory.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaBuildInfoFactory.java
@@ -132,7 +132,7 @@ BuildInfoPropertiesTranslator translator, boolean includeVolatile, boolean includeNonVolatile) { - ArtifactRoot outputPath = config.getIncludeDirectory(RepositoryName.MAIN); + ArtifactRoot outputPath = config.getBuildInfoDirectory(RepositoryName.MAIN); final Artifact output = context.getBuildInfoArtifact( propertyFileName,
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationValueTest.java b/src/test/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationValueTest.java index 1a8f286..2f3e339 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationValueTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationValueTest.java
@@ -54,7 +54,7 @@ .matches(outputDirPrefix); assertThat(config.getBinDirectory(RepositoryName.MAIN).getRoot().toString()) .matches(outputDirPrefix + "/bin"); - assertThat(config.getIncludeDirectory(RepositoryName.MAIN).getRoot().toString()) + assertThat(config.getBuildInfoDirectory(RepositoryName.MAIN).getRoot().toString()) .matches(outputDirPrefix + "/include"); assertThat(config.getTestLogsDirectory(RepositoryName.MAIN).getRoot().toString()) .matches(outputDirPrefix + "/testlogs");
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java index 3200fa1..625c375 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
@@ -1654,9 +1654,9 @@ /** * Gets a derived Artifact for testing in the subdirectory of the {@link - * BuildConfigurationValue#getIncludeDirectory} corresponding to the package of {@code owner}. So - * to specify a file foo/foo.o owned by target //foo:foo, {@code packageRelativePath} should just - * be "foo.h". + * BuildConfigurationValue#getBuildInfoDirectory} corresponding to the package of {@code owner}. + * So to specify a file foo/foo.o owned by target //foo:foo, {@code packageRelativePath} should + * just be "foo.h". */ protected Artifact getIncludeArtifact(String packageRelativePath, String owner) { return getIncludeArtifact(packageRelativePath, makeConfiguredTargetKey(owner)); @@ -1664,14 +1664,14 @@ /** * Gets a derived Artifact for testing in the subdirectory of the {@link - * BuildConfigurationValue#getIncludeDirectory} corresponding to the package of {@code owner}. So - * to specify a file foo/foo.o owned by target //foo:foo, {@code packageRelativePath} should just - * be "foo.h". + * BuildConfigurationValue#getBuildInfoDirectory} corresponding to the package of {@code owner}. + * So to specify a file foo/foo.o owned by target //foo:foo, {@code packageRelativePath} should + * just be "foo.h". */ protected Artifact getIncludeArtifact(String packageRelativePath, ArtifactOwner owner) { return getPackageRelativeDerivedArtifact( packageRelativePath, - targetConfig.getIncludeDirectory(owner.getLabel().getRepository()), + targetConfig.getBuildInfoDirectory(owner.getLabel().getRepository()), owner); }