diff options
Diffstat (limited to 'src/main')
9 files changed, 61 insertions, 79 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java index 12a2445580..7a4f72a4f7 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java @@ -32,7 +32,6 @@ import com.google.devtools.build.lib.actions.ActionGraph; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.ArtifactFactory; import com.google.devtools.build.lib.actions.ArtifactOwner; -import com.google.devtools.build.lib.actions.PackageRootResolver; import com.google.devtools.build.lib.actions.Root; import com.google.devtools.build.lib.analysis.ExtraActionArtifactsProvider.ExtraArtifactSet; import com.google.devtools.build.lib.analysis.config.BinTools; @@ -404,10 +403,10 @@ public class BuildView { }); } - private void prepareToBuild(BuildConfigurationCollection configurations, - PackageRootResolver resolver) throws ViewCreationFailedException { + private void prepareToBuild(BuildConfigurationCollection configurations) + throws ViewCreationFailedException { for (BuildConfiguration config : configurations.getAllConfigurations()) { - config.prepareToBuild(directories.getExecRoot(), getArtifactFactory(), resolver); + config.prepareToBuild(directories.getExecRoot()); } } @@ -485,7 +484,7 @@ public class BuildView { } } - prepareToBuild(configurations, new SkyframePackageRootResolver(skyframeExecutor, eventHandler)); + prepareToBuild(configurations); skyframeExecutor.injectWorkspaceStatusData(); SkyframeAnalysisResult skyframeAnalysisResult; try { 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 49740ecdc3..6032d1031a 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 @@ -29,8 +29,6 @@ import com.google.common.collect.ListMultimap; import com.google.common.collect.Maps; import com.google.common.collect.Multimap; import com.google.common.collect.MutableClassToInstanceMap; -import com.google.devtools.build.lib.actions.ArtifactFactory; -import com.google.devtools.build.lib.actions.PackageRootResolver; import com.google.devtools.build.lib.actions.Root; import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider; @@ -179,8 +177,7 @@ public final class BuildConfiguration { * <p>Do not use this method to change your fragment's state. */ @SuppressWarnings("unused") - public void prepareHook(Path execPath, ArtifactFactory artifactFactory, - PackageRootResolver resolver) throws ViewCreationFailedException { + public void prepareHook(Path execPath) throws ViewCreationFailedException { } /** @@ -2341,10 +2338,9 @@ public final class BuildConfiguration { * <p>C++ also requires this to resolve artifacts that are unconditionally included in every * compilation.</p> */ - public void prepareToBuild(Path execRoot, ArtifactFactory artifactFactory, - PackageRootResolver resolver) throws ViewCreationFailedException { + public void prepareToBuild(Path execRoot) throws ViewCreationFailedException { for (Fragment fragment : fragments.values()) { - fragment.prepareHook(execRoot, artifactFactory, resolver); + fragment.prepareHook(execRoot); } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java index feefa5ae14..6cc3a5c401 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java @@ -15,6 +15,7 @@ package com.google.devtools.build.lib.rules.cpp; import static com.google.devtools.build.lib.syntax.Type.BOOLEAN; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.Actions; @@ -48,6 +49,11 @@ import java.util.Map; * Implementation for the cc_toolchain rule. */ public class CcToolchain implements RuleConfiguredTargetFactory { + /** + * This file (found under the sysroot) may be unconditionally included in every C/C++ compilation. + */ + private static final PathFragment BUILTIN_INCLUDE_FILE_SUFFIX = + new PathFragment("include/stdc-predef.h"); @Override public ConfiguredTarget create(RuleContext ruleContext) { @@ -60,7 +66,7 @@ public class CcToolchain implements RuleConfiguredTargetFactory { final NestedSet<Artifact> objcopy = getFiles(ruleContext, "objcopy_files"); final NestedSet<Artifact> link = getFiles(ruleContext, "linker_files"); final NestedSet<Artifact> dwp = getFiles(ruleContext, "dwp_files"); - final NestedSet<Artifact> libcLink = inputsForLibcLink(ruleContext); + final NestedSet<Artifact> libcLink = inputsForLibc(ruleContext); String purposePrefix = Actions.escapeLabel(label) + "_"; String runtimeSolibDirBase = "_solib_" + "_" + Actions.escapeLabel(label); final PathFragment runtimeSolibDir = ruleContext.getConfiguration() @@ -143,7 +149,6 @@ public class CcToolchain implements RuleConfiguredTargetFactory { boolean supportsParamFiles = ruleContext.attributes().get("supports_param_files", BOOLEAN); boolean supportsHeaderParsing = ruleContext.attributes().get("supports_header_parsing", BOOLEAN); - CcToolchainProvider provider = new CcToolchainProvider( Preconditions.checkNotNull(ruleContext.getFragment(CppConfiguration.class)), @@ -163,7 +168,8 @@ public class CcToolchain implements RuleConfiguredTargetFactory { context, supportsParamFiles, supportsHeaderParsing, - getBuildVariables(ruleContext)); + getBuildVariables(ruleContext), + getBuiltinIncludes(ruleContext)); RuleConfiguredTargetBuilder builder = new RuleConfiguredTargetBuilder(ruleContext) .add(CcToolchainProvider.class, provider) @@ -192,10 +198,21 @@ public class CcToolchain implements RuleConfiguredTargetFactory { return builder.build(); } - private NestedSet<Artifact> inputsForLibcLink(RuleContext ruleContext) { - TransitiveInfoCollection libcLink = ruleContext.getPrerequisite(":libc_link", Mode.HOST); - return libcLink != null - ? libcLink.getProvider(FileProvider.class).getFilesToBuild() + private ImmutableList<Artifact> getBuiltinIncludes(RuleContext ruleContext) { + ImmutableList.Builder<Artifact> result = ImmutableList.builder(); + for (Artifact artifact : inputsForLibc(ruleContext)) { + if (artifact.getExecPath().endsWith(BUILTIN_INCLUDE_FILE_SUFFIX)) { + result.add(artifact); + } + } + + return result.build(); + } + + private NestedSet<Artifact> inputsForLibc(RuleContext ruleContext) { + TransitiveInfoCollection libc = ruleContext.getPrerequisite(":libc_top", Mode.HOST); + return libc != null + ? libc.getProvider(FileProvider.class).getFilesToBuild() : NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER); } @@ -203,17 +220,14 @@ public class CcToolchain implements RuleConfiguredTargetFactory { NestedSet<Artifact> crosstoolMiddleman) { return NestedSetBuilder.<Artifact>stableOrder() .addTransitive(crosstoolMiddleman) - // Use "libc_link" here, because it is functionally identical to the case - // below. If we introduce separate filegroups for compiling and linking, we - // need to fix that here. - .addTransitive(AnalysisUtils.getMiddlemanFor(ruleContext, ":libc_link")) + .addTransitive(AnalysisUtils.getMiddlemanFor(ruleContext, ":libc_top")) .build(); } private NestedSet<Artifact> fullInputsForLink(RuleContext ruleContext, NestedSet<Artifact> link) { return NestedSetBuilder.<Artifact>stableOrder() .addTransitive(link) - .addTransitive(AnalysisUtils.getMiddlemanFor(ruleContext, ":libc_link")) + .addTransitive(AnalysisUtils.getMiddlemanFor(ruleContext, ":libc_top")) .add(ruleContext.getAnalysisEnvironment().getEmbeddedToolArtifact( CppRuleClasses.BUILD_INTERFACE_SO)) .build(); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java index 9673e04c1e..05f866ff6e 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java @@ -13,6 +13,7 @@ // limitations under the License. package com.google.devtools.build.lib.rules.cpp; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.TransitiveInfoProvider; @@ -55,7 +56,8 @@ public final class CcToolchainProvider implements TransitiveInfoProvider { CppCompilationContext.EMPTY, false, false, - ImmutableMap.<String, String>of()); + ImmutableMap.<String, String>of(), + ImmutableList.<Artifact>of()); @Nullable private final CppConfiguration cppConfiguration; private final NestedSet<Artifact> crosstool; @@ -75,6 +77,7 @@ public final class CcToolchainProvider implements TransitiveInfoProvider { private final boolean supportsParamFiles; private final boolean supportsHeaderParsing; private final Map<String, String> buildVariables; + private final ImmutableList<Artifact> builtinIncludeFiles; public CcToolchainProvider( @Nullable CppConfiguration cppConfiguration, @@ -94,7 +97,8 @@ public final class CcToolchainProvider implements TransitiveInfoProvider { CppCompilationContext cppCompilationContext, boolean supportsParamFiles, boolean supportsHeaderParsing, - Map<String, String> buildVariables) { + Map<String, String> buildVariables, + ImmutableList<Artifact> builtinIncludeFiles) { this.cppConfiguration = cppConfiguration; this.crosstool = Preconditions.checkNotNull(crosstool); this.crosstoolMiddleman = Preconditions.checkNotNull(crosstoolMiddleman); @@ -113,6 +117,7 @@ public final class CcToolchainProvider implements TransitiveInfoProvider { this.supportsParamFiles = supportsParamFiles; this.supportsHeaderParsing = supportsHeaderParsing; this.buildVariables = buildVariables; + this.builtinIncludeFiles = builtinIncludeFiles; } /** @@ -243,4 +248,12 @@ public final class CcToolchainProvider implements TransitiveInfoProvider { public Map<String, String> getBuildVariables() { return buildVariables; } + + /** + * Return the set of include files that may be included even if they are not mentioned in the + * source file or any of the headers included by it. + */ + public ImmutableList<Artifact> getBuiltinIncludeFiles() { + return builtinIncludeFiles; + } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainRule.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainRule.java index 7ab982ff17..4519103d23 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainRule.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainRule.java @@ -47,7 +47,7 @@ public final class CcToolchainRule implements RuleDefinition { return ruleClass.endsWith("cc_toolchain"); } - private static final LateBoundLabel<BuildConfiguration> LIBC_LINK = + private static final LateBoundLabel<BuildConfiguration> LIBC_TOP = new LateBoundLabel<BuildConfiguration>(CppConfiguration.class) { @Override public Label getDefault( @@ -75,7 +75,7 @@ public final class CcToolchainRule implements RuleDefinition { .add(attr("supports_param_files", BOOLEAN).value(true)) .add(attr("supports_header_parsing", BOOLEAN).value(false)) // TODO(bazel-team): Should be using the TARGET configuration. - .add(attr(":libc_link", LABEL).cfg(HOST).value(LIBC_LINK)) + .add(attr(":libc_top", LABEL).cfg(HOST).value(LIBC_TOP)) .build(); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java index 3b7681b028..abbd13aecc 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java @@ -153,6 +153,7 @@ public class CppCompileAction extends AbstractAction implements IncludeScannable private final CppCompilationContext context; private final Collection<PathFragment> extraSystemIncludePrefixes; private final Iterable<IncludeScannable> lipoScannables; + private final ImmutableList<Artifact> builtinIncludeFiles; private final CppCompileCommandLine cppCompileCommandLine; private final boolean usePic; @@ -276,6 +277,7 @@ public class CppCompileAction extends AbstractAction implements IncludeScannable // We do not need to include the middleman artifact since it is a generated // artifact and will definitely exist prior to this action execution. this.mandatoryInputs = mandatoryInputs; + this.builtinIncludeFiles = CppHelper.getToolchain(ruleContext).getBuiltinIncludeFiles(); verifyIncludePaths(ruleContext); } @@ -342,8 +344,8 @@ public class CppCompileAction extends AbstractAction implements IncludeScannable @Nullable @Override - public Artifact getBuiltInIncludeFile() { - return cppConfiguration.getBuiltInIncludeFile(); + public List<Artifact> getBuiltInIncludeFiles() { + return builtinIncludeFiles; } public String getHostSystemName() { @@ -1106,6 +1108,10 @@ public class CppCompileAction extends AbstractAction implements IncludeScannable f.addPaths(getDeclaredIncludeSrcsInStableOrder()); f.addPaths(getExtraSystemIncludePrefixes()); f.addPaths(Artifact.asSortedPathFragments(getMandatoryInputs())); + // I'm not sure if getBuiltInIncludeFiles() is necessary here, since before an action cache hit + // is reported, include scanning needs to be run, and thus the changed set of files would be + // noticed. Better be safe than sorry. + f.addPaths(Artifact.asSortedPathFragments(getBuiltInIncludeFiles())); return f.hexDigestAndReset(); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java index e3be6f5b27..f151deaf59 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java @@ -25,10 +25,6 @@ import com.google.common.collect.Iterables; import com.google.common.collect.ListMultimap; import com.google.common.collect.Maps; import com.google.common.collect.Multimap; -import com.google.devtools.build.lib.actions.Artifact; -import com.google.devtools.build.lib.actions.ArtifactFactory; -import com.google.devtools.build.lib.actions.PackageRootResolutionException; -import com.google.devtools.build.lib.actions.PackageRootResolver; import com.google.devtools.build.lib.actions.Root; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.ViewCreationFailedException; @@ -70,8 +66,6 @@ import java.util.Map; import java.util.Set; import java.util.zip.ZipException; -import javax.annotation.Nullable; - /** * This class represents the C/C++ parts of the {@link BuildConfiguration}, * including the host architecture, target architecture, compiler version, and @@ -203,12 +197,6 @@ public class CppConfiguration extends BuildConfiguration.Fragment { public static final String FDO_STAMP_MACRO = "BUILD_FDO_TYPE"; /** - * This file (found under the sysroot) may be unconditionally included in every C/C++ compilation. - */ - private static final PathFragment BUILT_IN_INCLUDE_PATH_FRAGMENT = - new PathFragment("include/stdc-predef.h"); - - /** * Represents an optional flag that can be toggled using the package features mechanism. */ @Immutable @@ -308,7 +296,6 @@ public class CppConfiguration extends BuildConfiguration.Fragment { private final PathFragment sysroot; private final PathFragment runtimeSysroot; private final List<PathFragment> builtInIncludeDirectories; - private Artifact builtInIncludeFile; private final Map<String, PathFragment> toolPaths; private final PathFragment ldExecutable; @@ -1202,15 +1189,6 @@ public class CppConfiguration extends BuildConfiguration.Fragment { } /** - * Returns the built-in header automatically included by the toolchain compiler. All C++ files - * may implicitly include this file. May be null if {@link #getSysroot} is null. - */ - @Nullable - public Artifact getBuiltInIncludeFile() { - return builtInIncludeFile; - } - - /** * Returns the sysroot to be used. If the toolchain compiler does not support * different sysroots, or the sysroot is the same as the default sysroot, then * this method returns <code>null</code>. @@ -1914,28 +1892,7 @@ public class CppConfiguration extends BuildConfiguration.Fragment { } @Override - public void prepareHook(Path execRoot, ArtifactFactory artifactFactory, - PackageRootResolver resolver) throws ViewCreationFailedException { - // TODO(bazel-team): Remove the "relative" guard. sysroot should always be relative, and this - // should be enforced in the creation of CppConfiguration. - if (getSysroot() != null && !getSysroot().isAbsolute()) { - Root sysrootRoot; - try { - sysrootRoot = Iterables.getOnlyElement( - resolver.findPackageRootsForFiles( - // See doc of findPackageRootsForFiles for why we need a getChild here. - ImmutableList.of(getSysroot().getChild("dummy_child"))).entrySet()).getValue(); - } catch (PackageRootResolutionException prre) { - throw new ViewCreationFailedException("Failed to determine sysroot", prre); - } - - PathFragment sysrootExecPath = sysroot.getRelative(BUILT_IN_INCLUDE_PATH_FRAGMENT); - if (sysrootRoot.getPath().getRelative(sysrootExecPath).exists()) { - builtInIncludeFile = Preconditions.checkNotNull( - artifactFactory.getSourceArtifact(sysrootExecPath, sysrootRoot), - "%s %s", sysrootRoot, sysroot); - } - } + public void prepareHook(Path execRoot) throws ViewCreationFailedException { try { getFdoSupport().prepareToBuild(execRoot); } catch (ZipException e) { diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/IncludeScannable.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/IncludeScannable.java index 88b142bbb0..267a75063e 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/IncludeScannable.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/IncludeScannable.java @@ -73,7 +73,7 @@ public interface IncludeScannable { * does not mention it. */ @Nullable - Artifact getBuiltInIncludeFile(); + List<Artifact> getBuiltInIncludeFiles(); /** * Returns the artifact relative to which the {@code getCmdlineIncludes()} should be interpreted. diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/IncludeScanner.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/IncludeScanner.java index 0e14c4b6f5..c3592846ee 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/IncludeScanner.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/IncludeScanner.java @@ -106,10 +106,7 @@ public interface IncludeScanner { Set<Artifact> includes = Sets.newConcurrentHashSet(); final List<PathFragment> absoluteBuiltInIncludeDirs = new ArrayList<>(); - Artifact builtInInclude = action.getBuiltInIncludeFile(); - if (builtInInclude != null) { - includes.add(builtInInclude); - } + includes.addAll(action.getBuiltInIncludeFiles()); Profiler profiler = Profiler.instance(); try { |