Do not depend on CppConfiguration to provide CROSSTOOL file to cc_toolchain
Use CcSkyframeSupportFunction instead.
#6072
RELNOTES: None.
PiperOrigin-RevId: 214004051
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java
index 226f135..f613d8e 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java
@@ -20,8 +20,8 @@
import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.bazel.rules.cpp.BazelCppRuleClasses;
import com.google.devtools.build.lib.bazel.rules.sh.BazelShRuleClasses;
+import com.google.devtools.build.lib.rules.cpp.CcSkyframeSupportFunction;
import com.google.devtools.build.lib.rules.cpp.CcSkyframeSupportValue;
-import com.google.devtools.build.lib.rules.cpp.CcSupportFunction;
import com.google.devtools.build.lib.runtime.BlazeModule;
import com.google.devtools.build.lib.runtime.BlazeRuntime;
import com.google.devtools.build.lib.runtime.Command;
@@ -99,7 +99,8 @@
@Override
public void workspaceInit(
BlazeRuntime runtime, BlazeDirectories directories, WorkspaceBuilder builder) {
- builder.addSkyFunction(CcSkyframeSupportValue.SKYFUNCTION, new CcSupportFunction(directories));
+ builder.addSkyFunction(
+ CcSkyframeSupportValue.SKYFUNCTION, new CcSkyframeSupportFunction(directories));
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcSkyframeSupportFunction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcSkyframeSupportFunction.java
new file mode 100644
index 0000000..0e2192e
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcSkyframeSupportFunction.java
@@ -0,0 +1,114 @@
+// Copyright 2014 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.rules.cpp;
+
+import com.google.common.base.Preconditions;
+import com.google.devtools.build.lib.actions.FileValue;
+import com.google.devtools.build.lib.analysis.BlazeDirectories;
+import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
+import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.vfs.FileSystemUtils;
+import com.google.devtools.build.lib.vfs.Path;
+import com.google.devtools.build.lib.vfs.Root;
+import com.google.devtools.build.lib.vfs.RootedPath;
+import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.CrosstoolRelease;
+import com.google.devtools.build.skyframe.SkyFunction;
+import com.google.devtools.build.skyframe.SkyFunctionException;
+import com.google.devtools.build.skyframe.SkyKey;
+import com.google.devtools.build.skyframe.SkyValue;
+import java.io.IOException;
+import java.io.InputStream;
+import javax.annotation.Nullable;
+
+/**
+ * A {@link SkyFunction} that does things for FDO that a regular configured target is not allowed
+ * to.
+ *
+ * <p>This only exists because the value of {@code --fdo_optimize} can be a workspace-relative path
+ * and thus we need to depend on {@link BlazeDirectories} somehow, which neither the configuration
+ * nor the analysis phase can "officially" do.
+ *
+ * <p>The fix is probably to make it possible for {@link
+ * com.google.devtools.build.lib.analysis.actions.SymlinkAction} to create workspace-relative
+ * symlinks because action execution can hopefully depend on {@link BlazeDirectories}.
+ *
+ * <p>There is also the awful and incorrect {@link Path#exists()} call in {@link
+ * com.google.devtools.build.lib.view.proto.CcProtoProfileProvider#getProfile(
+ * com.google.devtools.build.lib.analysis.RuleContext)} which needs a {@link Path}.
+ */
+public class CcSkyframeSupportFunction implements SkyFunction {
+ private final BlazeDirectories directories;
+
+ public CcSkyframeSupportFunction(BlazeDirectories directories) {
+ this.directories = Preconditions.checkNotNull(directories);
+ }
+
+ @Nullable
+ @Override
+ public SkyValue compute(SkyKey skyKey, Environment env)
+ throws InterruptedException, CcSkyframeSupportException {
+ CcSkyframeSupportValue.Key key = (CcSkyframeSupportValue.Key) skyKey.argument();
+ Path fdoZipPath = null;
+ CrosstoolRelease crosstoolRelease = null;
+ if (key.getFdoZipPath() != null) {
+ fdoZipPath = directories.getWorkspace().getRelative(key.getFdoZipPath());
+ }
+
+ if (key.getCrosstoolPath() != null) {
+ try {
+ Root root;
+ // Dear reader, if your eye just twitched and the thought cannot escape your mind that
+ // I should've used execroot, beware, execroot is created after the analysis, and this
+ // function is executed during the analysis.
+ if (key.getCrosstoolPath().startsWith(Label.EXTERNAL_PACKAGE_NAME)) {
+ root = Root.fromPath(directories.getOutputBase());
+ } else {
+ root = Root.fromPath(directories.getWorkspace());
+ }
+ FileValue crosstoolFileValue =
+ (FileValue)
+ env.getValue(FileValue.key(RootedPath.toRootedPath(root, key.getCrosstoolPath())));
+ if (env.valuesMissing()) {
+ return null;
+ }
+
+ Path crosstoolFile = crosstoolFileValue.realRootedPath().asPath();
+ try (InputStream inputStream = crosstoolFile.getInputStream()) {
+ String crosstoolContent = new String(FileSystemUtils.readContentAsLatin1(inputStream));
+ crosstoolRelease =
+ CrosstoolConfigurationLoader.toReleaseConfiguration(
+ "CROSSTOOL file " + key.getCrosstoolPath(), crosstoolContent);
+ }
+ } catch (IOException | InvalidConfigurationException e) {
+ throw new CcSkyframeSupportException(e, key);
+ }
+ }
+
+ return new CcSkyframeSupportValue(fdoZipPath, crosstoolRelease);
+ }
+
+ @Nullable
+ @Override
+ public String extractTag(SkyKey skyKey) {
+ return null;
+ }
+
+ /** Exception encapsulating IOExceptions thrown in {@link CcSkyframeSupportFunction} */
+ public static class CcSkyframeSupportException extends SkyFunctionException {
+
+ public CcSkyframeSupportException(Exception cause, SkyKey childKey) {
+ super(cause, childKey);
+ }
+ }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcSkyframeSupportValue.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcSkyframeSupportValue.java
index 19aef12..d7a6487 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcSkyframeSupportValue.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcSkyframeSupportValue.java
@@ -19,6 +19,7 @@
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
+import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.CrosstoolRelease;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyFunctionName;
import com.google.devtools.build.skyframe.SkyKey;
@@ -28,9 +29,9 @@
/**
* A container for the path to the FDO profile.
*
- * <p>{@link CcSkyframeSupportValue} is created from {@link CcSupportFunction} (a {@link
+ * <p>{@link CcSkyframeSupportValue} is created from {@link CcSkyframeSupportFunction} (a {@link
* SkyFunction}), which is requested from Skyframe by the {@code cc_toolchain}/{@code
- * cc_toolchain_suite} rule. It's done this way because the path depends on both a command line
+ * cc_toolchain_suite} rules. It's done this way because the path depends on both a command line
* argument and the location of the workspace and the latter is not available either during
* configuration creation or during the analysis phase.
*/
@@ -45,20 +46,26 @@
public static class Key implements SkyKey {
private static final Interner<Key> interner = BlazeInterners.newWeakInterner();
- private final PathFragment filePath;
+ private final PathFragment fdoZipPath;
+ private final PathFragment crosstoolPath;
- private Key(PathFragment filePath) {
- this.filePath = filePath;
+ private Key(PathFragment fdoZipPath, PathFragment crosstoolPath) {
+ this.fdoZipPath = fdoZipPath;
+ this.crosstoolPath = crosstoolPath;
}
@AutoCodec.Instantiator
@AutoCodec.VisibleForSerialization
- static Key of(PathFragment filePath) {
- return interner.intern(new Key(filePath));
+ static Key of(PathFragment fdoZipPath, PathFragment crosstoolPath) {
+ return interner.intern(new Key(fdoZipPath, crosstoolPath));
}
- public PathFragment getFilePath() {
- return filePath;
+ public PathFragment getCrosstoolPath() {
+ return crosstoolPath;
+ }
+
+ public PathFragment getFdoZipPath() {
+ return fdoZipPath;
}
@Override
@@ -66,18 +73,18 @@
if (this == o) {
return true;
}
-
if (!(o instanceof Key)) {
return false;
}
-
- Key that = (Key) o;
- return Objects.equals(this.filePath, that.filePath);
+ Key key = (Key) o;
+ return Objects.equals(fdoZipPath, key.fdoZipPath)
+ && Objects.equals(crosstoolPath, key.crosstoolPath);
}
@Override
public int hashCode() {
- return Objects.hash(filePath);
+
+ return Objects.hash(fdoZipPath, crosstoolPath);
}
@Override
@@ -88,19 +95,26 @@
/** Path of the profile file passed to {@code --fdo_optimize} */
// TODO(lberki): This should be a PathFragment.
- // Except that CcProtoProfileProvider#getProfile() calls #exists() on it, which is ridiculously
- // incorrect.
- private final Path filePath;
+ // Except that CcProtoProfileProvider#getProfile() calls #exists() on it,
+ // This is all ridiculously incorrect and should be removed asap.
+ private final Path fdoZipPath;
- CcSkyframeSupportValue(Path filePath) {
- this.filePath = filePath;
+ private final CrosstoolRelease crosstoolRelease;
+
+ CcSkyframeSupportValue(Path fdoZipPath, CrosstoolRelease crosstoolRelease) {
+ this.fdoZipPath = fdoZipPath;
+ this.crosstoolRelease = crosstoolRelease;
}
- public Path getFilePath() {
- return filePath;
+ public Path getFdoZipPath() {
+ return fdoZipPath;
}
- public static SkyKey key(PathFragment fdoProfileArgument) {
- return Key.of(fdoProfileArgument);
+ public CrosstoolRelease getCrosstoolRelease() {
+ return crosstoolRelease;
+ }
+
+ public static SkyKey key(PathFragment fdoZipPath, PathFragment crosstoolPath) {
+ return Key.of(fdoZipPath, crosstoolPath);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcSupportFunction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcSupportFunction.java
deleted file mode 100644
index 113361f..0000000
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcSupportFunction.java
+++ /dev/null
@@ -1,63 +0,0 @@
-// Copyright 2014 The Bazel Authors. All rights reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-package com.google.devtools.build.lib.rules.cpp;
-
-import com.google.common.base.Preconditions;
-import com.google.devtools.build.lib.analysis.BlazeDirectories;
-import com.google.devtools.build.lib.vfs.Path;
-import com.google.devtools.build.skyframe.SkyFunction;
-import com.google.devtools.build.skyframe.SkyKey;
-import com.google.devtools.build.skyframe.SkyValue;
-import javax.annotation.Nullable;
-
-/**
- * A {@link SkyFunction} that does things for FDO that a regular configured target is not allowed
- * to.
- *
- * <p>This only exists because the value of {@code --fdo_optimize} can be a workspace-relative path
- * and thus we need to depend on {@link BlazeDirectories} somehow, which neither the configuration
- * nor the analysis phase can "officially" do.
- *
- * <p>The fix is probably to make it possible for {@link
- * com.google.devtools.build.lib.analysis.actions.SymlinkAction} to create workspace-relative
- * symlinks because action execution can hopefully depend on {@link BlazeDirectories}.
- *
- * <p>There is also the awful and incorrect {@link Path#exists()} call in {@link
- * com.google.devtools.build.lib.view.proto.CcProtoProfileProvider#getProfile(
- * com.google.devtools.build.lib.analysis.RuleContext)} which needs a {@link Path}.
- */
-public class CcSupportFunction implements SkyFunction {
- private final BlazeDirectories directories;
-
- public CcSupportFunction(BlazeDirectories directories) {
- this.directories = Preconditions.checkNotNull(directories);
- }
-
- @Nullable
- @Override
- public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedException {
- CcSkyframeSupportValue.Key key = (CcSkyframeSupportValue.Key) skyKey.argument();
- Path resolvedPath =
- key.getFilePath() == null
- ? null
- : directories.getWorkspace().getRelative(key.getFilePath());
- return new CcSkyframeSupportValue(resolvedPath);
- }
-
- @Nullable
- @Override
- public String extractTag(SkyKey skyKey) {
- return null;
- }
-}
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 3fc13b0..76b5b58 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
@@ -14,6 +14,7 @@
package com.google.devtools.build.lib.rules.cpp;
import static com.google.devtools.build.lib.syntax.Type.BOOLEAN;
+import static com.google.devtools.build.lib.syntax.Type.STRING;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
@@ -57,10 +58,12 @@
import com.google.devtools.build.lib.syntax.Type;
import com.google.devtools.build.lib.util.FileType;
import com.google.devtools.build.lib.util.Pair;
+import com.google.devtools.build.lib.util.StringUtil;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.CToolchain;
+import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.CrosstoolRelease;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.protobuf.TextFormat;
@@ -326,7 +329,6 @@
BuildConfiguration configuration = Preconditions.checkNotNull(ruleContext.getConfiguration());
CppConfiguration cppConfiguration =
Preconditions.checkNotNull(configuration.getFragment(CppConfiguration.class));
- CppToolchainInfo toolchainInfo = getCppToolchainInfo(ruleContext, cppConfiguration);
PathFragment fdoZip = null;
FdoInputFile prefetchHints = null;
@@ -407,14 +409,28 @@
return null;
}
- SkyKey fdoKey = CcSkyframeSupportValue.key(fdoZip);
+ // Is there a toolchain proto available on the target directly?
+ CToolchain toolchain = parseToolchainFromAttributes(ruleContext);
+ PathFragment crosstoolPath = null;
+ if (toolchain == null && cppConfiguration.getCrosstoolFromCcToolchainProtoAttribute() == null) {
+ crosstoolPath =
+ cppConfiguration
+ .getCrosstoolTopPathFragment()
+ .getChild(CrosstoolConfigurationLoader.CROSSTOOL_CONFIGURATION_FILENAME);
+ }
+
+ SkyKey ccSupportKey = CcSkyframeSupportValue.key(fdoZip, crosstoolPath);
SkyFunction.Environment skyframeEnv = ruleContext.getAnalysisEnvironment().getSkyframeEnv();
- CcSkyframeSupportValue fdoSupport = (CcSkyframeSupportValue) skyframeEnv.getValue(fdoKey);
+ CcSkyframeSupportValue ccSkyframeSupportValue =
+ (CcSkyframeSupportValue) skyframeEnv.getValue(ccSupportKey);
if (skyframeEnv.valuesMissing()) {
return null;
}
+ CppToolchainInfo toolchainInfo =
+ getCppToolchainInfo(ruleContext, cppConfiguration, ccSkyframeSupportValue, toolchain);
+
final Label label = ruleContext.getLabel();
final NestedSet<Artifact> crosstool = ruleContext.getPrerequisite("all_files", Mode.HOST)
.getProvider(FileProvider.class).getFilesToBuild();
@@ -550,12 +566,12 @@
if (fdoMode == FdoMode.LLVM_FDO) {
profileArtifact =
convertLLVMRawProfileToIndexed(
- fdoSupport.getFilePath().asFragment(), toolchainInfo, ruleContext);
+ ccSkyframeSupportValue.getFdoZipPath().asFragment(), toolchainInfo, ruleContext);
if (ruleContext.hasErrors()) {
return null;
}
} else if (fdoMode == FdoMode.AUTO_FDO || fdoMode == FdoMode.XBINARY_FDO) {
- Path fdoProfile = fdoSupport.getFilePath();
+ Path fdoProfile = ccSkyframeSupportValue.getFdoZipPath();
profileArtifact = ruleContext.getUniqueDirectoryArtifact(
"fdo",
fdoProfile.getBaseName(),
@@ -608,7 +624,7 @@
sysroot,
fdoMode,
new FdoProvider(
- fdoSupport.getFilePath(),
+ ccSkyframeSupportValue.getFdoZipPath(),
fdoMode,
cppConfiguration.getFdoInstrument(),
profileArtifact,
@@ -653,7 +669,11 @@
/** Finds an appropriate {@link CppToolchainInfo} for this target. */
private CppToolchainInfo getCppToolchainInfo(
- RuleContext ruleContext, CppConfiguration cppConfiguration) throws RuleErrorException {
+ RuleContext ruleContext,
+ CppConfiguration cppConfiguration,
+ CcSkyframeSupportValue ccSkyframeSupportValue,
+ CToolchain toolchainFromCcToolchainAttribute)
+ throws RuleErrorException {
if (cppConfiguration.enableCcToolchainConfigInfoFromSkylark()) {
// Attempt to obtain CppToolchainInfo from the 'toolchain_config' attribute of cc_toolchain.
@@ -676,7 +696,10 @@
}
// Attempt to find a toolchain based on the target attributes, not the configuration.
- CToolchain toolchain = getToolchainFromAttributes(ruleContext, cppConfiguration);
+ CToolchain toolchain = toolchainFromCcToolchainAttribute;
+ if (toolchain == null) {
+ toolchain = getToolchainFromAttributes(ruleContext, cppConfiguration, ccSkyframeSupportValue);
+ }
// If we found a toolchain, use it.
try {
@@ -699,15 +722,14 @@
@Nullable
private CToolchain parseToolchainFromAttributes(RuleContext ruleContext)
throws RuleErrorException {
- if (ruleContext.attributes().get("proto", Type.STRING).isEmpty()) {
+ String protoAttribute = StringUtil.emptyToNull(ruleContext.attributes().get("proto", STRING));
+ if (protoAttribute == null) {
return null;
}
- String data = ruleContext.attributes().get("proto", Type.STRING);
-
CToolchain.Builder builder = CToolchain.newBuilder();
try {
- TextFormat.merge(data, builder);
+ TextFormat.merge(protoAttribute, builder);
return builder.build();
} catch (ParseException e) {
throw ruleContext.throwWithAttributeError("proto", "Could not parse CToolchain data");
@@ -753,25 +775,31 @@
@Nullable
private CToolchain getToolchainFromAttributes(
- RuleContext ruleContext, CppConfiguration cppConfiguration) throws RuleErrorException {
-
- // Is there a toolchain proto available on the target directly?
- CToolchain toolchain = parseToolchainFromAttributes(ruleContext);
- if (toolchain != null) {
- return toolchain;
- }
+ RuleContext ruleContext,
+ CppConfiguration cppConfiguration,
+ CcSkyframeSupportValue ccSkyframeSupportValue)
+ throws RuleErrorException {
String toolchainIdentifier = ruleContext.attributes().get("toolchain_identifier", Type.STRING);
String cpu = ruleContext.attributes().get("cpu", Type.STRING);
String compiler = ruleContext.attributes().get("compiler", Type.STRING);
try {
+ CrosstoolRelease crosstoolRelease;
+ if (cppConfiguration.getCrosstoolFromCcToolchainProtoAttribute() != null) {
+ // We have cc_toolchain_suite.proto attribute set, let's use it
+ crosstoolRelease = cppConfiguration.getCrosstoolFromCcToolchainProtoAttribute();
+ } else {
+ // We use the proto from the CROSSTOOL file
+ crosstoolRelease = ccSkyframeSupportValue.getCrosstoolRelease();
+ }
+
return CToolchainSelectionUtils.selectCToolchain(
toolchainIdentifier,
cpu,
compiler,
cppConfiguration.getTransformedCpuFromOptions(),
cppConfiguration.getCompilerFromOptions(),
- cppConfiguration.getCrosstoolFile().getProto());
+ crosstoolRelease);
} catch (InvalidConfigurationException e) {
ruleContext.throwWithRuleError(
String.format("Error while selecting cc_toolchain: %s", e.getMessage()));
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 0c9e75f..d148191 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
@@ -34,13 +34,13 @@
import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.rules.cpp.CppConfigurationLoader.CppConfigurationParameters;
-import com.google.devtools.build.lib.rules.cpp.CrosstoolConfigurationLoader.CrosstoolFile;
import com.google.devtools.build.lib.rules.cpp.Link.LinkingMode;
import com.google.devtools.build.lib.skylarkbuildapi.cpp.CppConfigurationApi;
import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig;
+import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.CrosstoolRelease;
import java.util.Map;
import javax.annotation.Nullable;
@@ -175,9 +175,16 @@
public static final String FDO_STAMP_MACRO = "BUILD_FDO_TYPE";
private final Label crosstoolTop;
+ /**
+ * cc_toolchain_suite allows to override CROSSTOOL by using proto attribute. This attribute value
+ * is stored here so cc_toolchain can access it in the analysis. Don't use this for anything, it
+ * will be removed when b/113849758 is fixed. If you do, I'll send bubo to take your keyboard
+ * away.
+ */
+ @Deprecated private final CrosstoolRelease crosstoolFromCcToolchainProtoAttribute;
+
private final String transformedCpuFromOptions;
private final String compilerFromOptions;
- private final CrosstoolFile crosstoolFile;
// TODO(lberki): desiredCpu *should* be always the same as targetCpu, except that we don't check
// that the CPU we get from the toolchain matches BuildConfiguration.Options.cpu . So we store
// it here so that the output directory doesn't depend on the CToolchain. When we will eventually
@@ -264,9 +271,9 @@
return new CppConfiguration(
params.crosstoolTop,
+ params.crosstoolFromCcToolchainProtoAttribute,
params.transformedCpu,
params.compiler,
- params.crosstoolFile,
Preconditions.checkNotNull(params.commonOptions.cpu),
crosstoolTopPathFragment,
params.fdoPath,
@@ -301,9 +308,9 @@
private CppConfiguration(
Label crosstoolTop,
+ CrosstoolRelease crosstoolFromCcToolchainProtoAttribute,
String transformedCpuFromOptions,
String compilerFromOptions,
- CrosstoolFile crosstoolFile,
String desiredCpu,
PathFragment crosstoolTopPathFragment,
PathFragment fdoPath,
@@ -330,9 +337,9 @@
boolean shouldProvideMakeVariables,
CppToolchainInfo cppToolchainInfo) {
this.crosstoolTop = crosstoolTop;
+ this.crosstoolFromCcToolchainProtoAttribute = crosstoolFromCcToolchainProtoAttribute;
this.transformedCpuFromOptions = transformedCpuFromOptions;
this.compilerFromOptions = compilerFromOptions;
- this.crosstoolFile = crosstoolFile;
this.desiredCpu = desiredCpu;
this.crosstoolTopPathFragment = crosstoolTopPathFragment;
this.fdoPath = fdoPath;
@@ -390,11 +397,6 @@
return cppToolchainInfo.getToolchainIdentifier();
}
- /** Returns the contents of the CROSSTOOL for this configuration. */
- public CrosstoolFile getCrosstoolFile() {
- return crosstoolFile;
- }
-
/** Returns the label of the CROSSTOOL for this configuration. */
public Label getCrosstoolTop() {
return crosstoolTop;
@@ -1180,6 +1182,17 @@
return cppOptions.disableMakeVariables || !cppOptions.enableMakeVariables;
}
+ /**
+ * cc_toolchain_suite allows to override CROSSTOOL by using proto attribute. This attribute value
+ * is stored here so cc_toolchain can access it in the analysis. Don't use this for anything, it
+ * will be removed when b/113849758 is fixed. If you do, I'll send bubo to take your keyboard
+ * away.
+ */
+ @Deprecated
+ public CrosstoolRelease getCrosstoolFromCcToolchainProtoAttribute() {
+ return crosstoolFromCcToolchainProtoAttribute;
+ }
+
public boolean enableLinkoptsInUserLinkFlags() {
return cppOptions.enableLinkoptsInUserLinkFlags;
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfigurationLoader.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfigurationLoader.java
index d45a5a8..43790ea 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfigurationLoader.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfigurationLoader.java
@@ -14,6 +14,8 @@
package com.google.devtools.build.lib.rules.cpp;
+import static com.google.devtools.build.lib.rules.cpp.CrosstoolConfigurationLoader.toReleaseConfiguration;
+
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.analysis.RedirectChaser;
@@ -32,11 +34,12 @@
import com.google.devtools.build.lib.packages.NonconfigurableAttributeMapper;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.Target;
-import com.google.devtools.build.lib.rules.cpp.CrosstoolConfigurationLoader.CrosstoolFile;
import com.google.devtools.build.lib.syntax.Type;
+import com.google.devtools.build.lib.util.StringUtil;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.CToolchain;
+import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.CrosstoolRelease;
import com.google.devtools.common.options.OptionsParsingException;
import java.util.Map;
import javax.annotation.Nullable;
@@ -79,8 +82,6 @@
* Value class for all the data needed to create a {@link CppConfiguration}.
*/
public static class CppConfigurationParameters {
- protected final CrosstoolConfigurationLoader.CrosstoolFile crosstoolFile;
- protected final String cacheKeySuffix;
protected final BuildConfiguration.Options commonOptions;
protected final CppOptions cppOptions;
protected final Label crosstoolTop;
@@ -92,12 +93,12 @@
protected final CcToolchainConfigInfo ccToolchainConfigInfo;
protected final String transformedCpu;
protected final String compiler;
+ protected final CrosstoolRelease crosstoolFromCcToolchainProtoAttribute;
CppConfigurationParameters(
String transformedCpu,
String compiler,
- CrosstoolFile crosstoolFile,
- String cacheKeySuffix,
+ CrosstoolRelease crosstoolFromCcToolchainProtoAttribute,
BuildOptions buildOptions,
PathFragment fdoPath,
Label fdoOptimizeLabel,
@@ -108,8 +109,7 @@
CcToolchainConfigInfo ccToolchainConfigInfo) {
this.transformedCpu = transformedCpu;
this.compiler = compiler;
- this.crosstoolFile = crosstoolFile;
- this.cacheKeySuffix = cacheKeySuffix;
+ this.crosstoolFromCcToolchainProtoAttribute = crosstoolFromCcToolchainProtoAttribute;
this.commonOptions = buildOptions.get(BuildConfiguration.Options.class);
this.cppOptions = buildOptions.get(CppOptions.class);
this.fdoPath = fdoPath;
@@ -149,9 +149,9 @@
crosstoolTopLabel));
}
- CrosstoolConfigurationLoader.CrosstoolFile file =
+ CrosstoolRelease crosstoolRelease =
CrosstoolConfigurationLoader.readCrosstool(env, crosstoolTopLabel);
- if (file == null) {
+ if (crosstoolRelease == null) {
return null;
}
@@ -161,7 +161,12 @@
transformedCpu + (cppOptions.cppCompiler == null ? "" : ("|" + cppOptions.cppCompiler));
Label ccToolchainLabel =
selectCcToolchainLabel(
- cppOptions, crosstoolTopLabel, (Rule) crosstoolTop, file, transformedCpu, key);
+ cppOptions,
+ crosstoolTopLabel,
+ (Rule) crosstoolTop,
+ crosstoolRelease,
+ transformedCpu,
+ key);
Target ccToolchain = loadCcToolchainTarget(env, ccToolchainLabel);
if (ccToolchain == null) {
@@ -187,7 +192,7 @@
compilerAttribute,
transformedCpu,
cppOptions.cppCompiler,
- file.getProto());
+ crosstoolRelease);
cToolchain =
CppToolchainInfo.addLegacyFeatures(
@@ -204,6 +209,17 @@
}
}
+ String ccToolchainSuiteProtoAttributeValue =
+ StringUtil.emptyToNull(
+ NonconfigurableAttributeMapper.of((Rule) crosstoolTop).get("proto", Type.STRING));
+
+ CrosstoolRelease crosstoolFromCcToolchainProtoAttribute = null;
+ if (ccToolchainSuiteProtoAttributeValue != null) {
+ crosstoolFromCcToolchainProtoAttribute =
+ toReleaseConfiguration(
+ "cc_toolchain_suite rule " + crosstoolTopLabel, ccToolchainSuiteProtoAttributeValue);
+ }
+
PathFragment fdoPath = null;
Label fdoProfileLabel = null;
if (cppOptions.getFdoOptimize() != null) {
@@ -227,8 +243,7 @@
return new CppConfigurationParameters(
transformedCpu,
cppOptions.cppCompiler,
- file,
- file.getMd5(),
+ crosstoolFromCcToolchainProtoAttribute,
options,
fdoPath,
fdoProfileLabel,
@@ -263,7 +278,7 @@
CppOptions cppOptions,
Label crosstoolTopLabel,
Rule crosstoolTop,
- CrosstoolFile file,
+ CrosstoolRelease crosstoolRelease,
String transformedCpu,
String key)
throws InvalidConfigurationException {
@@ -293,7 +308,7 @@
/* compilerAttribute= */ null,
transformedCpu,
compiler,
- file.getProto());
+ crosstoolRelease);
ccToolchainLabel = toolchains.get(toolchain.getTargetCpu() + "|" + toolchain.getCompiler());
if (cppOptions.disableCcToolchainFromCrosstool) {
throw new InvalidConfigurationException(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigurationLoader.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigurationLoader.java
index 693b1a5..370f6cd 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigurationLoader.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigurationLoader.java
@@ -14,8 +14,6 @@
package com.google.devtools.build.lib.rules.cpp;
-import static java.nio.charset.StandardCharsets.UTF_8;
-
import com.google.common.annotations.VisibleForTesting;
import com.google.common.cache.Cache;
import com.google.common.cache.CacheBuilder;
@@ -29,12 +27,9 @@
import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.Target;
-import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.syntax.Type;
-import com.google.devtools.build.lib.util.Fingerprint;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
-import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig;
import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.CrosstoolRelease;
import com.google.protobuf.TextFormat;
import com.google.protobuf.TextFormat.ParseException;
@@ -53,7 +48,7 @@
* done in a saner way.
*/
public class CrosstoolConfigurationLoader {
- private static final String CROSSTOOL_CONFIGURATION_FILENAME = "CROSSTOOL";
+ static final String CROSSTOOL_CONFIGURATION_FILENAME = "CROSSTOOL";
/**
* Cache for storing result of toReleaseConfiguration function based on path and md5 sum of
@@ -61,110 +56,45 @@
*/
private static final Cache<String, CrosstoolRelease> crosstoolReleaseCache =
CacheBuilder.newBuilder().concurrencyLevel(4).maximumSize(100).build();
- /** A class that holds the results of reading a CROSSTOOL file. */
- @AutoCodec
- public static class CrosstoolFile {
- private final String location;
- private final CrosstoolConfig.CrosstoolRelease proto;
- private final String md5;
-
- @AutoCodec.Instantiator
- CrosstoolFile(String location, CrosstoolConfig.CrosstoolRelease proto, String md5) {
- this.location = location;
- this.proto = proto;
- this.md5 = md5;
- }
-
- /**
- * Returns a user-friendly location of the CROSSTOOL proto for error messages.
- */
- public String getLocation() {
- return location;
- }
-
- /**
- * Returns the parsed contents of the CROSSTOOL file.
- */
- public CrosstoolConfig.CrosstoolRelease getProto() {
- return proto;
- }
-
- /**
- * Returns an MD5 hash of the CROSSTOOL file contents.
- */
- public String getMd5() {
- return md5;
- }
- }
private CrosstoolConfigurationLoader() {
}
/**
- * Reads the given <code>data</code> String, which must be in ascii format,
- * into a protocol buffer. It uses the <code>name</code> parameter for error
- * messages.
+ * Reads the given <code>data</code> String, which must be in ascii format, into a protocol
+ * buffer. It uses the <code>name</code> parameter for error messages.
*
* @throws IOException if the parsing failed
*/
@VisibleForTesting
- static CrosstoolConfig.CrosstoolRelease toReleaseConfiguration(String name, String data)
- throws IOException {
- CrosstoolConfig.CrosstoolRelease.Builder builder =
- CrosstoolConfig.CrosstoolRelease.newBuilder();
+ public static CrosstoolRelease toReleaseConfiguration(String name, String data)
+ throws InvalidConfigurationException {
+ CrosstoolRelease.Builder builder = CrosstoolRelease.newBuilder();
try {
TextFormat.merge(data, builder);
return builder.build();
} catch (ParseException e) {
- throw new IOException("Could not read the crosstool configuration file '" + name + "', "
- + "because of a parser error (" + e.getMessage() + ")");
+ throw new InvalidConfigurationException(
+ "Could not read the crosstool configuration file '"
+ + name
+ + "', "
+ + "because of a parser error ("
+ + e.getMessage()
+ + ")");
} catch (UninitializedMessageException e) {
- throw new IOException("Could not read the crosstool configuration file '" + name + "', "
- + "because of an incomplete protocol buffer (" + e.getMessage() + ")");
+ throw new InvalidConfigurationException(
+ "Could not read the crosstool configuration file '"
+ + name
+ + "', "
+ + "because of an incomplete protocol buffer ("
+ + e.getMessage()
+ + ")");
}
}
- /**
- * This class is the in-memory representation of a text-formatted Crosstool proto file.
- *
- * <p>This layer of abstraction is here so that we can load these protos either from BUILD files
- * or from CROSSTOOL files.
- *
- * <p>An implementation of this class should override {@link #getContents()} and call
- * the constructor with the MD5 checksum of what that method will return and a human-readable name
- * used in error messages.
- */
- private abstract static class CrosstoolProto {
- private final byte[] md5;
- private final String name;
-
- private CrosstoolProto(byte[] md5, String name) {
- this.md5 = md5;
- this.name = name;
- }
-
- /**
- * The binary MD5 checksum of the proto.
- */
- public byte[] getMd5() {
- return md5;
- }
-
- /**
- * A user-friendly string describing the location of the proto.
- */
- public String getName() {
- return name;
- }
-
- /**
- * The proto itself.
- */
- public abstract String getContents() throws IOException;
- }
-
- private static CrosstoolProto getCrosstoolProtofromBuildFile(
- ConfigurationEnvironment env, Label crosstoolTop) throws InterruptedException {
+ private static CrosstoolRelease getCrosstoolProtoFromBuildFile(
+ ConfigurationEnvironment env, Label crosstoolTop)
+ throws InterruptedException, InvalidConfigurationException {
Target target;
try {
target = env.getTarget(crosstoolTop);
@@ -183,22 +113,22 @@
}
final String contents = NonconfigurableAttributeMapper.of(rule).get("proto", Type.STRING);
- byte[] md5 = new Fingerprint().addBytes(contents.getBytes(UTF_8)).digestAndReset();
- return new CrosstoolProto(md5, "cc_toolchain_suite rule " + crosstoolTop.toString()) {
-
- @Override
- public String getContents() throws IOException {
- return contents;
- }
- };
+ return toReleaseConfiguration("cc_toolchain_suite rule " + crosstoolTop, contents);
}
- private static CrosstoolProto getCrosstoolProtoFromCrosstoolFile(
+ private static CrosstoolRelease findCrosstoolConfiguration(
ConfigurationEnvironment env, Label crosstoolTop)
throws IOException, InvalidConfigurationException, InterruptedException {
- final Path path;
+
+ CrosstoolRelease crosstoolProtoFromBuildFile =
+ getCrosstoolProtoFromBuildFile(env, crosstoolTop);
+ if (crosstoolProtoFromBuildFile != null) {
+ return crosstoolProtoFromBuildFile;
+ }
+ Path path;
try {
- Package containingPackage = env.getTarget(crosstoolTop).getPackage();
+ Package containingPackage;
+ containingPackage = env.getTarget(crosstoolTop).getPackage();
if (containingPackage == null) {
return null;
}
@@ -209,54 +139,40 @@
}
if (path == null || !path.exists()) {
- return null;
- }
-
- return new CrosstoolProto(path.getDigest(), "CROSSTOOL file " + path.getPathString()) {
- @Override
- public String getContents() throws IOException {
- try (InputStream inputStream = path.getInputStream()) {
- return new String(FileSystemUtils.readContentAsLatin1(inputStream));
- }
- }
- };
- }
-
- private static CrosstoolFile findCrosstoolConfiguration(
- ConfigurationEnvironment env, Label crosstoolTop)
- throws IOException, InvalidConfigurationException, InterruptedException {
-
- CrosstoolProto crosstoolProto = getCrosstoolProtofromBuildFile(env, crosstoolTop);
- if (crosstoolProto == null) {
- crosstoolProto = getCrosstoolProtoFromCrosstoolFile(env, crosstoolTop);
- }
-
- if (crosstoolProto == null) {
+ // Normally you'd expect to return null when path is null (so Skyframe computes the value),
+ // and throw when path doesn't exist. But because {@link ConfigurationFragmentFunction}
+ // doesn't propagate the exceptions when there are valuesMissing(), we need to throw
+ // this exception always just to be sure it gets through.
throw new InvalidConfigurationException("The crosstool_top you specified was resolved to '" +
crosstoolTop + "', which does not contain a CROSSTOOL file.");
- } else {
- // Do this before we read the data, so if it changes, we get a different MD5 the next time.
- // Alternatively, we could calculate the MD5 of the contents, which we also read, but this
- // is faster if the file comes from a file system with md5 support.
- final CrosstoolProto finalProto = crosstoolProto;
- String md5 = BaseEncoding.base16().lowerCase().encode(finalProto.getMd5());
- CrosstoolConfig.CrosstoolRelease release;
- try {
- release =
- crosstoolReleaseCache.get(
- md5, () -> toReleaseConfiguration(finalProto.getName(), finalProto.getContents()));
- } catch (ExecutionException e) {
- throw new InvalidConfigurationException(e);
- }
-
- return new CrosstoolFile(finalProto.getName(), release, md5);
}
+
+ // Do this before we read the data, so if it changes, we get a different MD5 the next time.
+ // Alternatively, we could calculate the MD5 of the contents, which we also read, but this
+ // is faster if the file comes from a file system with md5 support.
+ String md5 = BaseEncoding.base16().lowerCase().encode(path.getDigest());
+ CrosstoolRelease release;
+ try {
+ release =
+ crosstoolReleaseCache.get(
+ md5,
+ () -> {
+ try (InputStream inputStream = path.getInputStream()) {
+ return toReleaseConfiguration(
+ "CROSSTOOL file " + path,
+ new String(FileSystemUtils.readContentAsLatin1(inputStream)));
+ }
+ });
+ } catch (ExecutionException e) {
+ throw new InvalidConfigurationException(e);
+ }
+
+ return release;
}
/** Reads a crosstool file. */
@Nullable
- public static CrosstoolConfigurationLoader.CrosstoolFile readCrosstool(
- ConfigurationEnvironment env, Label crosstoolTop)
+ public static CrosstoolRelease readCrosstool(ConfigurationEnvironment env, Label crosstoolTop)
throws InvalidConfigurationException, InterruptedException {
crosstoolTop = RedirectChaser.followRedirects(env, crosstoolTop, "crosstool_top");
if (crosstoolTop == null) {
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisMock.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisMock.java
index 03368f63..588e526 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisMock.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisMock.java
@@ -26,8 +26,8 @@
import com.google.devtools.build.lib.packages.util.MockCcSupport;
import com.google.devtools.build.lib.packages.util.MockPythonSupport;
import com.google.devtools.build.lib.packages.util.MockToolsConfig;
+import com.google.devtools.build.lib.rules.cpp.CcSkyframeSupportFunction;
import com.google.devtools.build.lib.rules.cpp.CcSkyframeSupportValue;
-import com.google.devtools.build.lib.rules.cpp.CcSupportFunction;
import com.google.devtools.build.lib.rules.repository.LocalRepositoryFunction;
import com.google.devtools.build.lib.rules.repository.LocalRepositoryRule;
import com.google.devtools.build.lib.rules.repository.RepositoryDelegatorFunction;
@@ -136,7 +136,7 @@
SkyFunctions.REPOSITORY,
new RepositoryLoaderFunction(),
CcSkyframeSupportValue.SKYFUNCTION,
- new CcSupportFunction(directories));
+ new CcSkyframeSupportFunction(directories));
}
public static class Delegate extends AnalysisMock {
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainTest.java
index 843fc93c..74bb8f5 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainTest.java
@@ -105,6 +105,7 @@
.setSupportsInterfaceSharedObjects(false)
.buildPartial());
useConfiguration();
+ invalidatePackages();
ConfiguredTarget target = getConfiguredTarget("//a:b");
CcToolchainProvider toolchainProvider =
@@ -115,6 +116,7 @@
.isFalse();
useConfiguration("--interface_shared_objects");
+ invalidatePackages();
target = getConfiguredTarget("//a:b");
toolchainProvider = (CcToolchainProvider) target.get(ToolchainInfo.PROVIDER);
assertThat(
@@ -130,6 +132,7 @@
.setSupportsInterfaceSharedObjects(true)
.buildPartial());
useConfiguration();
+ invalidatePackages();
target = getConfiguredTarget("//a:b");
toolchainProvider = (CcToolchainProvider) target.get(ToolchainInfo.PROVIDER);
@@ -139,6 +142,7 @@
.isTrue();
useConfiguration("--nointerface_shared_objects");
+ invalidatePackages();
target = getConfiguredTarget("//a:b");
toolchainProvider = (CcToolchainProvider) target.get(ToolchainInfo.PROVIDER);
assertThat(
@@ -484,6 +488,7 @@
.buildPartial());
useConfiguration();
+ invalidatePackages();
ConfiguredTarget target = getConfiguredTarget("//a:b");
CcToolchainProvider toolchainProvider =
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigurationLoaderTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigurationLoaderTest.java
index de8db5b..90d833e 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigurationLoaderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigurationLoaderTest.java
@@ -785,7 +785,7 @@
try {
CrosstoolConfigurationLoader.toReleaseConfiguration("/CROSSTOOL", "major_version: \"12\"");
fail();
- } catch (IOException e) {
+ } catch (InvalidConfigurationException e) {
assertStringStartsWith(
"Could not read the crosstool configuration file "
+ "'/CROSSTOOL', because of an incomplete protocol buffer",
@@ -872,7 +872,7 @@
try {
CrosstoolConfigurationLoader.toReleaseConfiguration("/CROSSTOOL", "some xxx : yak \"");
fail();
- } catch (IOException e) {
+ } catch (InvalidConfigurationException e) {
assertStringStartsWith(
"Could not read the crosstool configuration file "
+ "'/CROSSTOOL', because of a parser error",
