From d92db2d4ecfee707860dca67393e91769c68fdb4 Mon Sep 17 00:00:00 2001 From: lberki Date: Tue, 3 Apr 2018 01:59:32 -0700 Subject: Remove CcToolchainProvider#getEnvironment() and all the supporting infrastructure. This was added in unknown commit to provide a different environment to Apple toolchains, then its use removed in unknown commit in favor of getting the environment variables from the CToolchain proto. I haven't done my research if that's a better approach, but it looks like it (the less hard-coded stuff we have in Java, the better), but worst of all is surely to have *two* such mechanisms. RELNOTES: None. PiperOrigin-RevId: 191411878 --- .../devtools/build/lib/rules/cpp/CcCompilationHelper.java | 1 - .../com/google/devtools/build/lib/rules/cpp/CcToolchain.java | 11 ----------- .../devtools/build/lib/rules/cpp/CcToolchainProvider.java | 8 -------- .../google/devtools/build/lib/rules/cpp/CppCompileAction.java | 8 -------- .../devtools/build/lib/rules/cpp/CppCompileActionBuilder.java | 8 -------- .../devtools/build/lib/rules/cpp/FakeCppCompileAction.java | 1 - .../devtools/build/lib/rules/cpp/CcToolchainProviderTest.java | 2 -- 7 files changed, 39 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java index 7ac3f194b5..102b42427c 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java @@ -1509,7 +1509,6 @@ public final class CcCompilationHelper { new CppCompileActionBuilder(ruleContext, ccToolchain, configuration); builder.setSourceFile(source); builder.setCcCompilationInfo(ccCompilationInfo); - builder.addEnvironment(ccToolchain.getEnvironment()); builder.setCoptsFilter(coptsFilter); return builder; } 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 e87c34ff08..c99bf1d24a 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 @@ -554,7 +554,6 @@ public class CcToolchain implements RuleConfiguredTargetFactory { toolchainInfo.supportsInterfaceSharedObjects() ? ruleContext.getPrerequisiteArtifact("$link_dynamic_library_tool", Mode.HOST) : null, - getEnvironment(ruleContext), builtInIncludeDirectories, sysroot, fdoMode); @@ -846,16 +845,6 @@ public class CcToolchain implements RuleConfiguredTargetFactory { // To be overridden in subclasses. } - /** - * Returns a map of environment variables to be added to the compile actions created for this - * toolchain. Ideally, this will get replaced by features, which also allow setting env variables. - * - * @param ruleContext the rule context - */ - protected ImmutableMap getEnvironment(RuleContext ruleContext) { - return ImmutableMap.of(); - } - private PathFragment calculateSysroot(RuleContext ruleContext, PathFragment defaultSysroot) { TransitiveInfoCollection sysrootTarget = ruleContext.getPrerequisite(":libc_top", Mode.TARGET); 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 43fb642aa7..5347f5175b 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 @@ -81,7 +81,6 @@ public final class CcToolchainProvider extends ToolchainInfo { /* builtinIncludeFiles= */ ImmutableList.of(), /* coverageEnvironment= */ NestedSetBuilder.emptySet(Order.COMPILE_ORDER), /* linkDynamicLibraryTool= */ null, - /* environment= */ ImmutableMap.of(), /* builtInIncludeDirectories= */ ImmutableList.of(), /* sysroot= */ null, FdoMode.OFF); @@ -113,7 +112,6 @@ public final class CcToolchainProvider extends ToolchainInfo { private final ImmutableList builtinIncludeFiles; private final NestedSet> coverageEnvironment; @Nullable private final Artifact linkDynamicLibraryTool; - private final ImmutableMap environment; private final ImmutableList builtInIncludeDirectories; @Nullable private final PathFragment sysroot; private final FdoMode fdoMode; @@ -147,7 +145,6 @@ public final class CcToolchainProvider extends ToolchainInfo { ImmutableList builtinIncludeFiles, NestedSet> coverageEnvironment, Artifact linkDynamicLibraryTool, - ImmutableMap environment, ImmutableList builtInIncludeDirectories, @Nullable PathFragment sysroot, FdoMode fdoMode) { @@ -179,7 +176,6 @@ public final class CcToolchainProvider extends ToolchainInfo { this.builtinIncludeFiles = builtinIncludeFiles; this.coverageEnvironment = coverageEnvironment; this.linkDynamicLibraryTool = linkDynamicLibraryTool; - this.environment = environment; this.builtInIncludeDirectories = builtInIncludeDirectories; this.sysroot = sysroot; this.fdoMode = fdoMode; @@ -508,10 +504,6 @@ public final class CcToolchainProvider extends ToolchainInfo { return coverageEnvironment; } - public ImmutableMap getEnvironment() { - return environment; - } - /** * Returns the tool which should be used for linking dynamic libraries, or in case it's not * specified by the crosstool this will be @tools_repository/tools/cpp:link_dynamic_library 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 913088648f..9d42abea81 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 @@ -194,7 +194,6 @@ public class CppCompileAction extends AbstractAction private final ImmutableList additionalIncludeScanningRoots; @VisibleForTesting public final CompileCommandLine compileCommandLine; private final ImmutableMap executionInfo; - private final ImmutableMap environment; private final String actionName; private final FeatureConfiguration featureConfiguration; @@ -256,7 +255,6 @@ public class CppCompileAction extends AbstractAction * @param lipoScannables List of artifacts to include-scan when this action is a lipo action * @param additionalIncludeScanningRoots list of additional artifacts to include-scan * @param actionClassId TODO(bazel-team): Add parameter description - * @param environment TODO(bazel-team): Add parameter description * @param actionName a string giving the name of this action for the purpose of toolchain * evaluation * @param cppSemantics C++ compilation semantics @@ -291,7 +289,6 @@ public class CppCompileAction extends AbstractAction ImmutableList additionalIncludeScanningRoots, UUID actionClassId, ImmutableMap executionInfo, - ImmutableMap environment, String actionName, CppSemantics cppSemantics, CcToolchainProvider cppProvider, @@ -333,7 +330,6 @@ public class CppCompileAction extends AbstractAction .setVariables(variables) .build(), executionInfo, - environment, actionName, featureConfiguration, actionClassId, @@ -374,7 +370,6 @@ public class CppCompileAction extends AbstractAction ImmutableList additionalIncludeScanningRoots, CompileCommandLine compileCommandLine, ImmutableMap executionInfo, - ImmutableMap environment, String actionName, FeatureConfiguration featureConfiguration, UUID actionClassId, @@ -407,7 +402,6 @@ public class CppCompileAction extends AbstractAction this.additionalIncludeScanningRoots = additionalIncludeScanningRoots; this.compileCommandLine = compileCommandLine; this.executionInfo = executionInfo; - this.environment = environment; this.actionName = actionName; this.featureConfiguration = featureConfiguration; this.needsDotdInputPruning = needsDotdInputPruning; @@ -771,9 +765,7 @@ public class CppCompileAction extends AbstractAction environment.put("PWD", "/proc/self/cwd"); } - environment.putAll(this.environment); environment.putAll(compileCommandLine.getEnvironment()); - return ImmutableMap.copyOf(environment); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java index aefe027332..6a5271520a 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java @@ -75,7 +75,6 @@ public class CppCompileActionBuilder { private final ImmutableList.Builder additionalIncludeScanningRoots; private Boolean shouldScanIncludes; private Map executionInfo = new LinkedHashMap<>(); - private Map environment = new LinkedHashMap<>(); private CppSemantics cppSemantics; private CcToolchainProvider ccToolchain; @Nullable private final Artifact grepIncludes; @@ -160,7 +159,6 @@ public class CppCompileActionBuilder { this.lipoScannableMap = other.lipoScannableMap; this.shouldScanIncludes = other.shouldScanIncludes; this.executionInfo = new LinkedHashMap<>(other.executionInfo); - this.environment = new LinkedHashMap<>(other.environment); this.localShellEnvironment = other.localShellEnvironment; this.codeCoverageEnabled = other.codeCoverageEnabled; this.cppSemantics = other.cppSemantics; @@ -420,7 +418,6 @@ public class CppCompileActionBuilder { additionalIncludeScanningRoots.build(), actionClassId, ImmutableMap.copyOf(executionInfo), - ImmutableMap.copyOf(environment), getActionName(), cppSemantics, ccToolchain, @@ -542,11 +539,6 @@ public class CppCompileActionBuilder { return variables; } - public CppCompileActionBuilder addEnvironment(Map environment) { - this.environment.putAll(environment); - return this; - } - public CppCompileActionBuilder addExecutionInfo(Map executionInfo) { this.executionInfo.putAll(executionInfo); return this; diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java index 7906362a3d..a3aa127b07 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java @@ -118,7 +118,6 @@ public class FakeCppCompileAction extends CppCompileAction { ImmutableList.of(), GUID, executionInfo, - ImmutableMap.of(), CppCompileAction.CPP_COMPILE, cppSemantics, cppProvider, diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderTest.java index bd1fd017b8..3a8b07a08f 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderTest.java @@ -64,7 +64,6 @@ public class CcToolchainProviderTest { /* builtinIncludeFiles= */ ImmutableList.of(), /* coverageEnvironment= */ NestedSetBuilder.emptySet(Order.COMPILE_ORDER), /* linkDynamicLibraryTool= */ null, - /* environment= */ ImmutableMap.of(), /* builtInIncludeDirectories= */ ImmutableList.of(), /* sysroot= */ null, FdoMode.OFF); @@ -99,7 +98,6 @@ public class CcToolchainProviderTest { /* builtinIncludeFiles= */ ImmutableList.of(), /* coverageEnvironment= */ NestedSetBuilder.emptySet(Order.COMPILE_ORDER), /* linkDynamicLibraryTool= */ null, - /* environment= */ ImmutableMap.of(), /* builtInIncludeDirectories= */ ImmutableList.of(), /* sysroot= */ null, FdoMode.OFF); -- cgit v1.2.3