Skip to content

Commit

Permalink
Do not depend on CppConfiguration to provide CROSSTOOL file to cc_too…
Browse files Browse the repository at this point in the history
…lchain

Use CcSkyframeSupportFunction instead.

#6072

RELNOTES: None.
PiperOrigin-RevId: 214004051
  • Loading branch information
hlopko authored and Copybara-Service committed Sep 21, 2018
1 parent 3936191 commit 57a1de5
Show file tree
Hide file tree
Showing 11 changed files with 324 additions and 281 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -99,7 +99,8 @@ public void initializeRuleClasses(ConfiguredRuleClassProvider.Builder builder) {
@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
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
*/
Expand All @@ -45,39 +46,45 @@ public class CcSkyframeSupportValue implements SkyValue {
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
public boolean equals(Object o) {
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
Expand All @@ -88,19 +95,26 @@ public SkyFunctionName functionName() {

/** 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;

private final CrosstoolRelease crosstoolRelease;

CcSkyframeSupportValue(Path fdoZipPath, CrosstoolRelease crosstoolRelease) {
this.fdoZipPath = fdoZipPath;
this.crosstoolRelease = crosstoolRelease;
}

CcSkyframeSupportValue(Path filePath) {
this.filePath = filePath;
public Path getFdoZipPath() {
return fdoZipPath;
}

public Path getFilePath() {
return filePath;
public CrosstoolRelease getCrosstoolRelease() {
return crosstoolRelease;
}

public static SkyKey key(PathFragment fdoProfileArgument) {
return Key.of(fdoProfileArgument);
public static SkyKey key(PathFragment fdoZipPath, PathFragment crosstoolPath) {
return Key.of(fdoZipPath, crosstoolPath);
}
}

This file was deleted.

Loading

0 comments on commit 57a1de5

Please sign in to comment.