Skip to content

Commit

Permalink
Put the hack into CppCompileAction and fix nits.
Browse files Browse the repository at this point in the history
  • Loading branch information
coeuvre committed Apr 14, 2021
1 parent baa440f commit 8216e7d
Show file tree
Hide file tree
Showing 8 changed files with 36 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -260,4 +260,9 @@ public enum WorkerProtocolFormat {
* "requires-xcode-label:unstable" and "requires-xcode:1.0".
*/
public static final String REQUIRES_XCODE_LABEL = "requires-xcode-label";

/**
* Requires the execution service do NOT share caches across different workspace.
*/
public static final String DIFFERENTIATE_WORKSPACE_CACHE = "internal-differentiate-workspace-cache";
}
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,12 @@ public static boolean requiresNetwork(Spawn spawn, boolean defaultSandboxDisallo
return defaultSandboxDisallowNetwork;
}

/** Returns whether caches from other workspace can be used for this spawn. */
public static boolean shouldDifferentiateWorkspaceCache(Spawn spawn) {
return spawn.getExecutionInfo()
.containsKey(ExecutionRequirements.DIFFERENTIATE_WORKSPACE_CACHE);
}

/**
* Returns whether a Spawn claims to support being executed with the persistent worker strategy
* according to its execution info tags.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
import com.google.devtools.build.lib.remote.common.RemoteExecutionClient;
import com.google.devtools.build.lib.remote.common.RemotePathResolver;
import com.google.devtools.build.lib.remote.common.RemotePathResolver.DefaultRemotePathResolver;
import com.google.devtools.build.lib.remote.common.RemotePathResolver.SiblingExternalLayoutResolver;
import com.google.devtools.build.lib.remote.common.RemotePathResolver.SiblingRepositoryLayoutResolver;
import com.google.devtools.build.lib.remote.options.RemoteOptions;
import com.google.devtools.build.lib.remote.util.DigestUtil;
import com.google.devtools.build.lib.runtime.CommandEnvironment;
Expand Down Expand Up @@ -91,7 +91,7 @@ RemotePathResolver createRemotePathResolver() {
RemotePathResolver remotePathResolver;
if (buildLanguageOptions != null && buildLanguageOptions.experimentalSiblingRepositoryLayout) {
RemoteOptions remoteOptions = checkNotNull(env.getOptions().getOptions(RemoteOptions.class));
remotePathResolver = new SiblingExternalLayoutResolver(execRoot, remoteOptions.incompatibleRemoteOutputPathsRelativeToInputRoot);
remotePathResolver = new SiblingRepositoryLayoutResolver(execRoot, remoteOptions.incompatibleRemoteOutputPathsRelativeToInputRoot);
} else {
remotePathResolver = new DefaultRemotePathResolver(execRoot);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,8 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context)
maybeWriteParamFilesLocally(spawn);

// Get the remote platform properties.
Platform platform = PlatformUtils.getPlatformProto(spawn, remoteOptions);
Platform platform = PlatformUtils
.getPlatformProto(spawn, remoteOptions, getExtraPlatformProperties(spawn, execRoot));

Command command =
buildCommand(
Expand Down Expand Up @@ -698,16 +699,10 @@ private SpawnResult handleError(
}

static List<Property> getExtraPlatformProperties(Spawn spawn, Path execRoot) {
// Hack on Windows. The included headers dumped by cl.exe in stdout contain absolute paths.
// When compiling the file from different workspace, the shared cache will cause header
// dependency checking to fail. This was initially fixed by a hack (see
// https://github.com/bazelbuild/bazel/issues/9172 for more details), but is broken again due
// to cl/356735700. We include workspace name here so action results from different workspaces
// on Windows won't be shared.
boolean isWindows = System.getProperty("os.name").startsWith("Windows");
if (isWindows) {
if (Spawns.shouldDifferentiateWorkspaceCache(spawn)) {
return ImmutableList
.of(Property.newBuilder().setName("workspace").setValue(execRoot.getBaseName()).build());
.of(Property.newBuilder().setName("bazel-workspace").setValue(execRoot.getBaseName())
.build());
}

return ImmutableList.of();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,20 +130,20 @@ public Path resolveLocalPath(ActionInput actionInput) {
}

/**
* A {@link RemotePathResolver} used when {@code --experimental_sibling_external_layout} is set.
* A {@link RemotePathResolver} used when {@code --experimental_sibling_repository_layout} is set.
* Use parent directory of {@code execRoot} and set {@code workingDirectory} to the base name of
* {@code execRoot}.
*
* The paths of outputs are relative to {@code workingDirectory} if
* {@code --incompatible_remote_output_paths_relative_to_input_root} is not set, otherwise,
* relative to input root.
*/
class SiblingExternalLayoutResolver implements RemotePathResolver {
class SiblingRepositoryLayoutResolver implements RemotePathResolver {

private final Path execRoot;
private final boolean incompatibleRemoteOutputPathsRelativeToInputRoot;

public SiblingExternalLayoutResolver(Path execRoot,
public SiblingRepositoryLayoutResolver(Path execRoot,
boolean incompatibleRemoteOutputPathsRelativeToInputRoot) {
this.execRoot = execRoot;
this.incompatibleRemoteOutputPathsRelativeToInputRoot = incompatibleRemoteOutputPathsRelativeToInputRoot;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,13 +62,6 @@ static DirectoryTree fromActionInputs(
throws IOException {
Map<PathFragment, DirectoryNode> tree = new HashMap<>();
int numFiles = buildFromActionInputs(inputs, metadataProvider, execRoot, digestUtil, tree);
// // Make sure working directory is exists
// PathFragment workingDirectory = PathFragment.create(execRoot.getBaseName());
// if (!tree.containsKey(workingDirectory)) {
// DirectoryNode dir = new DirectoryNode(workingDirectory.toString());
// tree.put(workingDirectory, dir);
// createParentDirectoriesIfNotExist(workingDirectory, dir, tree);
// }
return new DirectoryTree(tree, numFiles);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1477,6 +1477,19 @@ protected Spawn createSpawn(Map<String, String> clientEnv) throws ActionExecutio
.build();
}

if (featureConfiguration.isEnabled(CppRuleClasses.PARSE_SHOWINCLUDES)) {
// Hack on Windows. The included headers dumped by cl.exe in stdout contain absolute paths.
// When compiling the file from different workspace, the shared cache will cause header
// dependency checking to fail. This was initially fixed by a hack (see
// https://github.com/bazelbuild/bazel/issues/9172 for more details), but is broken again due
// to cl/356735700. We require execution service to ignore caches from other workspace.
executionInfo =
ImmutableMap.<String, String>builderWithExpectedSize(executionInfo.size() + 1)
.putAll(executionInfo)
.put(ExecutionRequirements.DIFFERENTIATE_WORKSPACE_CACHE, "")
.build();
}

try {
return new SimpleSpawn(
this,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext;
import com.google.devtools.build.lib.remote.common.RemoteCacheClient.ActionKey;
import com.google.devtools.build.lib.remote.common.RemotePathResolver;
import com.google.devtools.build.lib.remote.common.RemotePathResolver.SiblingExternalLayoutResolver;
import com.google.devtools.build.lib.remote.common.RemotePathResolver.SiblingRepositoryLayoutResolver;
import com.google.devtools.build.lib.remote.grpc.ChannelConnectionFactory;
import com.google.devtools.build.lib.remote.merkletree.MerkleTree;
import com.google.devtools.build.lib.remote.options.RemoteOptions;
Expand Down Expand Up @@ -402,7 +402,7 @@ public void testDownloadAllResultsForSiblingLayoutAndRelativeToInputRoot() throw
RemoteOptions remoteOptions = Options.getDefaults(RemoteOptions.class);
GrpcCacheClient client = newClient(remoteOptions);
RemoteCache remoteCache = new RemoteCache(client, remoteOptions, DIGEST_UTIL);
RemotePathResolver remotePathResolver = new SiblingExternalLayoutResolver(execRoot, true);
RemotePathResolver remotePathResolver = new SiblingRepositoryLayoutResolver(execRoot, true);

Digest fooDigest = DIGEST_UTIL.computeAsUtf8("foo-contents");
Digest barDigest = DIGEST_UTIL.computeAsUtf8("bar-contents");
Expand Down

0 comments on commit 8216e7d

Please sign in to comment.