Skip to content

Commit

Permalink
Extract LocalEnvProvider members to classes.
Browse files Browse the repository at this point in the history
Create a PosixLocalEnvProvider and
WindowsLocalEnvProvider class, with singleton
instances for now.

This refactoring should not change functionality,
it's just a requirement for an upcoming change.

That upcoming change is for these classes to
respect the client environment's TMPDIR or
TMP/TEMP envvars.

See bazelbuild#4376

Change-Id: I032bb6f18adf8af9e43e6bc543c09c58adae3863
PiperOrigin-RevId: 180799936
  • Loading branch information
laszlocsomor authored and Copybara-Service committed Jan 4, 2018
1 parent f475101 commit a495bae
Show file tree
Hide file tree
Showing 9 changed files with 102 additions and 41 deletions.
2 changes: 2 additions & 0 deletions src/main/java/com/google/devtools/build/lib/exec/local/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ java_library(
srcs = [
"LocalEnvProvider.java",
"LocalSpawnRunner.java",
"PosixLocalEnvProvider.java",
"WindowsLocalEnvProvider.java",
],
data = [
"//src/main/tools:process-wrapper",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@
// limitations under the License.
package com.google.devtools.build.lib.exec.local;

import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Maps;
import com.google.devtools.build.lib.vfs.Path;
import java.io.IOException;
import java.util.Map;
Expand All @@ -35,34 +33,6 @@ public Map<String, String> rewriteLocalEnv(
}
};

public static final LocalEnvProvider ADD_TEMP_POSIX =
new LocalEnvProvider() {
@Override
public Map<String, String> rewriteLocalEnv(
Map<String, String> env, Path execRoot, Path tmpDir, String productName)
throws IOException {
ImmutableMap.Builder<String, String> result = ImmutableMap.builder();
result.putAll(Maps.filterKeys(env, k -> !k.equals("TMPDIR")));
result.put("TMPDIR", tmpDir.getPathString());
return result.build();
}
};

public static final LocalEnvProvider ADD_TEMP_WINDOWS =
new LocalEnvProvider() {
@Override
public Map<String, String> rewriteLocalEnv(
Map<String, String> env, Path execRoot, Path tmpDir, String productName)
throws IOException {
ImmutableMap.Builder<String, String> result = ImmutableMap.builder();
result.putAll(Maps.filterKeys(env, k -> !k.equals("TMP") && !k.equals("TEMP")));
String tmpPath = tmpDir.getPathString().replace('/', '\\');
result.put("TMP", tmpPath);
result.put("TEMP", tmpPath);
return result.build();
}
};

/** Rewrites the environment if necessary. */
Map<String, String> rewriteLocalEnv(
Map<String, String> env, Path execRoot, Path tmpDir, String productName) throws IOException;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// Copyright 2018 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.exec.local;

import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Maps;
import com.google.devtools.build.lib.vfs.Path;
import java.io.IOException;
import java.util.Map;

/** {@link LocalEnvProvider} implementation for actions running on Unix-like platforms. */
public final class PosixLocalEnvProvider implements LocalEnvProvider {

public static final PosixLocalEnvProvider INSTANCE = new PosixLocalEnvProvider();

/**
* Compute an environment map for local actions on Unix-like platforms (e.g. Linux, macOS).
*
* <p>Returns a map with the same keys and values as {@code env}. Overrides the value of TMPDIR
* (or adds it if not present in {@code env}) by {@code tmpDir}.
*/
@Override
public Map<String, String> rewriteLocalEnv(
Map<String, String> env, Path execRoot, Path tmpDir, String productName) throws IOException {
ImmutableMap.Builder<String, String> result = ImmutableMap.builder();
result.putAll(Maps.filterKeys(env, k -> !k.equals("TMPDIR")));
result.put("TMPDIR", tmpDir.getPathString());
return result.build();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// Copyright 2018 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.exec.local;

import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Maps;
import com.google.devtools.build.lib.vfs.Path;
import java.io.IOException;
import java.util.Map;

/** {@link LocalEnvProvider} implementation for actions running on Windows. */
public final class WindowsLocalEnvProvider implements LocalEnvProvider {

public static final WindowsLocalEnvProvider INSTANCE = new WindowsLocalEnvProvider();

/**
* Compute an environment map for local actions on Windows.
*
* <p>Returns a map with the same keys and values as {@code env}. Overrides the value of TMP and
* TEMP (or adds them if not present in {@code env}) by {@code tmpDir}.
*
* <p>The values for TMP and TEMP will use backslashes as directory separators.
*/
@Override
public Map<String, String> rewriteLocalEnv(
Map<String, String> env, Path execRoot, Path tmpDir, String productName) throws IOException {
ImmutableMap.Builder<String, String> result = ImmutableMap.builder();
result.putAll(Maps.filterKeys(env, k -> !k.equals("TMP") && !k.equals("TEMP")));
String tmpPath = tmpDir.getPathString().replace('/', '\\');
result.put("TMP", tmpPath);
result.put("TEMP", tmpPath);
return result.build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import com.google.devtools.build.lib.actions.UserExecException;
import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.exec.local.LocalEnvProvider;
import com.google.devtools.build.lib.exec.local.PosixLocalEnvProvider;
import com.google.devtools.build.lib.runtime.CommandEnvironment;
import com.google.devtools.build.lib.runtime.LinuxSandboxUtil;
import com.google.devtools.build.lib.shell.Command;
Expand Down Expand Up @@ -166,7 +167,7 @@ public static boolean isSupported(CommandEnvironment cmdEnv) {
this.inaccessibleHelperFile = inaccessibleHelperFile;
this.inaccessibleHelperDir = inaccessibleHelperDir;
this.timeoutKillDelay = timeoutKillDelay;
this.localEnvProvider = LocalEnvProvider.ADD_TEMP_POSIX;
this.localEnvProvider = PosixLocalEnvProvider.INSTANCE;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import com.google.devtools.build.lib.actions.SpawnResult;
import com.google.devtools.build.lib.exec.apple.XCodeLocalEnvProvider;
import com.google.devtools.build.lib.exec.local.LocalEnvProvider;
import com.google.devtools.build.lib.exec.local.PosixLocalEnvProvider;
import com.google.devtools.build.lib.runtime.CommandEnvironment;
import com.google.devtools.build.lib.runtime.ProcessWrapperUtil;
import com.google.devtools.build.lib.util.OS;
Expand Down Expand Up @@ -88,9 +89,7 @@ public static boolean isSupported(CommandEnvironment cmdEnv) {
this.timeoutKillDelay = timeoutKillDelay;
this.processWrapper = ProcessWrapperUtil.getProcessWrapper(cmdEnv);
this.localEnvProvider =
OS.getCurrent() == OS.DARWIN
? new XCodeLocalEnvProvider()
: LocalEnvProvider.ADD_TEMP_POSIX;
OS.getCurrent() == OS.DARWIN ? new XCodeLocalEnvProvider() : PosixLocalEnvProvider.INSTANCE;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import com.google.devtools.build.lib.exec.local.LocalEnvProvider;
import com.google.devtools.build.lib.exec.local.LocalExecutionOptions;
import com.google.devtools.build.lib.exec.local.LocalSpawnRunner;
import com.google.devtools.build.lib.exec.local.PosixLocalEnvProvider;
import com.google.devtools.build.lib.runtime.CommandEnvironment;
import com.google.devtools.build.lib.util.OS;
import com.google.devtools.build.lib.vfs.Path;
Expand Down Expand Up @@ -98,9 +99,7 @@ private static SpawnRunner createFallbackRunner(CommandEnvironment env) {
LocalExecutionOptions localExecutionOptions =
env.getOptions().getOptions(LocalExecutionOptions.class);
LocalEnvProvider localEnvProvider =
OS.getCurrent() == OS.DARWIN
? new XCodeLocalEnvProvider()
: LocalEnvProvider.ADD_TEMP_POSIX;
OS.getCurrent() == OS.DARWIN ? new XCodeLocalEnvProvider() : PosixLocalEnvProvider.INSTANCE;
return
new LocalSpawnRunner(
env.getExecRoot(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
import com.google.devtools.build.lib.exec.local.LocalEnvProvider;
import com.google.devtools.build.lib.exec.local.LocalExecutionOptions;
import com.google.devtools.build.lib.exec.local.LocalSpawnRunner;
import com.google.devtools.build.lib.exec.local.PosixLocalEnvProvider;
import com.google.devtools.build.lib.exec.local.WindowsLocalEnvProvider;
import com.google.devtools.build.lib.rules.cpp.IncludeScanningContext;
import com.google.devtools.build.lib.rules.cpp.SpawnGccStrategy;
import com.google.devtools.build.lib.rules.test.ExclusiveTestStrategy;
Expand Down Expand Up @@ -104,8 +106,8 @@ private static SpawnRunner createLocalRunner(CommandEnvironment env) {
OS.getCurrent() == OS.DARWIN
? new XCodeLocalEnvProvider()
: (OS.getCurrent() == OS.WINDOWS
? LocalEnvProvider.ADD_TEMP_WINDOWS
: LocalEnvProvider.ADD_TEMP_POSIX);
? WindowsLocalEnvProvider.INSTANCE
: PosixLocalEnvProvider.INSTANCE);
return
new LocalSpawnRunner(
env.getExecRoot(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
import com.google.devtools.build.lib.exec.local.LocalEnvProvider;
import com.google.devtools.build.lib.exec.local.LocalExecutionOptions;
import com.google.devtools.build.lib.exec.local.LocalSpawnRunner;
import com.google.devtools.build.lib.exec.local.PosixLocalEnvProvider;
import com.google.devtools.build.lib.exec.local.WindowsLocalEnvProvider;
import com.google.devtools.build.lib.runtime.CommandEnvironment;
import com.google.devtools.build.lib.util.OS;

Expand Down Expand Up @@ -58,8 +60,8 @@ private static SpawnRunner createFallbackRunner(CommandEnvironment env) {
OS.getCurrent() == OS.DARWIN
? new XCodeLocalEnvProvider()
: (OS.getCurrent() == OS.WINDOWS
? LocalEnvProvider.ADD_TEMP_WINDOWS
: LocalEnvProvider.ADD_TEMP_POSIX);
? WindowsLocalEnvProvider.INSTANCE
: PosixLocalEnvProvider.INSTANCE);
return new LocalSpawnRunner(
env.getExecRoot(),
localExecutionOptions,
Expand Down

0 comments on commit a495bae

Please sign in to comment.