From b6f7c5e1ba47d6c30d45c8aac2fcd6a32de83d29 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Mon, 24 Jun 2024 08:49:10 -0700 Subject: [PATCH] Add basic C++ path mapping support Basic support for path mapping for `CppCompile` actions is added by wiring up `PathMapper` with: * structured path variables for files and include paths (`Artifact` and `NestedSet`) * inputs/outputs via `Spawn#getPathMapper()` * header discovery Also turns `external_include_paths` into a structured variable, which was missed in #22463. The following features are known to be unsupported for now: * `layering_check` (requires rewriting paths to and in module maps) * source tree artifacts (requires wiring up `PathMapper` in `CppCompileActionTemplate`) * location expanded paths to generated files such as sanitizer suppression lists (requires heuristically rewriting paths in `user_compile_flags`) These limitations will be lifted in follow-up PRs. Work towards #6526 RELNOTES: Experimental support for path mapping `CppCompile` actions can be enabled via `--modify_execution_info=CppCompile=+supports-path-mapping`. Closes #22445. PiperOrigin-RevId: 646109274 Change-Id: I6f4eb92b6be3052547f144c681b6588e9fc40693 --- .../build/lib/actions/SimpleSpawn.java | 79 ++++++- .../build/lib/rules/cpp/CcModule.java | 3 +- .../lib/rules/cpp/CcToolchainFeatures.java | 86 ++++--- .../lib/rules/cpp/CcToolchainVariables.java | 75 +++--- .../lib/rules/cpp/CompileBuildVariables.java | 4 +- .../lib/rules/cpp/CompileCommandLine.java | 28 ++- .../build/lib/rules/cpp/CppCompileAction.java | 81 +++++-- .../rules/cpp/CppCompileActionTemplate.java | 12 +- .../build/lib/rules/cpp/CppHelper.java | 3 +- .../build/lib/rules/cpp/HeaderDiscovery.java | 8 +- .../lib/rules/cpp/LinkBuildVariables.java | 6 +- .../build/lib/rules/cpp/LinkCommandLine.java | 19 +- .../lib/rules/cpp/LtoBackendArtifacts.java | 2 +- .../build/lib/exec/util/SpawnBuilder.java | 47 +--- .../lib/packages/util/MockCcSupport.java | 4 +- .../google/devtools/build/lib/rules/cpp/BUILD | 1 + .../rules/cpp/CcToolchainFeaturesTest.java | 17 +- .../rules/cpp/CompileBuildVariablesTest.java | 39 ++-- .../lib/rules/cpp/CompileCommandLineTest.java | 5 +- .../lib/rules/cpp/CppLinkActionTest.java | 5 +- .../lib/rules/cpp/HeaderDiscoveryTest.java | 23 +- .../lib/rules/cpp/LibraryToLinkValueTest.java | 59 ++--- .../lib/rules/cpp/LinkBuildVariablesTest.java | 21 +- .../lib/rules/cpp/StarlarkCcCommonTest.java | 39 +++- .../build/lib/rules/objc/ObjcLibraryTest.java | 10 +- src/test/shell/bazel/path_mapping_test.sh | 221 ++++++++++++++++++ .../config_stripped_outputs_lib.sh | 8 +- .../config_stripped_outputs_test.sh | 149 ++++++++++++ 28 files changed, 810 insertions(+), 244 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/actions/SimpleSpawn.java b/src/main/java/com/google/devtools/build/lib/actions/SimpleSpawn.java index b4f2c95b777c11..edfa4078d0dbe5 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/SimpleSpawn.java +++ b/src/main/java/com/google/devtools/build/lib/actions/SimpleSpawn.java @@ -42,6 +42,7 @@ public final class SimpleSpawn implements Spawn { private final ImmutableList outputs; // If null, all outputs are mandatory. @Nullable private final Set mandatoryOutputs; + private final PathMapper pathMapper; private final LocalResourcesSupplier localResourcesSupplier; private ResourceSet localResourcesCached; @@ -57,7 +58,8 @@ private SimpleSpawn( Collection outputs, @Nullable final Set mandatoryOutputs, @Nullable ResourceSet localResources, - @Nullable LocalResourcesSupplier localResourcesSupplier) { + @Nullable LocalResourcesSupplier localResourcesSupplier, + PathMapper pathMapper) { this.owner = Preconditions.checkNotNull(owner); this.arguments = Preconditions.checkNotNull(arguments); this.environment = Preconditions.checkNotNull(environment); @@ -81,6 +83,7 @@ private SimpleSpawn( this.localResourcesSupplier = localResourcesSupplier; this.localResourcesCached = null; } + this.pathMapper = pathMapper; } public SimpleSpawn( @@ -107,7 +110,8 @@ public SimpleSpawn( outputs, mandatoryOutputs, localResources, - /*localResourcesSupplier=*/ null); + /* localResourcesSupplier= */ null, + PathMapper.NOOP); } @SuppressWarnings("TooManyParameters") @@ -134,8 +138,67 @@ public SimpleSpawn( tools, outputs, mandatoryOutputs, - /*localResources=*/ null, - localResourcesSupplier); + /* localResources= */ null, + localResourcesSupplier, + PathMapper.NOOP); + } + + public SimpleSpawn( + ActionExecutionMetadata owner, + ImmutableList arguments, + ImmutableMap environment, + ImmutableMap executionInfo, + RunfilesSupplier runfilesSupplier, + ImmutableMap> filesetMappings, + NestedSet inputs, + NestedSet tools, + Collection outputs, + @Nullable Set mandatoryOutputs, + LocalResourcesSupplier localResourcesSupplier, + PathMapper pathMapper) { + this( + owner, + arguments, + environment, + executionInfo, + runfilesSupplier, + filesetMappings, + inputs, + tools, + outputs, + mandatoryOutputs, + /* localResources= */ null, + localResourcesSupplier, + pathMapper); + } + + public SimpleSpawn( + ActionExecutionMetadata owner, + ImmutableList arguments, + ImmutableMap environment, + ImmutableMap executionInfo, + RunfilesSupplier runfilesSupplier, + ImmutableMap> filesetMappings, + NestedSet inputs, + NestedSet tools, + Collection outputs, + @Nullable Set mandatoryOutputs, + ResourceSet localResources, + PathMapper pathMapper) { + this( + owner, + arguments, + environment, + executionInfo, + runfilesSupplier, + filesetMappings, + inputs, + tools, + outputs, + mandatoryOutputs, + localResources, + /* localResourcesSupplier= */ null, + pathMapper); } public SimpleSpawn( @@ -159,7 +222,8 @@ public SimpleSpawn( outputs, /* mandatoryOutputs= */ null, localResources, - /* localResourcesSupplier= */ null); + /* localResourcesSupplier= */ null, + PathMapper.NOOP); } public SimpleSpawn( @@ -265,6 +329,11 @@ public ResourceSet getLocalResources() throws ExecException { return localResourcesCached; } + @Override + public PathMapper getPathMapper() { + return pathMapper; + } + @Override public String getMnemonic() { return owner.getMnemonic(); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java index ab0ba8f645c754..a8361a29a04a91 100755 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java @@ -27,6 +27,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.PathMapper; import com.google.devtools.build.lib.analysis.AnalysisUtils; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue; @@ -305,7 +306,7 @@ public Dict getEnvironmentVariable( return Dict.immutableCopyOf( featureConfiguration .getFeatureConfiguration() - .getEnvironmentVariables(actionName, variables)); + .getEnvironmentVariables(actionName, variables, PathMapper.NOOP)); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java index f61334a34c33cf..ad7d993a227f0a 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java @@ -30,6 +30,7 @@ import com.google.common.collect.Iterables; import com.google.common.collect.Maps; import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander; +import com.google.devtools.build.lib.actions.PathMapper; import com.google.devtools.build.lib.concurrent.BlazeInterners; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.rules.cpp.CcToolchainVariables.Expandable; @@ -122,11 +123,12 @@ String getString() { public void expand( CcToolchainVariables variables, @Nullable ArtifactExpander expander, + PathMapper pathMapper, List commandLine) throws ExpansionException { StringBuilder flag = new StringBuilder(); for (StringChunk chunk : chunks) { - flag.append(chunk.expand(variables)); + flag.append(chunk.expand(variables, pathMapper)); } commandLine.add(flag.toString().intern()); } @@ -170,9 +172,10 @@ static class SingleChunkFlag implements Expandable { public void expand( CcToolchainVariables variables, @Nullable ArtifactExpander artifactExpander, + PathMapper pathMapper, List commandLine) throws ExpansionException { - commandLine.add(chunk.expand(variables)); + commandLine.add(chunk.expand(variables, pathMapper)); } @Override @@ -251,14 +254,16 @@ private boolean canBeExpanded(CcToolchainVariables variables) { * value of the entry is expanded with the given {@code variables}. */ public void addEnvEntry( - CcToolchainVariables variables, ImmutableMap.Builder envBuilder) + CcToolchainVariables variables, + ImmutableMap.Builder envBuilder, + PathMapper pathMapper) throws ExpansionException { if (!canBeExpanded(variables)) { return; } StringBuilder value = new StringBuilder(); for (StringChunk chunk : valueChunks) { - value.append(chunk.expand(variables)); + value.append(chunk.expand(variables, pathMapper)); } envBuilder.put(key, value.toString()); } @@ -372,29 +377,30 @@ private FlagGroup(CToolchain.FlagGroup flagGroup) throws EvalException { public void expand( CcToolchainVariables variables, @Nullable ArtifactExpander expander, + PathMapper pathMapper, final List commandLine) throws ExpansionException { - if (!canBeExpanded(variables, expander)) { + if (!canBeExpanded(variables, expander, pathMapper)) { return; } if (iterateOverVariable != null) { for (CcToolchainVariables.VariableValue variableValue : - variables.getSequenceVariable(iterateOverVariable, expander)) { + variables.getSequenceVariable(iterateOverVariable, expander, pathMapper)) { CcToolchainVariables nestedVariables = new SingleVariables(variables, iterateOverVariable, variableValue); for (Expandable expandable : expandables) { - expandable.expand(nestedVariables, expander, commandLine); + expandable.expand(nestedVariables, expander, pathMapper, commandLine); } } } else { for (Expandable expandable : expandables) { - expandable.expand(variables, expander, commandLine); + expandable.expand(variables, expander, pathMapper, commandLine); } } } private boolean canBeExpanded( - CcToolchainVariables variables, @Nullable ArtifactExpander expander) + CcToolchainVariables variables, @Nullable ArtifactExpander expander, PathMapper pathMapper) throws ExpansionException { for (String variable : expandIfAllAvailable) { if (!variables.isAvailable(variable, expander)) { @@ -420,7 +426,7 @@ private boolean canBeExpanded( && (!variables.isAvailable(expandIfEqual.variable, expander) || !variables .getVariable(expandIfEqual.variable) - .getStringValue(expandIfEqual.variable) + .getStringValue(expandIfEqual.variable, pathMapper) .equals(expandIfEqual.value))) { return false; } @@ -445,9 +451,10 @@ private boolean canBeExpanded( private void expandCommandLine( CcToolchainVariables variables, @Nullable ArtifactExpander expander, + PathMapper pathMapper, final List commandLine) throws ExpansionException { - expand(variables, expander, commandLine); + expand(variables, expander, pathMapper, commandLine); } @Override @@ -568,6 +575,7 @@ private void expandCommandLine( CcToolchainVariables variables, Set enabledFeatureNames, @Nullable ArtifactExpander expander, + PathMapper pathMapper, List commandLine) throws ExpansionException { for (String variable : expandIfAllAvailable) { @@ -582,7 +590,7 @@ private void expandCommandLine( return; } for (FlagGroup flagGroup : flagGroups) { - flagGroup.expandCommandLine(variables, expander, commandLine); + flagGroup.expandCommandLine(variables, expander, pathMapper, commandLine); } } @@ -716,6 +724,7 @@ ImmutableSet getWithFeatureSets() { private void expandEnvironment( String action, CcToolchainVariables variables, + PathMapper pathMapper, Set enabledFeatureNames, ImmutableMap.Builder envBuilder) throws ExpansionException { @@ -726,7 +735,7 @@ private void expandEnvironment( return; } for (EnvEntry envEntry : envEntries) { - envEntry.addEnvEntry(variables, envBuilder); + envEntry.addEnvEntry(variables, envBuilder, pathMapper); } } @@ -844,11 +853,12 @@ public String getName() { private void expandEnvironment( String action, CcToolchainVariables variables, + PathMapper pathMapper, Set enabledFeatureNames, ImmutableMap.Builder envBuilder) throws ExpansionException { for (EnvSet envSet : envSets) { - envSet.expandEnvironment(action, variables, enabledFeatureNames, envBuilder); + envSet.expandEnvironment(action, variables, pathMapper, enabledFeatureNames, envBuilder); } } @@ -858,10 +868,12 @@ private void expandCommandLine( CcToolchainVariables variables, Set enabledFeatureNames, @Nullable ArtifactExpander expander, + PathMapper pathMapper, List commandLine) throws ExpansionException { for (FlagSet flagSet : flagSets) { - flagSet.expandCommandLine(action, variables, enabledFeatureNames, expander, commandLine); + flagSet.expandCommandLine( + action, variables, enabledFeatureNames, expander, pathMapper, commandLine); } } @@ -1160,11 +1172,12 @@ private void expandCommandLine( CcToolchainVariables variables, Set enabledFeatureNames, @Nullable ArtifactExpander expander, + PathMapper pathMapper, List commandLine) throws ExpansionException { for (FlagSet flagSet : flagSets) { flagSet.expandCommandLine( - actionName, variables, enabledFeatureNames, expander, commandLine); + actionName, variables, enabledFeatureNames, expander, pathMapper, commandLine); } } @@ -1375,59 +1388,74 @@ boolean actionIsConfigured(String actionName) { /** @return the command line for the given {@code action}. */ public List getCommandLine(String action, CcToolchainVariables variables) throws ExpansionException { - return getCommandLine(action, variables, /* expander= */ null); + return getCommandLine(action, variables, /* expander= */ null, PathMapper.NOOP); } public List getCommandLine( - String action, CcToolchainVariables variables, @Nullable ArtifactExpander expander) + String action, + CcToolchainVariables variables, + @Nullable ArtifactExpander expander, + PathMapper pathMapper) throws ExpansionException { List commandLine = new ArrayList<>(); if (actionIsConfigured(action)) { actionConfigByActionName .get(action) - .expandCommandLine(variables, enabledFeatureNames, expander, commandLine); + .expandCommandLine(variables, enabledFeatureNames, expander, pathMapper, commandLine); } for (Feature feature : enabledFeatures) { - feature.expandCommandLine(action, variables, enabledFeatureNames, expander, commandLine); + feature.expandCommandLine( + action, variables, enabledFeatureNames, expander, pathMapper, commandLine); } return commandLine; } - /** @return the flags expanded for the given {@code action} in per-feature buckets. */ + /** + * @return the flags expanded for the given {@code action} in per-feature buckets. + */ public ImmutableList>> getPerFeatureExpansions( - String action, CcToolchainVariables variables) throws ExpansionException { - return getPerFeatureExpansions(action, variables, null); + String action, CcToolchainVariables variables, PathMapper pathMapper) + throws ExpansionException { + return getPerFeatureExpansions(action, variables, null, pathMapper); } public ImmutableList>> getPerFeatureExpansions( - String action, CcToolchainVariables variables, @Nullable ArtifactExpander expander) + String action, + CcToolchainVariables variables, + @Nullable ArtifactExpander expander, + PathMapper pathMapper) throws ExpansionException { ImmutableList.Builder>> perFeatureExpansions = ImmutableList.builder(); if (actionIsConfigured(action)) { List commandLine = new ArrayList<>(); ActionConfig actionConfig = actionConfigByActionName.get(action); - actionConfig.expandCommandLine(variables, enabledFeatureNames, expander, commandLine); + actionConfig.expandCommandLine( + variables, enabledFeatureNames, expander, pathMapper, commandLine); perFeatureExpansions.add(Pair.of(actionConfig.getName(), commandLine)); } for (Feature feature : enabledFeatures) { List commandLine = new ArrayList<>(); - feature.expandCommandLine(action, variables, enabledFeatureNames, expander, commandLine); + feature.expandCommandLine( + action, variables, enabledFeatureNames, expander, pathMapper, commandLine); perFeatureExpansions.add(Pair.of(feature.getName(), commandLine)); } return perFeatureExpansions.build(); } - /** @return the environment variables (key/value pairs) for the given {@code action}. */ + /** + * @return the environment variables (key/value pairs) for the given {@code action}. + */ public ImmutableMap getEnvironmentVariables( - String action, CcToolchainVariables variables) throws ExpansionException { + String action, CcToolchainVariables variables, PathMapper pathMapper) + throws ExpansionException { ImmutableMap.Builder envBuilder = ImmutableMap.builder(); for (Feature feature : enabledFeatures) { - feature.expandEnvironment(action, variables, enabledFeatureNames, envBuilder); + feature.expandEnvironment(action, variables, pathMapper, enabledFeatureNames, envBuilder); } return envBuilder.buildOrThrow(); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainVariables.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainVariables.java index 561ff493ee8f04..fd819a5a33e565 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainVariables.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainVariables.java @@ -25,6 +25,7 @@ import com.google.common.collect.Sets.SetView; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander; +import com.google.devtools.build.lib.actions.PathMapper; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.concurrent.BlazeInterners; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; @@ -64,7 +65,7 @@ interface StringChunk { * * @param variables binding of variable names to their values for a single flag expansion. */ - String expand(CcToolchainVariables variables) throws ExpansionException; + String expand(CcToolchainVariables variables, PathMapper pathMapper) throws ExpansionException; String getString(); } @@ -79,7 +80,7 @@ private static final class StringLiteralChunk implements StringChunk { } @Override - public String expand(CcToolchainVariables variables) { + public String expand(CcToolchainVariables variables, PathMapper pathMapper) { return text; } @@ -116,12 +117,13 @@ private static final class VariableChunk implements StringChunk { } @Override - public String expand(CcToolchainVariables variables) throws ExpansionException { + public String expand(CcToolchainVariables variables, PathMapper pathMapper) + throws ExpansionException { // We check all variables in FlagGroup.expandCommandLine. // If we arrive here with the variable not being available, the variable was provided, but // the nesting level of the NestedSequence was deeper than the nesting level of the flag // groups. - return variables.getStringVariable(variableName); + return variables.getStringVariable(variableName, pathMapper); } @Override @@ -273,6 +275,7 @@ public interface Expandable { void expand( CcToolchainVariables variables, @Nullable ArtifactExpander expander, + PathMapper pathMapper, List commandLine) throws ExpansionException; } @@ -292,10 +295,11 @@ void expand( *

Throws {@link ExpansionException} when the variable is not a {@link StringSequence}. */ public static ImmutableList toStringList( - CcToolchainVariables variables, String variableName) throws ExpansionException { + CcToolchainVariables variables, String variableName, PathMapper pathMapper) + throws ExpansionException { ImmutableList.Builder result = ImmutableList.builder(); - for (VariableValue value : variables.getSequenceVariable(variableName)) { - result.add(value.getStringValue(variableName)); + for (VariableValue value : variables.getSequenceVariable(variableName, pathMapper)) { + result.add(value.getStringValue(variableName, pathMapper)); } return result.build(); } @@ -413,18 +417,21 @@ private VariableValue getStructureVariable( return variable; } - public String getStringVariable(String variableName) throws ExpansionException { - return getVariable(variableName, /* expander= */ null).getStringValue(variableName); + public String getStringVariable(String variableName, PathMapper pathMapper) + throws ExpansionException { + return getVariable(variableName, /* expander= */ null).getStringValue(variableName, pathMapper); } - public Iterable getSequenceVariable(String variableName) - throws ExpansionException { - return getVariable(variableName, /* expander= */ null).getSequenceValue(variableName); + public Iterable getSequenceVariable( + String variableName, PathMapper pathMapper) throws ExpansionException { + return getVariable(variableName, /* expander= */ null) + .getSequenceValue(variableName, pathMapper); } public Iterable getSequenceVariable( - String variableName, @Nullable ArtifactExpander expander) throws ExpansionException { - return getVariable(variableName, expander).getSequenceValue(variableName); + String variableName, @Nullable ArtifactExpander expander, PathMapper pathMapper) + throws ExpansionException { + return getVariable(variableName, expander).getSequenceValue(variableName, pathMapper); } /** Returns whether {@code variable} is set. */ @@ -464,16 +471,18 @@ interface VariableValue { * StringValue), or throw exception if it cannot (e.g. Sequence). * * @param variableName name of the variable value at hand, for better exception message. + * @param pathMapper */ - String getStringValue(String variableName) throws ExpansionException; + String getStringValue(String variableName, PathMapper pathMapper) throws ExpansionException; /** * Returns Iterable value of the variable, if the variable type can be converted to a Iterable * (e.g. Sequence), or throw exception if it cannot (e.g. StringValue). * * @param variableName name of the variable value at hand, for better exception message. + * @param pathMapper */ - Iterable getSequenceValue(String variableName) + Iterable getSequenceValue(String variableName, PathMapper pathMapper) throws ExpansionException; /** @@ -498,8 +507,9 @@ VariableValue getFieldValue( /** * Adapter for {@link VariableValue} predefining error handling methods. Override {@link * #getVariableTypeName()}, {@link #isTruthy()}, and one of {@link #getFieldValue(String, - * String)}, {@link #getSequenceValue(String)}, or {@link #getStringValue(String)}, and you'll get - * error handling for the other methods for free. + * String)}, {@link VariableValue#getSequenceValue(String, PathMapper)}, or {@link + * VariableValue#getStringValue(String, PathMapper)}, and you'll get error handling for the other + * methods for free. */ abstract static class VariableValueAdapter implements VariableValue { @@ -536,7 +546,8 @@ public VariableValue getFieldValue( } @Override - public String getStringValue(String variableName) throws ExpansionException { + public String getStringValue(String variableName, PathMapper pathMapper) + throws ExpansionException { throw new ExpansionException( String.format( "Invalid toolchain configuration: Cannot expand variable '%s': expected string, " @@ -545,8 +556,8 @@ public String getStringValue(String variableName) throws ExpansionException { } @Override - public Iterable getSequenceValue(String variableName) - throws ExpansionException { + public Iterable getSequenceValue( + String variableName, PathMapper pathMapper) throws ExpansionException { throw new ExpansionException( String.format( "Invalid toolchain configuration: Cannot expand variable '%s': expected sequence, " @@ -946,7 +957,8 @@ private Sequence(ImmutableList values) { } @Override - public ImmutableList getSequenceValue(String variableName) { + public ImmutableList getSequenceValue( + String variableName, PathMapper pathMapper) { return values; } @@ -1000,7 +1012,8 @@ private StringSequence(Iterable values) { } @Override - public ImmutableList getSequenceValue(String variableName) { + public ImmutableList getSequenceValue( + String variableName, PathMapper pathMapper) { ImmutableList.Builder sequences = ImmutableList.builderWithExpectedSize(values.size()); for (String value : values) { @@ -1060,7 +1073,8 @@ private StringSetSequence(NestedSet values) { } @Override - public ImmutableList getSequenceValue(String variableName) { + public ImmutableList getSequenceValue( + String variableName, PathMapper pathMapper) { ImmutableList valuesList = values.toList(); ImmutableList.Builder sequences = ImmutableList.builderWithExpectedSize(valuesList.size()); @@ -1107,12 +1121,13 @@ private PathFragmentSetSequence(NestedSet values) { } @Override - public ImmutableList getSequenceValue(String variableName) { + public ImmutableList getSequenceValue( + String variableName, PathMapper pathMapper) { ImmutableList valuesList = values.toList(); ImmutableList.Builder sequences = ImmutableList.builderWithExpectedSize(valuesList.size()); for (PathFragment value : valuesList) { - sequences.add(new StringValue(value.getSafePathString())); + sequences.add(new StringValue(pathMapper.map(value).getSafePathString())); } return sequences.build(); } @@ -1210,7 +1225,7 @@ private static final class StringValue extends VariableValueAdapter { } @Override - public String getStringValue(String variableName) { + public String getStringValue(String variableName, PathMapper pathMapper) { return value; } @@ -1257,7 +1272,7 @@ private static BooleanValue of(boolean value) { } @Override - public String getStringValue(String variableName) { + public String getStringValue(String variableName, PathMapper pathMapper) { return value ? "1" : "0"; } @@ -1288,8 +1303,8 @@ private static final class ArtifactValue extends VariableValueAdapter { } @Override - public String getStringValue(String variableName) { - return value.getExecPathString(); + public String getStringValue(String variableName, PathMapper pathMapper) { + return pathMapper.getMappedExecPathString(value); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileBuildVariables.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileBuildVariables.java index 9babf176344cfb..97b9f90fa71cb4 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileBuildVariables.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileBuildVariables.java @@ -459,9 +459,9 @@ private static void setupSpecificVariables( } if (!externalIncludeDirs.isEmpty()) { - buildVariables.addStringSequenceVariable( + buildVariables.addPathFragmentSequenceVariable( EXTERNAL_INCLUDE_PATHS.getVariableName(), - Iterables.transform(externalIncludeDirs, PathFragment::getSafePathString)); + NestedSetBuilder.wrap(Order.STABLE_ORDER, externalIncludeDirs)); } buildVariables.addAllStringVariables(additionalBuildVariables); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileCommandLine.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileCommandLine.java index 7fc061f70ee55d..ddc3fdbb20c5ca 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileCommandLine.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileCommandLine.java @@ -20,6 +20,7 @@ import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.CommandLine; import com.google.devtools.build.lib.actions.CommandLineExpansionException; +import com.google.devtools.build.lib.actions.PathMapper; import com.google.devtools.build.lib.rules.cpp.CcCommon.CoptsFilter; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.ExpansionException; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration; @@ -61,9 +62,10 @@ private boolean isGenerateDotdFile(Artifact sourceArtifact) { } /** Returns the environment variables that should be set for C++ compile actions. */ - ImmutableMap getEnvironment() throws CommandLineExpansionException { + ImmutableMap getEnvironment(PathMapper pathMapper) + throws CommandLineExpansionException { try { - return featureConfiguration.getEnvironmentVariables(actionName, variables); + return featureConfiguration.getEnvironmentVariables(actionName, variables, pathMapper); } catch (ExpansionException e) { throw new CommandLineExpansionException(e.getMessage()); } @@ -84,7 +86,9 @@ public String getToolPath() { * unmodified original variables are used. */ List getArguments( - @Nullable PathFragment parameterFilePath, @Nullable CcToolchainVariables overwrittenVariables) + @Nullable PathFragment parameterFilePath, + @Nullable CcToolchainVariables overwrittenVariables, + PathMapper pathMapper) throws CommandLineExpansionException { List commandLine = new ArrayList<>(); @@ -95,7 +99,7 @@ List getArguments( if (parameterFilePath != null) { commandLine.add("@" + parameterFilePath.getSafePathString()); } else { - commandLine.addAll(getCompilerOptions(overwrittenVariables)); + commandLine.addAll(getCompilerOptions(overwrittenVariables, pathMapper)); } return commandLine; } @@ -112,13 +116,14 @@ public CommandLine getFilteredFeatureConfigurationCommandLine(CppCompileAction c @Override public Iterable arguments() throws CommandLineExpansionException { CcToolchainVariables overwrittenVariables = cppCompileAction.getOverwrittenVariables(); - List compilerOptions = getCompilerOptions(overwrittenVariables); + List compilerOptions = getCompilerOptions(overwrittenVariables, PathMapper.NOOP); return ImmutableList.builder().add(getToolPath()).addAll(compilerOptions).build(); } }; } - public List getCompilerOptions(@Nullable CcToolchainVariables overwrittenVariables) + public List getCompilerOptions( + @Nullable CcToolchainVariables overwrittenVariables, PathMapper pathMapper) throws CommandLineExpansionException { try { List options = new ArrayList<>(); @@ -130,7 +135,8 @@ public List getCompilerOptions(@Nullable CcToolchainVariables overwritte updatedVariables = variablesBuilder.build(); } addFilteredOptions( - options, featureConfiguration.getPerFeatureExpansions(actionName, updatedVariables)); + options, + featureConfiguration.getPerFeatureExpansions(actionName, updatedVariables, pathMapper)); return options; } catch (ExpansionException e) { @@ -171,15 +177,15 @@ public CcToolchainVariables getVariables() { /** * Returns all user provided copts flags. * - * TODO(b/64108724): Get rid of this method when we don't need to parse copts to collect include - * directories anymore (meaning there is a way of specifying include directories using an + *

TODO(b/64108724): Get rid of this method when we don't need to parse copts to collect + * include directories anymore (meaning there is a way of specifying include directories using an * explicit attribute, not using platform-dependent garbage bag that copts is). */ - public ImmutableList getCopts() { + public ImmutableList getCopts(PathMapper pathMapper) { if (variables.isAvailable(CompileBuildVariables.USER_COMPILE_FLAGS.getVariableName())) { try { return CcToolchainVariables.toStringList( - variables, CompileBuildVariables.USER_COMPILE_FLAGS.getVariableName()); + variables, CompileBuildVariables.USER_COMPILE_FLAGS.getVariableName(), pathMapper); } catch (ExpansionException e) { throw new IllegalStateException( "Should not happen - 'user_compile_flags' should be a string list, but wasn't.", e); 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 5dfb93b07d89a9..146ec1395b4fd6 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 @@ -50,6 +50,7 @@ import com.google.devtools.build.lib.actions.ExecutionRequirements; import com.google.devtools.build.lib.actions.ParamFileInfo; import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType; +import com.google.devtools.build.lib.actions.PathMapper; import com.google.devtools.build.lib.actions.ResourceSet; import com.google.devtools.build.lib.actions.SimpleSpawn; import com.google.devtools.build.lib.actions.Spawn; @@ -57,7 +58,10 @@ import com.google.devtools.build.lib.actions.extra.CppCompileInfo; import com.google.devtools.build.lib.actions.extra.EnvironmentVariable; import com.google.devtools.build.lib.actions.extra.ExtraActionInfo; +import com.google.devtools.build.lib.analysis.actions.PathMappers; import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue; +import com.google.devtools.build.lib.analysis.config.CoreOptions; +import com.google.devtools.build.lib.analysis.config.CoreOptions.OutputPathsMode; import com.google.devtools.build.lib.analysis.starlark.Args; import com.google.devtools.build.lib.bugreport.BugReport; import com.google.devtools.build.lib.cmdline.LabelConstants; @@ -672,7 +676,7 @@ public CcCompilationContext getCcCompilationContext() { public List getQuoteIncludeDirs() { ImmutableList.Builder result = ImmutableList.builder(); result.addAll(ccCompilationContext.getQuoteIncludeDirs()); - ImmutableList copts = compileCommandLine.getCopts(); + ImmutableList copts = compileCommandLine.getCopts(PathMapper.NOOP); for (int i = 0; i < copts.size(); i++) { String opt = copts.get(i); if (opt.startsWith("-iquote")) { @@ -693,7 +697,7 @@ public List getQuoteIncludeDirs() { public List getIncludeDirs() { ImmutableList.Builder result = ImmutableList.builder(); result.addAll(ccCompilationContext.getIncludeDirs()); - for (String opt : compileCommandLine.getCopts()) { + for (String opt : compileCommandLine.getCopts(PathMapper.NOOP)) { if (opt.startsWith("-I") || opt.startsWith("/I")) { // We insist on the combined form "-Idir". String includeDir = opt.substring(2); @@ -857,9 +861,14 @@ public ImmutableMap getIncompleteEnvironmentForTesting() } } - @Override() + @Override public ImmutableMap getEffectiveEnvironment(Map clientEnv) throws CommandLineExpansionException { + return getEffectiveEnvironment(clientEnv, PathMapper.NOOP); + } + + public ImmutableMap getEffectiveEnvironment( + Map clientEnv, PathMapper pathMapper) throws CommandLineExpansionException { ActionEnvironment env = getEnvironment(); Map environment = Maps.newLinkedHashMapWithExpectedSize(env.estimatedSize()); env.resolve(environment, clientEnv); @@ -870,13 +879,17 @@ public ImmutableMap getEffectiveEnvironment(Map environment.put("PWD", "/proc/self/cwd"); } - environment.putAll(compileCommandLine.getEnvironment()); + environment.putAll(compileCommandLine.getEnvironment(pathMapper)); return ImmutableMap.copyOf(environment); } @Override public List getArguments() throws CommandLineExpansionException { - return compileCommandLine.getArguments(paramFilePath, getOverwrittenVariables()); + return getArguments(PathMapper.NOOP); + } + + private List getArguments(PathMapper pathMapper) throws CommandLineExpansionException { + return compileCommandLine.getArguments(paramFilePath, getOverwrittenVariables(), pathMapper); } @Override @@ -884,7 +897,7 @@ public Sequence getStarlarkArgv() throws EvalException { try { return StarlarkList.immutableCopyOf( compileCommandLine.getArguments( - /* parameterFilePath= */ null, getOverwrittenVariables())); + /* parameterFilePath= */ null, getOverwrittenVariables(), PathMapper.NOOP)); } catch (CommandLineExpansionException ex) { throw new EvalException(ex); @@ -919,7 +932,8 @@ public ExtraActionInfo.Builder getExtraActionInfo(ActionKeyContext actionKeyCont CppCompileInfo.Builder info = CppCompileInfo.newBuilder(); info.setTool(compileCommandLine.getToolPath()); - List options = compileCommandLine.getCompilerOptions(getOverwrittenVariables()); + List options = + compileCommandLine.getCompilerOptions(getOverwrittenVariables(), PathMapper.NOOP); for (String option : options) { info.addCompilerOption(option); @@ -954,7 +968,7 @@ public ExtraActionInfo.Builder getExtraActionInfo(ActionKeyContext actionKeyCont /** Returns the compiler options. */ @VisibleForTesting public List getCompilerOptions() throws CommandLineExpansionException { - return compileCommandLine.getCompilerOptions(/*overwrittenVariables=*/ null); + return compileCommandLine.getCompilerOptions(/* overwrittenVariables= */ null, PathMapper.NOOP); } @Override @@ -1270,7 +1284,7 @@ public void computeKey( actionKeyContext, fp, getEnvironment(), - compileCommandLine.getEnvironment(), + compileCommandLine.getEnvironment(PathMapper.NOOP), executionInfo, getCommandLineKey(), ccCompilationContext.getDeclaredIncludeSrcs(), @@ -1279,7 +1293,9 @@ public void computeKey( additionalPrunableHeaders, builtInIncludeDirectories, ccCompilationContext.getTransitiveCompilationPrerequisites(), - cppConfiguration().validateTopLevelHeaderInclusions()); + cppConfiguration().validateTopLevelHeaderInclusions(), + getMnemonic(), + PathMappers.getOutputPathsMode(configuration)); } // Separated into a helper method so that it can be called from CppCompileActionTemplate. @@ -1296,7 +1312,9 @@ static void computeKey( NestedSet prunableHeaders, List builtInIncludeDirectories, NestedSet inputsForInvalidation, - boolean validateTopLevelHeaderInclusions) + boolean validateTopLevelHeaderInclusions, + String mnemonic, + OutputPathsMode outputPathsMode) throws CommandLineExpansionException, InterruptedException { fp.addUUID(GUID); env.addTo(fp); @@ -1323,6 +1341,8 @@ static void computeKey( // This is needed for CppLinkstampCompile. fp.addInt(0); actionKeyContext.addNestedSetToFingerprint(fp, inputsForInvalidation); + + PathMappers.addToFingerprint(mnemonic, executionInfo, outputPathsMode, fp); } private byte[] getCommandLineKey() throws CommandLineExpansionException { @@ -1348,12 +1368,14 @@ static byte[] computeCommandLineKey(List compilerOptions) { @Override public ActionResult execute(ActionExecutionContext actionExecutionContext) throws ActionExecutionException, InterruptedException { + PathMapper pathMapper = PathMappers.create(this, PathMappers.getOutputPathsMode(configuration)); + if (featureConfiguration.isEnabled(CppRuleClasses.COMPILER_PARAM_FILE)) { try { paramFileActionInput = new ParamFileActionInput( paramFilePath, - compileCommandLine.getCompilerOptions(getOverwrittenVariables()), + compileCommandLine.getCompilerOptions(getOverwrittenVariables(), pathMapper), // TODO(b/132888308): Support MSVC, which has its own method of escaping strings. ParameterFileType.GCC_QUOTED, StandardCharsets.ISO_8859_1); @@ -1389,7 +1411,10 @@ public ActionResult execute(ActionExecutionContext actionExecutionContext) Spawn spawn; try { spawn = - createSpawn(actionExecutionContext.getExecRoot(), actionExecutionContext.getClientEnv()); + createSpawn( + actionExecutionContext.getExecRoot(), + actionExecutionContext.getClientEnv(), + pathMapper); } finally { clearAdditionalInputs(); } @@ -1438,7 +1463,8 @@ public ActionResult execute(ActionExecutionContext actionExecutionContext) scanningContext.getArtifactResolver(), showIncludesFilterForStdout, showIncludesFilterForStderr, - siblingRepositoryLayout); + siblingRepositoryLayout, + pathMapper); updateActionInputs(discoveredInputs); validateInclusions(actionExecutionContext, discoveredInputs); return ActionResult.create(spawnResults); @@ -1456,7 +1482,8 @@ public ActionResult execute(ActionExecutionContext actionExecutionContext) execRoot, scanningContext.getArtifactResolver(), dotDContents, - siblingRepositoryLayout); + siblingRepositoryLayout, + pathMapper); dotDContents = null; // Garbage collect in-memory .d contents. updateActionInputs(discoveredInputs); @@ -1522,7 +1549,8 @@ private boolean shouldParseShowIncludes() { return featureConfiguration.isEnabled(CppRuleClasses.PARSE_SHOWINCLUDES); } - Spawn createSpawn(Path execRoot, Map clientEnv) throws ActionExecutionException { + Spawn createSpawn(Path execRoot, Map clientEnv, PathMapper pathMapper) + throws ActionExecutionException { // Intentionally not adding {@link CppCompileAction#inputsForInvalidation}, those are not needed // for execution. NestedSetBuilder inputsBuilder = @@ -1581,8 +1609,8 @@ Spawn createSpawn(Path execRoot, Map clientEnv) throws ActionExe try { return new SimpleSpawn( this, - ImmutableList.copyOf(getArguments()), - getEffectiveEnvironment(clientEnv), + ImmutableList.copyOf(getArguments(pathMapper)), + getEffectiveEnvironment(clientEnv, pathMapper), executionInfo.buildOrThrow(), /* runfilesSupplier= */ null, /* filesetMappings= */ ImmutableMap.of(), @@ -1595,7 +1623,8 @@ Spawn createSpawn(Path execRoot, Map clientEnv) throws ActionExe enabledCppCompileResourcesEstimation(), getMnemonic(), OS.getCurrent(), - inputs.memoizedFlattenAndGetSize())); + inputs.memoizedFlattenAndGetSize()), + pathMapper); } catch (CommandLineExpansionException e) { String message = String.format( @@ -1611,7 +1640,8 @@ private NestedSet discoverInputsFromShowIncludesFilters( ArtifactResolver artifactResolver, ShowIncludesFilter showIncludesFilterForStdout, ShowIncludesFilter showIncludesFilterForStderr, - boolean siblingRepositoryLayout) + boolean siblingRepositoryLayout, + PathMapper pathMapper) throws ActionExecutionException { Collection stdoutDeps = showIncludesFilterForStdout.getDependencies(execRoot); Collection stderrDeps = showIncludesFilterForStderr.getDependencies(execRoot); @@ -1643,7 +1673,8 @@ private NestedSet discoverInputsFromShowIncludesFilters( getAllowedDerivedInputs(), execRoot, artifactResolver, - siblingRepositoryLayout); + siblingRepositoryLayout, + pathMapper); } @VisibleForTesting @@ -1652,7 +1683,8 @@ public NestedSet discoverInputsFromDotdFiles( Path execRoot, ArtifactResolver artifactResolver, byte[] dotDContents, - boolean siblingRepositoryLayout) + boolean siblingRepositoryLayout, + PathMapper pathMapper) throws ActionExecutionException { Preconditions.checkNotNull(getDotdFile(), "Trying to scan .d file which is unset"); return HeaderDiscovery.discoverInputsFromDependencies( @@ -1664,7 +1696,8 @@ public NestedSet discoverInputsFromDotdFiles( getAllowedDerivedInputs(), execRoot, artifactResolver, - siblingRepositoryLayout); + siblingRepositoryLayout, + pathMapper); } private DependencySet processDepset( @@ -1812,7 +1845,7 @@ public String describeKey() { // The first element in getArguments() is actually the command to execute. String legend = " Command: "; try { - for (String argument : ShellEscaper.escapeAll(getArguments())) { + for (String argument : ShellEscaper.escapeAll(getArguments(PathMapper.NOOP))) { message.append(legend); message.append(argument); message.append('\n'); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionTemplate.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionTemplate.java index 9b8ac2725f135f..1b79739d28f2df 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionTemplate.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionTemplate.java @@ -31,6 +31,8 @@ import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact; import com.google.devtools.build.lib.actions.CommandLineExpansionException; import com.google.devtools.build.lib.actions.MiddlemanType; +import com.google.devtools.build.lib.actions.PathMapper; +import com.google.devtools.build.lib.analysis.config.CoreOptions.OutputPathsMode; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; @@ -195,10 +197,10 @@ protected void computeKey( actionKeyContext, fp, cppCompileActionBuilder.getActionEnvironment(), - commandLine.getEnvironment(), + commandLine.getEnvironment(PathMapper.NOOP), cppCompileActionBuilder.getExecutionInfo(), CppCompileAction.computeCommandLineKey( - commandLine.getCompilerOptions(/* overwrittenVariables= */ null)), + commandLine.getCompilerOptions(/* overwrittenVariables= */ null, PathMapper.NOOP)), cppCompileActionBuilder.getCcCompilationContext().getDeclaredIncludeSrcs(), mandatoryInputs, mandatoryInputs, @@ -207,7 +209,11 @@ protected void computeKey( cppCompileActionBuilder.getInputsForInvalidation(), toolchain .getCppConfigurationEvenThoughItCanBeDifferentThanWhatTargetHas() - .validateTopLevelHeaderInclusions()); + .validateTopLevelHeaderInclusions(), + getMnemonic(), + // TODO(fmeum): Replace this with the actual value once CppCompileActionTemplate supports + // path mapping. + OutputPathsMode.OFF); } private boolean shouldCompileHeaders() { diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java index af09bc16599e3c..69bb5ede9271a9 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java @@ -26,6 +26,7 @@ import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact; import com.google.devtools.build.lib.actions.ArtifactRoot; import com.google.devtools.build.lib.actions.FailAction; +import com.google.devtools.build.lib.actions.PathMapper; import com.google.devtools.build.lib.analysis.AliasProvider; import com.google.devtools.build.lib.analysis.AnalysisUtils; import com.google.devtools.build.lib.analysis.Expander; @@ -490,7 +491,7 @@ public static ImmutableMap getEnvironmentVariables( String actionName) throws RuleErrorException { try { - return featureConfiguration.getEnvironmentVariables(actionName, variables); + return featureConfiguration.getEnvironmentVariables(actionName, variables, PathMapper.NOOP); } catch (ExpansionException e) { throw ruleErrorConsumer.throwWithRuleError(e); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscovery.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscovery.java index dba575441b259d..c00c4f9dad64f6 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscovery.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscovery.java @@ -19,6 +19,7 @@ import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact; import com.google.devtools.build.lib.actions.ArtifactResolver; +import com.google.devtools.build.lib.actions.PathMapper; import com.google.devtools.build.lib.cmdline.LabelConstants; import com.google.devtools.build.lib.cmdline.PackageIdentifier; import com.google.devtools.build.lib.cmdline.RepositoryName; @@ -71,7 +72,8 @@ static NestedSet discoverInputsFromDependencies( NestedSet allowedDerivedInputs, Path execRoot, ArtifactResolver artifactResolver, - boolean siblingRepositoryLayout) + boolean siblingRepositoryLayout, + PathMapper pathMapper) throws ActionExecutionException { Map regularDerivedArtifacts = new HashMap<>(); Map treeArtifacts = new HashMap<>(); @@ -89,9 +91,9 @@ static NestedSet discoverInputsFromDependencies( // whose path changed, that is not taken into account by the action cache, and it will get an // action cache hit using the remaining un-renamed artifact. if (a.isTreeArtifact()) { - treeArtifacts.putIfAbsent(a.getExecPath(), (SpecialArtifact) a); + treeArtifacts.putIfAbsent(pathMapper.map(a.getExecPath()), (SpecialArtifact) a); } else { - regularDerivedArtifacts.putIfAbsent(a.getExecPath(), a); + regularDerivedArtifacts.putIfAbsent(pathMapper.map(a.getExecPath()), a); } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariables.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariables.java index 2f2c1a17ebe3ac..925f5bb3f00779 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariables.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariables.java @@ -17,6 +17,7 @@ import com.google.common.base.Predicates; import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; +import com.google.devtools.build.lib.actions.PathMapper; import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration; @@ -282,7 +283,10 @@ public static CcToolchainVariables setupVariables( opts.addAll(userLinkFlags); opts.addAll( featureConfiguration.getCommandLine( - CppActionNames.LTO_INDEXING, buildVariables.build(), /* expander= */ null)); + CppActionNames.LTO_INDEXING, + buildVariables.build(), + /* expander= */ null, + PathMapper.NOOP)); opts.addAll(cppConfiguration.getLtoIndexOptions()); userLinkFlagsWithLtoIndexingIfNeeded = opts.build(); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java index db750f1a7aa0e3..55f88569158c6e 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java @@ -34,7 +34,6 @@ import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration; import com.google.devtools.build.lib.rules.cpp.Link.LinkTargetType; import com.google.devtools.build.lib.rules.cpp.Link.LinkerOrArchiver; -import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.errorprone.annotations.CanIgnoreReturnValue; import java.util.ArrayList; @@ -197,13 +196,16 @@ public List getCommandLine(@Nullable ArtifactExpander expander) String linkerParamFile = variables .getVariable(LINKER_PARAM_FILE.getVariableName()) - .getStringValue(LINKER_PARAM_FILE.getVariableName()); + .getStringValue(LINKER_PARAM_FILE.getVariableName(), PathMapper.NOOP); argv.addAll( - featureConfiguration.getCommandLine(actionName, variables, expander).stream() + featureConfiguration + .getCommandLine(actionName, variables, expander, PathMapper.NOOP) + .stream() .filter(s -> s.contains(linkerParamFile)) .collect(toImmutableList())); } else { - argv.addAll(featureConfiguration.getCommandLine(actionName, variables, expander)); + argv.addAll( + featureConfiguration.getCommandLine(actionName, variables, expander, PathMapper.NOOP)); } } catch (ExpansionException e) { throw new CommandLineExpansionException(e.getMessage()); @@ -220,13 +222,16 @@ public List getParamCommandLine(@Nullable ArtifactExpander expander) String linkerParamFile = variables .getVariable(LINKER_PARAM_FILE.getVariableName()) - .getStringValue(LINKER_PARAM_FILE.getVariableName()); + .getStringValue(LINKER_PARAM_FILE.getVariableName(), PathMapper.NOOP); argv.addAll( - featureConfiguration.getCommandLine(actionName, variables, expander).stream() + featureConfiguration + .getCommandLine(actionName, variables, expander, PathMapper.NOOP) + .stream() .filter(s -> !s.contains(linkerParamFile)) .collect(toImmutableList())); } else { - argv.addAll(featureConfiguration.getCommandLine(actionName, variables, expander)); + argv.addAll( + featureConfiguration.getCommandLine(actionName, variables, expander, PathMapper.NOOP)); } } catch (ExpansionException e) { throw new CommandLineExpansionException(e.getMessage()); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendArtifacts.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendArtifacts.java index 2ed7feb1d4fa87..169e7a9a70745b 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendArtifacts.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendArtifacts.java @@ -367,7 +367,7 @@ public ImmutableList arguments( try { args.addAll( featureConfiguration.getCommandLine( - CppActionNames.LTO_BACKEND, buildVariables, artifactExpander)); + CppActionNames.LTO_BACKEND, buildVariables, artifactExpander, pathMapper)); } catch (ExpansionException e) { throw new CommandLineExpansionException(e.getMessage()); } diff --git a/src/test/java/com/google/devtools/build/lib/exec/util/SpawnBuilder.java b/src/test/java/com/google/devtools/build/lib/exec/util/SpawnBuilder.java index 334684427c53f4..c0515b9d1c4630 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/util/SpawnBuilder.java +++ b/src/test/java/com/google/devtools/build/lib/exec/util/SpawnBuilder.java @@ -23,7 +23,6 @@ import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.ActionInputHelper; import com.google.devtools.build.lib.actions.Artifact; -import com.google.devtools.build.lib.actions.DelegateSpawn; import com.google.devtools.build.lib.actions.EmptyRunfilesSupplier; import com.google.devtools.build.lib.actions.FilesetOutputSymlink; import com.google.devtools.build.lib.actions.PathMapper; @@ -66,27 +65,6 @@ public final class SpawnBuilder { private PathMapper pathMapper = PathMapper.NOOP; private boolean builtForToolConfiguration; - /** - * A {@link DelegateSpawn} that supports output path mapping as described in {@link - * com.google.devtools.build.lib.actions.PathMapper}. - * - *

By overriding {@link #getPathMapper()} - and only in test code - instead of adding an extra - * field in {@link SimpleSpawn}, we avoid further pressuring memory on large build graphs. - */ - private static class PathStrippableSpawn extends DelegateSpawn { - private final PathMapper pathMapper; - - public PathStrippableSpawn(Spawn spawn, PathMapper pathMapper) { - super(spawn); - this.pathMapper = pathMapper; - } - - @Override - public PathMapper getPathMapper() { - return pathMapper; - } - } - public SpawnBuilder(String... args) { this.args = ImmutableList.copyOf(args); } @@ -102,19 +80,18 @@ public Spawn build() { platform, execProperties, builtForToolConfiguration); - return new PathStrippableSpawn( - new SimpleSpawn( - owner, - ImmutableList.copyOf(args), - ImmutableMap.copyOf(environment), - ImmutableMap.copyOf(executionInfo), - runfilesSupplier, - ImmutableMap.copyOf(filesetMappings), - inputs.build(), - tools.build(), - ImmutableSet.copyOf(outputs), - mandatoryOutputs, - resourceSet), + return new SimpleSpawn( + owner, + ImmutableList.copyOf(args), + ImmutableMap.copyOf(environment), + ImmutableMap.copyOf(executionInfo), + runfilesSupplier, + ImmutableMap.copyOf(filesetMappings), + inputs.build(), + tools.build(), + ImmutableSet.copyOf(outputs), + mandatoryOutputs, + resourceSet, pathMapper); } diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java b/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java index 499733c00493f1..519e98fac19a94 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java +++ b/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java @@ -20,6 +20,7 @@ import com.google.common.base.Predicate; import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.PathMapper; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.PackageIdentifier; import com.google.devtools.build.lib.packages.util.Crosstool.CcToolchainConfig; @@ -179,7 +180,8 @@ public static ImmutableList getLinkopts(LinkCommandLine linkCommandLine) .isAvailable(LinkBuildVariables.USER_LINK_FLAGS.getVariableName())) { return CcToolchainVariables.toStringList( linkCommandLine.getBuildVariables(), - LinkBuildVariables.USER_LINK_FLAGS.getVariableName()); + LinkBuildVariables.USER_LINK_FLAGS.getVariableName(), + PathMapper.NOOP); } else { return ImmutableList.of(); } diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/BUILD b/src/test/java/com/google/devtools/build/lib/rules/cpp/BUILD index 18e7e91f87759d..97231ee9e395be 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/BUILD +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/BUILD @@ -374,6 +374,7 @@ java_test( name = "CompileBuildVariablesTest", srcs = ["CompileBuildVariablesTest.java"], deps = [ + "//src/main/java/com/google/devtools/build/lib/actions:artifacts", "//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster", "//src/main/java/com/google/devtools/build/lib/analysis:configured_target", "//src/main/java/com/google/devtools/build/lib/rules/cpp", diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeaturesTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeaturesTest.java index fa527099c61565..cb8dbd282fe944 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeaturesTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeaturesTest.java @@ -28,6 +28,7 @@ import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.ArtifactRoot; import com.google.devtools.build.lib.actions.ArtifactRoot.RootType; +import com.google.devtools.build.lib.actions.PathMapper; import com.google.devtools.build.lib.actions.util.ActionsTestUtil; import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.ActionConfig; @@ -289,7 +290,8 @@ public void testEnvVars() throws Exception { "feature { name: 'g' }") .getFeatureConfiguration(ImmutableSet.of("a", "b", "d", "f")); ImmutableMap env = - configuration.getEnvironmentVariables(CppActionNames.CPP_COMPILE, createVariables()); + configuration.getEnvironmentVariables( + CppActionNames.CPP_COMPILE, createVariables(), PathMapper.NOOP); assertThat(env) .containsExactly( "foo", "bar", "cat", "meow", "dog", "woof", @@ -314,7 +316,8 @@ public void testEnvVarsWithMissingVariableIsNotExpanded() throws Exception { .getFeatureConfiguration(ImmutableSet.of("a")); ImmutableMap env = - configuration.getEnvironmentVariables(CppActionNames.CPP_COMPILE, createVariables()); + configuration.getEnvironmentVariables( + CppActionNames.CPP_COMPILE, createVariables(), PathMapper.NOOP); assertThat(env).doesNotContainEntry("foo", "bar"); } @@ -334,7 +337,7 @@ public void testEnvVarsWithAllVariablesPresentAreExpanded() throws Exception { ImmutableMap env = configuration.getEnvironmentVariables( - CppActionNames.CPP_COMPILE, createVariables("v", "1")); + CppActionNames.CPP_COMPILE, createVariables("v", "1"), PathMapper.NOOP); assertThat(env).containsExactly("foo", "bar").inOrder(); } @@ -355,7 +358,7 @@ public void testEnvVarsWithAllVariablesPresentAreExpandedWithVariableExpansion() ImmutableMap env = configuration.getEnvironmentVariables( - CppActionNames.CPP_COMPILE, createVariables("v", "1")); + CppActionNames.CPP_COMPILE, createVariables("v", "1"), PathMapper.NOOP); assertThat(env).containsExactly("foo", "1").inOrder(); } @@ -1728,7 +1731,7 @@ public void testLibraryToLinkValue() throws ExpansionException { assertThat( LibraryToLinkValue.forDynamicLibrary("foo") .getFieldValue("LibraryToLinkValue", LibraryToLinkValue.NAME_FIELD_NAME) - .getStringValue(LibraryToLinkValue.NAME_FIELD_NAME)) + .getStringValue(LibraryToLinkValue.NAME_FIELD_NAME, PathMapper.NOOP)) .isEqualTo("foo"); assertThat( LibraryToLinkValue.forDynamicLibrary("foo") @@ -1745,10 +1748,10 @@ public void testLibraryToLinkValue() throws ExpansionException { Iterable objects = LibraryToLinkValue.forObjectFileGroup(testArtifacts, false) .getFieldValue("LibraryToLinkValue", LibraryToLinkValue.OBJECT_FILES_FIELD_NAME) - .getSequenceValue(LibraryToLinkValue.OBJECT_FILES_FIELD_NAME); + .getSequenceValue(LibraryToLinkValue.OBJECT_FILES_FIELD_NAME, PathMapper.NOOP); ImmutableList.Builder objectNames = ImmutableList.builder(); for (VariableValue object : objects) { - objectNames.add(object.getStringValue("name")); + objectNames.add(object.getStringValue("name", PathMapper.NOOP)); } assertThat(objectNames.build()) .containsExactlyElementsIn(Iterables.transform(testArtifacts, Artifact::getExecPathString)); diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CompileBuildVariablesTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CompileBuildVariablesTest.java index 9e163c15931267..bca4814470d09b 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CompileBuildVariablesTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CompileBuildVariablesTest.java @@ -18,6 +18,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; +import com.google.devtools.build.lib.actions.PathMapper; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget; import com.google.devtools.build.lib.analysis.util.AnalysisMock; @@ -59,9 +60,13 @@ public void testPresenceOfBasicVariables() throws Exception { CcToolchainVariables variables = getCompileBuildVariables("//x:bin", "bin"); - assertThat(variables.getStringVariable(CompileBuildVariables.SOURCE_FILE.getVariableName())) + assertThat( + variables.getStringVariable( + CompileBuildVariables.SOURCE_FILE.getVariableName(), PathMapper.NOOP)) .contains("x/bin.cc"); - assertThat(variables.getStringVariable(CompileBuildVariables.OUTPUT_FILE.getVariableName())) + assertThat( + variables.getStringVariable( + CompileBuildVariables.OUTPUT_FILE.getVariableName(), PathMapper.NOOP)) .contains("_objs/bin/bin"); } @@ -76,7 +81,7 @@ public void testPresenceOfConfigurationCompileFlags() throws Exception { ImmutableList userCopts = CcToolchainVariables.toStringList( - variables, CompileBuildVariables.USER_COMPILE_FLAGS.getVariableName()); + variables, CompileBuildVariables.USER_COMPILE_FLAGS.getVariableName(), PathMapper.NOOP); assertThat(userCopts) .containsAtLeastElementsIn(ImmutableList.of("-foo", "-bar")) .inOrder(); @@ -93,7 +98,7 @@ public void testPresenceOfUserCompileFlags() throws Exception { ImmutableList copts = CcToolchainVariables.toStringList( - variables, CompileBuildVariables.USER_COMPILE_FLAGS.getVariableName()); + variables, CompileBuildVariables.USER_COMPILE_FLAGS.getVariableName(), PathMapper.NOOP); assertThat(copts).contains("-foo"); } @@ -110,7 +115,7 @@ public void testPerFileCoptsAreInUserCompileFlags() throws Exception { ImmutableList copts = CcToolchainVariables.toStringList( - variables, CompileBuildVariables.USER_COMPILE_FLAGS.getVariableName()); + variables, CompileBuildVariables.USER_COMPILE_FLAGS.getVariableName(), PathMapper.NOOP); assertThat(copts).containsExactly("-foo").inOrder(); } @@ -129,7 +134,7 @@ public void testHostPerFileCoptsAreInUserCompileFlags() throws Exception { ImmutableList copts = CcToolchainVariables.toStringList( - variables, CompileBuildVariables.USER_COMPILE_FLAGS.getVariableName()); + variables, CompileBuildVariables.USER_COMPILE_FLAGS.getVariableName(), PathMapper.NOOP); assertThat(copts).contains("-foo"); assertThat(copts).doesNotContain("-bar"); assertThat(copts).doesNotContain("-baz"); @@ -148,7 +153,7 @@ public void testPresenceOfSysrootBuildVariable() throws Exception { CcToolchainVariables variables = getCompileBuildVariables("//x:bin", "bin"); - assertThat(variables.getStringVariable(CcCommon.SYSROOT_VARIABLE_NAME)) + assertThat(variables.getStringVariable(CcCommon.SYSROOT_VARIABLE_NAME, PathMapper.NOOP)) .isEqualTo("/usr/local/custom-sysroot"); } @@ -166,7 +171,7 @@ public void testTargetSysrootWithoutPlatforms() throws Exception { CcToolchainVariables variables = getCompileBuildVariables("//x:bin", "bin"); - assertThat(variables.getStringVariable(CcCommon.SYSROOT_VARIABLE_NAME)) + assertThat(variables.getStringVariable(CcCommon.SYSROOT_VARIABLE_NAME, PathMapper.NOOP)) .isEqualTo("target_libc"); } @@ -188,7 +193,7 @@ public void testTargetSysrootWithPlatforms() throws Exception { CcToolchainVariables variables = getCompileBuildVariables("//x:bin", "bin"); - assertThat(variables.getStringVariable(CcCommon.SYSROOT_VARIABLE_NAME)) + assertThat(variables.getStringVariable(CcCommon.SYSROOT_VARIABLE_NAME, PathMapper.NOOP)) .isEqualTo("target_libc"); } @@ -208,7 +213,8 @@ public void testPresenceOfPerObjectDebugFileBuildVariable() throws Exception { assertThat( variables.getStringVariable( - CompileBuildVariables.PER_OBJECT_DEBUG_INFO_FILE.getVariableName())) + CompileBuildVariables.PER_OBJECT_DEBUG_INFO_FILE.getVariableName(), + PathMapper.NOOP)) .isNotNull(); } @@ -227,7 +233,8 @@ public void testPresenceOfIsUsingFissionVariable() throws Exception { CcToolchainVariables variables = getCompileBuildVariables("//x:bin", "bin"); assertThat( - variables.getStringVariable(CompileBuildVariables.IS_USING_FISSION.getVariableName())) + variables.getStringVariable( + CompileBuildVariables.IS_USING_FISSION.getVariableName(), PathMapper.NOOP)) .isNotNull(); } @@ -298,7 +305,8 @@ public void testPresenceOfPerObjectDebugFileBuildVariableUsingLegacyFields() thr assertThat( variables.getStringVariable( - CompileBuildVariables.PER_OBJECT_DEBUG_INFO_FILE.getVariableName())) + CompileBuildVariables.PER_OBJECT_DEBUG_INFO_FILE.getVariableName(), + PathMapper.NOOP)) .isNotNull(); } @@ -313,7 +321,8 @@ public void testPresenceOfMinOsVersionBuildVariable() throws Exception { scratch.file("x/bin.cc"); CcToolchainVariables variables = getCompileBuildVariables("//x:bin", "bin"); - assertThat(variables.getStringVariable(CcCommon.MINIMUM_OS_VERSION_VARIABLE_NAME)) + assertThat( + variables.getStringVariable(CcCommon.MINIMUM_OS_VERSION_VARIABLE_NAME, PathMapper.NOOP)) .isEqualTo("6"); } @@ -362,7 +371,9 @@ public void testExternalIncludePathsVariable() throws Exception { assertThat( CcToolchainVariables.toStringList( - variables, CompileBuildVariables.EXTERNAL_INCLUDE_PATHS.getVariableName()) + variables, + CompileBuildVariables.EXTERNAL_INCLUDE_PATHS.getVariableName(), + PathMapper.NOOP) .stream() .map(x -> removeOutDirectory(x)) .collect(ImmutableList.toImmutableList())) diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CompileCommandLineTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CompileCommandLineTest.java index f71d504a6d0a2f..1aa4ac6cf4d1d4 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CompileCommandLineTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CompileCommandLineTest.java @@ -19,6 +19,7 @@ import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.ArtifactRoot; import com.google.devtools.build.lib.actions.ArtifactRoot.RootType; +import com.google.devtools.build.lib.actions.PathMapper; import com.google.devtools.build.lib.actions.util.ActionsTestUtil; import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; import com.google.devtools.build.lib.rules.cpp.CcCommon.CoptsFilter; @@ -100,7 +101,7 @@ public void testFeatureConfigurationCommandLineIsUsed() throws Exception { .build(); assertThat( compileCommandLine.getArguments( - /* parameterFilePath= */ null, /* overwrittenVariables= */ null)) + /* parameterFilePath= */ null, /* overwrittenVariables= */ null, PathMapper.NOOP)) .contains("-some_foo_flag"); } @@ -143,7 +144,7 @@ private List getCompileCommandLineWithCoptsFilter(String featureName) th .setCoptsFilter(CoptsFilter.fromRegex(Pattern.compile(".*i_am_a_flag.*"))) .build(); return compileCommandLine.getArguments( - /* parameterFilePath= */ null, /* overwrittenVariables= */ null); + /* parameterFilePath= */ null, /* overwrittenVariables= */ null, PathMapper.NOOP); } private CompileCommandLine.Builder makeCompileCommandLineBuilder() throws Exception { diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java index c2159d0f179844..f810a06bcb9c95 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java @@ -313,10 +313,11 @@ public void testExposesRuntimeLibrarySearchDirectoriesVariable() throws Exceptio .getLinkCommandLineForTesting() .getBuildVariables() .getSequenceVariable( - LinkBuildVariables.RUNTIME_LIBRARY_SEARCH_DIRECTORIES.getVariableName()); + LinkBuildVariables.RUNTIME_LIBRARY_SEARCH_DIRECTORIES.getVariableName(), + PathMapper.NOOP); List directories = new ArrayList<>(); for (VariableValue value : runtimeLibrarySearchDirectories) { - directories.add(value.getStringValue("runtime_library_search_directory")); + directories.add(value.getStringValue("runtime_library_search_directory", PathMapper.NOOP)); } assertThat(Joiner.on(" ").join(directories)).matches(".*some-dir .*some-other-dir"); } diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscoveryTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscoveryTest.java index d1c2c0aeb4e225..9284e9eca59b30 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscoveryTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscoveryTest.java @@ -24,6 +24,7 @@ import com.google.devtools.build.lib.actions.ArtifactResolver; import com.google.devtools.build.lib.actions.ArtifactRoot; import com.google.devtools.build.lib.actions.ArtifactRoot.RootType; +import com.google.devtools.build.lib.actions.PathMapper; import com.google.devtools.build.lib.actions.util.ActionsTestUtil; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; @@ -68,16 +69,18 @@ private void checkHeaderInclusion( ImmutableList dependencies, NestedSet includedHeaders) throws ActionExecutionException { - HeaderDiscovery.discoverInputsFromDependencies( - new ActionsTestUtil.NullAction(), - ActionsTestUtil.createArtifact(artifactRoot, derivedRoot.getRelative("foo.cc")), - /*shouldValidateInclusions=*/ true, - dependencies, - /*permittedSystemIncludePrefixes=*/ ImmutableList.of(), - includedHeaders, - execRoot, - artifactResolver, - /*siblingRepositoryLayout=*/ false); + var unused = + HeaderDiscovery.discoverInputsFromDependencies( + new ActionsTestUtil.NullAction(), + ActionsTestUtil.createArtifact(artifactRoot, derivedRoot.getRelative("foo.cc")), + /* shouldValidateInclusions= */ true, + dependencies, + /* permittedSystemIncludePrefixes= */ ImmutableList.of(), + includedHeaders, + execRoot, + artifactResolver, + /* siblingRepositoryLayout= */ false, + PathMapper.NOOP); } private SpecialArtifact treeArtifact(Path path) { diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/LibraryToLinkValueTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/LibraryToLinkValueTest.java index b2c758fec5e25f..6cc1e2ab786f1b 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/LibraryToLinkValueTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/LibraryToLinkValueTest.java @@ -22,6 +22,7 @@ import com.google.devtools.build.lib.actions.Artifact.SourceArtifact; import com.google.devtools.build.lib.actions.ArtifactOwner; import com.google.devtools.build.lib.actions.ArtifactRoot; +import com.google.devtools.build.lib.actions.PathMapper; import com.google.devtools.build.lib.rules.cpp.CcToolchainVariables.LibraryToLinkValue; import com.google.devtools.build.lib.vfs.DigestHashFunction; import com.google.devtools.build.lib.vfs.PathFragment; @@ -108,7 +109,7 @@ public void getFieldValue_forDynamicLibrary() throws Exception { /* field= */ "type", /* expander= */ null, /* throwOnMissingVariable= */ false) - .getStringValue("variable name doesn't matter")) + .getStringValue("variable name doesn't matter", PathMapper.NOOP)) .isEqualTo("dynamic_library"); assertThat( libraryToLinkValue @@ -117,7 +118,7 @@ public void getFieldValue_forDynamicLibrary() throws Exception { /* field= */ "is_whole_archive", /* expander= */ null, /* throwOnMissingVariable= */ false) - .getStringValue("variable name doesn't matter")) + .getStringValue("variable name doesn't matter", PathMapper.NOOP)) .isEqualTo("0"); assertThat( libraryToLinkValue @@ -126,7 +127,7 @@ public void getFieldValue_forDynamicLibrary() throws Exception { /* field= */ "name", /* expander= */ null, /* throwOnMissingVariable= */ false) - .getStringValue("variable name doesn't matter")) + .getStringValue("variable name doesn't matter", PathMapper.NOOP)) .isEqualTo("foo"); assertThat( libraryToLinkValue.getFieldValue( @@ -147,7 +148,7 @@ public void getFieldValue_forVersionedDynamicLibrary() throws Exception { /* field= */ "type", /* expander= */ null, /* throwOnMissingVariable= */ false) - .getStringValue("variable name doesn't matter")) + .getStringValue("variable name doesn't matter", PathMapper.NOOP)) .isEqualTo("versioned_dynamic_library"); assertThat( libraryToLinkValue @@ -156,7 +157,7 @@ public void getFieldValue_forVersionedDynamicLibrary() throws Exception { /* field= */ "is_whole_archive", /* expander= */ null, /* throwOnMissingVariable= */ false) - .getStringValue("variable name doesn't matter")) + .getStringValue("variable name doesn't matter", PathMapper.NOOP)) .isEqualTo("0"); assertThat( libraryToLinkValue @@ -165,7 +166,7 @@ public void getFieldValue_forVersionedDynamicLibrary() throws Exception { /* field= */ "name", /* expander= */ null, /* throwOnMissingVariable= */ false) - .getStringValue("variable name doesn't matter")) + .getStringValue("variable name doesn't matter", PathMapper.NOOP)) .isEqualTo("foo"); assertThat( libraryToLinkValue.getFieldValue( @@ -186,7 +187,7 @@ public void getFieldValue_forInterfaceLibrary() throws Exception { /* field= */ "type", /* expander= */ null, /* throwOnMissingVariable= */ false) - .getStringValue("variable name doesn't matter")) + .getStringValue("variable name doesn't matter", PathMapper.NOOP)) .isEqualTo("interface_library"); assertThat( libraryToLinkValue @@ -195,7 +196,7 @@ public void getFieldValue_forInterfaceLibrary() throws Exception { /* field= */ "is_whole_archive", /* expander= */ null, /* throwOnMissingVariable= */ false) - .getStringValue("variable name doesn't matter")) + .getStringValue("variable name doesn't matter", PathMapper.NOOP)) .isEqualTo("0"); assertThat( libraryToLinkValue @@ -204,7 +205,7 @@ public void getFieldValue_forInterfaceLibrary() throws Exception { /* field= */ "name", /* expander= */ null, /* throwOnMissingVariable= */ false) - .getStringValue("variable name doesn't matter")) + .getStringValue("variable name doesn't matter", PathMapper.NOOP)) .isEqualTo("foo"); assertThat( libraryToLinkValue.getFieldValue( @@ -226,7 +227,7 @@ public void getFieldValue_forStaticLibrary() throws Exception { /* field= */ "type", /* expander= */ null, /* throwOnMissingVariable= */ false) - .getStringValue("variable name doesn't matter")) + .getStringValue("variable name doesn't matter", PathMapper.NOOP)) .isEqualTo("static_library"); assertThat( libraryToLinkValue @@ -235,7 +236,7 @@ public void getFieldValue_forStaticLibrary() throws Exception { /* field= */ "is_whole_archive", /* expander= */ null, /* throwOnMissingVariable= */ false) - .getStringValue("variable name doesn't matter")) + .getStringValue("variable name doesn't matter", PathMapper.NOOP)) .isEqualTo("0"); assertThat( libraryToLinkValue @@ -244,7 +245,7 @@ public void getFieldValue_forStaticLibrary() throws Exception { /* field= */ "name", /* expander= */ null, /* throwOnMissingVariable= */ false) - .getStringValue("variable name doesn't matter")) + .getStringValue("variable name doesn't matter", PathMapper.NOOP)) .isEqualTo("foo"); assertThat( libraryToLinkValue.getFieldValue( @@ -266,7 +267,7 @@ public void getFieldValue_forStaticLibrary_isWholeArchive() throws Exception { /* field= */ "type", /* expander= */ null, /* throwOnMissingVariable= */ false) - .getStringValue("variable name doesn't matter")) + .getStringValue("variable name doesn't matter", PathMapper.NOOP)) .isEqualTo("static_library"); assertThat( libraryToLinkValue @@ -275,7 +276,7 @@ public void getFieldValue_forStaticLibrary_isWholeArchive() throws Exception { /* field= */ "is_whole_archive", /* expander= */ null, /* throwOnMissingVariable= */ false) - .getStringValue("variable name doesn't matter")) + .getStringValue("variable name doesn't matter", PathMapper.NOOP)) .isEqualTo("1"); assertThat( libraryToLinkValue @@ -284,7 +285,7 @@ public void getFieldValue_forStaticLibrary_isWholeArchive() throws Exception { /* field= */ "name", /* expander= */ null, /* throwOnMissingVariable= */ false) - .getStringValue("variable name doesn't matter")) + .getStringValue("variable name doesn't matter", PathMapper.NOOP)) .isEqualTo("foo"); assertThat( libraryToLinkValue.getFieldValue( @@ -306,7 +307,7 @@ public void getFieldValue_forObjectFile() throws Exception { /* field= */ "type", /* expander= */ null, /* throwOnMissingVariable= */ false) - .getStringValue("variable name doesn't matter")) + .getStringValue("variable name doesn't matter", PathMapper.NOOP)) .isEqualTo("object_file"); assertThat( libraryToLinkValue @@ -315,7 +316,7 @@ public void getFieldValue_forObjectFile() throws Exception { /* field= */ "is_whole_archive", /* expander= */ null, /* throwOnMissingVariable= */ false) - .getStringValue("variable name doesn't matter")) + .getStringValue("variable name doesn't matter", PathMapper.NOOP)) .isEqualTo("0"); assertThat( libraryToLinkValue @@ -324,7 +325,7 @@ public void getFieldValue_forObjectFile() throws Exception { /* field= */ "name", /* expander= */ null, /* throwOnMissingVariable= */ false) - .getStringValue("variable name doesn't matter")) + .getStringValue("variable name doesn't matter", PathMapper.NOOP)) .isEqualTo("foo"); assertThat( libraryToLinkValue.getFieldValue( @@ -346,7 +347,7 @@ public void getFieldValue_forObjectFile_isWholeArchive() throws Exception { /* field= */ "type", /* expander= */ null, /* throwOnMissingVariable= */ false) - .getStringValue("variable name doesn't matter")) + .getStringValue("variable name doesn't matter", PathMapper.NOOP)) .isEqualTo("object_file"); assertThat( libraryToLinkValue @@ -355,7 +356,7 @@ public void getFieldValue_forObjectFile_isWholeArchive() throws Exception { /* field= */ "is_whole_archive", /* expander= */ null, /* throwOnMissingVariable= */ false) - .getStringValue("variable name doesn't matter")) + .getStringValue("variable name doesn't matter", PathMapper.NOOP)) .isEqualTo("1"); assertThat( libraryToLinkValue @@ -364,7 +365,7 @@ public void getFieldValue_forObjectFile_isWholeArchive() throws Exception { /* field= */ "name", /* expander= */ null, /* throwOnMissingVariable= */ false) - .getStringValue("variable name doesn't matter")) + .getStringValue("variable name doesn't matter", PathMapper.NOOP)) .isEqualTo("foo"); assertThat( libraryToLinkValue.getFieldValue( @@ -394,7 +395,7 @@ public void getFieldValue_forObjectFileGroup() throws Exception { /* field= */ "type", /* expander= */ null, /* throwOnMissingVariable= */ false) - .getStringValue("variable name doesn't matter")) + .getStringValue("variable name doesn't matter", PathMapper.NOOP)) .isEqualTo("object_file_group"); assertThat( libraryToLinkValue @@ -403,7 +404,7 @@ public void getFieldValue_forObjectFileGroup() throws Exception { /* field= */ "is_whole_archive", /* expander= */ null, /* throwOnMissingVariable= */ false) - .getStringValue("variable name doesn't matter")) + .getStringValue("variable name doesn't matter", PathMapper.NOOP)) .isEqualTo("0"); assertThat( libraryToLinkValue.getFieldValue( @@ -420,8 +421,8 @@ public void getFieldValue_forObjectFileGroup() throws Exception { /* field= */ "object_files", /* expander= */ null, /* throwOnMissingVariable= */ false) - .getSequenceValue("variable name doesn't matter")) - .getStringValue("variable name doesn't matter")) + .getSequenceValue("variable name doesn't matter", PathMapper.NOOP)) + .getStringValue("variable name doesn't matter", PathMapper.NOOP)) .isEqualTo("artifact"); } @@ -444,7 +445,7 @@ public void getFieldValue_forObjectFileGroup_isWholeArchive() throws Exception { /* field= */ "type", /* expander= */ null, /* throwOnMissingVariable= */ false) - .getStringValue("variable name doesn't matter")) + .getStringValue("variable name doesn't matter", PathMapper.NOOP)) .isEqualTo("object_file_group"); assertThat( libraryToLinkValue @@ -453,7 +454,7 @@ public void getFieldValue_forObjectFileGroup_isWholeArchive() throws Exception { /* field= */ "is_whole_archive", /* expander= */ null, /* throwOnMissingVariable= */ false) - .getStringValue("variable name doesn't matter")) + .getStringValue("variable name doesn't matter", PathMapper.NOOP)) .isEqualTo("1"); assertThat( libraryToLinkValue.getFieldValue( @@ -470,8 +471,8 @@ public void getFieldValue_forObjectFileGroup_isWholeArchive() throws Exception { /* field= */ "object_files", /* expander= */ null, /* throwOnMissingVariable= */ false) - .getSequenceValue("variable name doesn't matter")) - .getStringValue("variable name doesn't matter")) + .getSequenceValue("variable name doesn't matter", PathMapper.NOOP)) + .getStringValue("variable name doesn't matter", PathMapper.NOOP)) .isEqualTo("artifact"); } } diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariablesTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariablesTest.java index 5b07cb02fb209a..07a999fe29496f 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariablesTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariablesTest.java @@ -21,6 +21,7 @@ import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.Action; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.PathMapper; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.util.AnalysisMock; @@ -94,7 +95,7 @@ public void testLibrariesToLinkAreExported() throws Exception { assertThat(librariesToLinkSequence).isNotNull(); Iterable librariesToLink = librariesToLinkSequence.getSequenceValue( - LinkBuildVariables.LIBRARIES_TO_LINK.getVariableName()); + LinkBuildVariables.LIBRARIES_TO_LINK.getVariableName(), PathMapper.NOOP); assertThat(librariesToLink).hasSize(1); VariableValue nameValue = librariesToLink @@ -104,7 +105,7 @@ public void testLibrariesToLinkAreExported() throws Exception { LinkBuildVariables.LIBRARIES_TO_LINK.getVariableName(), LibraryToLinkValue.NAME_FIELD_NAME); assertThat(nameValue).isNotNull(); - String name = nameValue.getStringValue(LibraryToLinkValue.NAME_FIELD_NAME); + String name = nameValue.getStringValue(LibraryToLinkValue.NAME_FIELD_NAME, PathMapper.NOOP); assertThat(name).matches(".*a\\..*o"); } @@ -139,7 +140,7 @@ public void testLinkSimpleLibName() throws Exception { variables.getVariable(LinkBuildVariables.LIBRARIES_TO_LINK.getVariableName()); Iterable librariestoLink = librariesToLinkSequence.getSequenceValue( - LinkBuildVariables.LIBRARIES_TO_LINK.getVariableName()); + LinkBuildVariables.LIBRARIES_TO_LINK.getVariableName(), PathMapper.NOOP); VariableValue nameValue = librariestoLink .iterator() @@ -148,7 +149,7 @@ public void testLinkSimpleLibName() throws Exception { LinkBuildVariables.LIBRARIES_TO_LINK.getVariableName(), LibraryToLinkValue.NAME_FIELD_NAME); assertThat(nameValue).isNotNull(); - String name = nameValue.getStringValue(LibraryToLinkValue.NAME_FIELD_NAME); + String name = nameValue.getStringValue(LibraryToLinkValue.NAME_FIELD_NAME, PathMapper.NOOP); assertThat(name).isEqualTo("bar"); } @@ -166,7 +167,7 @@ public void testLinkVersionedLibName() throws Exception { variables.getVariable(LinkBuildVariables.LIBRARIES_TO_LINK.getVariableName()); Iterable librariestoLink = librariesToLinkSequence.getSequenceValue( - LinkBuildVariables.LIBRARIES_TO_LINK.getVariableName()); + LinkBuildVariables.LIBRARIES_TO_LINK.getVariableName(), PathMapper.NOOP); VariableValue nameValue = librariestoLink .iterator() @@ -175,7 +176,7 @@ public void testLinkVersionedLibName() throws Exception { LinkBuildVariables.LIBRARIES_TO_LINK.getVariableName(), LibraryToLinkValue.NAME_FIELD_NAME); assertThat(nameValue).isNotNull(); - String name = nameValue.getStringValue(LibraryToLinkValue.NAME_FIELD_NAME); + String name = nameValue.getStringValue(LibraryToLinkValue.NAME_FIELD_NAME, PathMapper.NOOP); assertThat(name).isEqualTo("libbar.so.1a.2"); } @@ -193,7 +194,7 @@ public void testLinkUnusualLibName() throws Exception { variables.getVariable(LinkBuildVariables.LIBRARIES_TO_LINK.getVariableName()); Iterable librariestoLink = librariesToLinkSequence.getSequenceValue( - LinkBuildVariables.LIBRARIES_TO_LINK.getVariableName()); + LinkBuildVariables.LIBRARIES_TO_LINK.getVariableName(), PathMapper.NOOP); VariableValue nameValue = librariestoLink .iterator() @@ -202,7 +203,7 @@ public void testLinkUnusualLibName() throws Exception { LinkBuildVariables.LIBRARIES_TO_LINK.getVariableName(), LibraryToLinkValue.NAME_FIELD_NAME); assertThat(nameValue).isNotNull(); - String name = nameValue.getStringValue(LibraryToLinkValue.NAME_FIELD_NAME); + String name = nameValue.getStringValue(LibraryToLinkValue.NAME_FIELD_NAME, PathMapper.NOOP); assertThat(name).isEqualTo("_libbar.so"); } @@ -626,7 +627,7 @@ public void testUserLinkFlagsWithLinkoptOption() throws Exception { ImmutableList userLinkFlags = CcToolchainVariables.toStringList( - testVariables, LinkBuildVariables.USER_LINK_FLAGS.getVariableName()); + testVariables, LinkBuildVariables.USER_LINK_FLAGS.getVariableName(), PathMapper.NOOP); assertThat(userLinkFlags).containsAtLeast("-foo", "-bar").inOrder(); } @@ -655,7 +656,7 @@ public void testLinkerInputsOverrideWholeArchive() throws Exception { assertThat(librariesToLinkSequence).isNotNull(); Iterable librariesToLink = librariesToLinkSequence.getSequenceValue( - LinkBuildVariables.LIBRARIES_TO_LINK.getVariableName()); + LinkBuildVariables.LIBRARIES_TO_LINK.getVariableName(), PathMapper.NOOP); assertThat(Iterables.size(librariesToLink)).isAnyOf(2, 3); Iterator librariesToLinkIterator = librariesToLink.iterator(); diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/StarlarkCcCommonTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/StarlarkCcCommonTest.java index f642a172a6b0df..dd5c542b5ba2af 100755 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/StarlarkCcCommonTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/StarlarkCcCommonTest.java @@ -25,6 +25,7 @@ import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.CommandLineExpansionException; +import com.google.devtools.build.lib.actions.PathMapper; import com.google.devtools.build.lib.actions.util.ActionsTestUtil; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.RuleContext; @@ -502,7 +503,7 @@ public void testGetEnvironment() throws Exception { assertThat(environmentVariables) .containsExactlyEntriesIn( featureConfiguration.getEnvironmentVariables( - CppActionNames.CPP_COMPILE, CcToolchainVariables.EMPTY)); + CppActionNames.CPP_COMPILE, CcToolchainVariables.EMPTY, PathMapper.NOOP)); } @Test @@ -7445,9 +7446,21 @@ public void testVariableExtensionCompileApi() throws Exception { CppCompileAction action = (CppCompileAction) getGeneratingAction(artifactByPath(getFilesToBuild(target), ".o")); - action.getCompileCommandLine().getVariables().getSequenceVariable("string_sequence_variable"); - action.getCompileCommandLine().getVariables().getStringVariable("string_variable"); - action.getCompileCommandLine().getVariables().getSequenceVariable("string_depset_variable"); + var unused1 = + action + .getCompileCommandLine() + .getVariables() + .getSequenceVariable("string_sequence_variable", PathMapper.NOOP); + var unused2 = + action + .getCompileCommandLine() + .getVariables() + .getStringVariable("string_variable", PathMapper.NOOP); + var unused3 = + action + .getCompileCommandLine() + .getVariables() + .getSequenceVariable("string_depset_variable", PathMapper.NOOP); } @Test @@ -7461,12 +7474,15 @@ public void testVariableExtensionLinkingContextApi() throws Exception { action .getLinkCommandLineForTesting() .getBuildVariables() - .getSequenceVariable("string_sequence_variable"); - action.getLinkCommandLineForTesting().getBuildVariables().getStringVariable("string_variable"); + .getSequenceVariable("string_sequence_variable", PathMapper.NOOP); action .getLinkCommandLineForTesting() .getBuildVariables() - .getSequenceVariable("string_depset_variable"); + .getStringVariable("string_variable", PathMapper.NOOP); + action + .getLinkCommandLineForTesting() + .getBuildVariables() + .getSequenceVariable("string_depset_variable", PathMapper.NOOP); } @Test @@ -7482,12 +7498,15 @@ public void testVariableExtensionLinkApi() throws Exception { action .getLinkCommandLineForTesting() .getBuildVariables() - .getSequenceVariable("string_sequence_variable"); - action.getLinkCommandLineForTesting().getBuildVariables().getStringVariable("string_variable"); + .getSequenceVariable("string_sequence_variable", PathMapper.NOOP); + action + .getLinkCommandLineForTesting() + .getBuildVariables() + .getStringVariable("string_variable", PathMapper.NOOP); action .getLinkCommandLineForTesting() .getBuildVariables() - .getSequenceVariable("string_depset_variable"); + .getSequenceVariable("string_depset_variable", PathMapper.NOOP); } @Test diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcLibraryTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcLibraryTest.java index 8f180ce1862e81..0fee4a51145b5f 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcLibraryTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcLibraryTest.java @@ -33,6 +33,7 @@ import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.CommandAction; import com.google.devtools.build.lib.actions.ExecutionRequirements; +import com.google.devtools.build.lib.actions.PathMapper; import com.google.devtools.build.lib.actions.util.ActionsTestUtil; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.FilesToRunProvider; @@ -1431,7 +1432,12 @@ public void testUsesDotdPruning() throws Exception { ActionExecutionException.class, () -> compileAction.discoverInputsFromDotdFiles( - new ActionExecutionContextBuilder().build(), null, null, null, false)); + new ActionExecutionContextBuilder().build(), + null, + null, + null, + false, + PathMapper.NOOP)); assertThat(expected).hasMessageThat().contains("error while parsing .d file"); } @@ -2218,7 +2224,7 @@ public void testCoptsLocationIsExpanded() throws Exception { useConfiguration("--apple_platform_type=ios", "--cpu=ios_x86_64"); CppCompileAction compileA = (CppCompileAction) compileAction("//bin:lib", "lib1.o"); - assertThat(compileA.compileCommandLine.getCopts()) + assertThat(compileA.compileCommandLine.getCopts(PathMapper.NOOP)) .containsAtLeast("bin/lib1.m", "bin/lib2.m", "bin/data.data", "bin/header.h"); } diff --git a/src/test/shell/bazel/path_mapping_test.sh b/src/test/shell/bazel/path_mapping_test.sh index 4b575c3b520ba0..560625b8993106 100755 --- a/src/test/shell/bazel/path_mapping_test.sh +++ b/src/test/shell/bazel/path_mapping_test.sh @@ -325,4 +325,225 @@ EOF bazel build --experimental_output_paths=strip //pkg:all &> $TEST_log || fail "build failed unexpectedly" } +# Verifies that path mapping results in cache hits for CppCompile actions +# subject to transitions that don't affect their inputs. +function test_path_stripping_cc_remote() { + local -r pkg="${FUNCNAME[0]}" + + mkdir -p "$pkg" + cat > "$pkg/BUILD" < "$pkg/main.cc" < +#include "$pkg/lib1/lib1.h" +#include "lib2.h" + +int main() { + std::cout << GetLib1Greeting() << std::endl; + std::cout << GetLib2Greeting() << std::endl; + return 0; +} +EOF + + mkdir -p "$pkg"/lib1 + cat > "$pkg/lib1/BUILD" < "$pkg/lib1/lib1.h" <<'EOF' +#ifndef LIB1_H_ +#define LIB1_H_ + +#include + +std::string GetLib1Greeting(); + +#endif +EOF + cat > "$pkg/lib1/lib1.cc" <<'EOF' +#include "lib1.h" +#include "other_dir/utils.h" + +std::string GetLib1Greeting() { + return AsGreeting("lib1"); +} +EOF + + mkdir -p "$pkg"/lib2 + cat > "$pkg/lib2/BUILD" < "$pkg/lib2/lib2.h.tpl" <<'EOF' +#ifndef LIB2_H_ +#define LIB2_H_ + +#include + +std::string GetLib2Greeting(); + +#endif +EOF + cat > "$pkg/lib2/lib2.cc.tpl" <<'EOF' +#include "lib2.h" +#include "other_dir/utils.h" + +std::string GetLib2Greeting() { + return AsGreeting("lib2"); +} +EOF + + mkdir -p "$pkg"/common/utils + cat > "$pkg/common/utils/BUILD" <<'EOF' +load(":defs.bzl", "greeting_setting") + +greeting_setting( + name = "greeting", + build_setting_default = "Hello", +) +genrule( + name = "gen_header", + srcs = ["utils.h.tpl"], + outs = ["dir/utils.h"], + cmd = "cp $< $@", +) +genrule( + name = "gen_source", + srcs = ["utils.cc.tpl"], + outs = ["dir/utils.cc"], + cmd = "sed -e 's/{GREETING}/$(GREETING)/' $< > $@", + toolchains = [":greeting"], +) +cc_library( + name = "utils", + srcs = ["dir/utils.cc"], + hdrs = ["dir/utils.h"], + include_prefix = "other_dir", + strip_include_prefix = "dir", + visibility = ["//visibility:public"], +) +EOF + cat > "$pkg/common/utils/utils.h.tpl" <<'EOF' +#ifndef SOME_PKG_UTILS_H_ +#define SOME_PKG_UTILS_H_ + +#include + +std::string AsGreeting(const std::string& name); +#endif +EOF + cat > "$pkg/common/utils/defs.bzl" < "$pkg/common/utils/utils.cc.tpl" <<'EOF' +#include "utils.h" + +std::string AsGreeting(const std::string& name) { + return "{GREETING}, " + name + "!"; +} +EOF + + bazel run \ + --verbose_failures \ + --experimental_output_paths=strip \ + --modify_execution_info=CppCompile=+supports-path-mapping \ + --remote_executor=grpc://localhost:${worker_port} \ + --features=-module_maps \ + "//$pkg:main" &>"$TEST_log" || fail "Expected success" + + expect_log 'Hello, lib1!' + expect_log 'Hello, lib2!' + expect_not_log 'remote cache hit' + + bazel run \ + --verbose_failures \ + --experimental_output_paths=strip \ + --modify_execution_info=CppCompile=+supports-path-mapping \ + --remote_executor=grpc://localhost:${worker_port} \ + --features=-module_maps \ + "//$pkg:transitioned_main" &>"$TEST_log" || fail "Expected success" + + expect_log 'Hi there, lib1!' + expect_log 'Hi there, lib2!' + # Compilation actions for lib1, lib2 and main should result in cache hits due + # to path stripping, utils is legitimately different and should not. + expect_log ' 3 remote cache hit' +} + run_suite "path mapping tests" diff --git a/src/test/shell/integration/config_stripped_outputs_lib.sh b/src/test/shell/integration/config_stripped_outputs_lib.sh index 63a4e689d979ea..e58aa50c5665ca 100644 --- a/src/test/shell/integration/config_stripped_outputs_lib.sh +++ b/src/test/shell/integration/config_stripped_outputs_lib.sh @@ -44,9 +44,9 @@ function assert_paths_stripped() { # "set -e" and silently ignored. output_paths=$(echo "$cmd" | tr -s ' ' '\n' | xargs -n 1 | grep -E -o "${bazel_out}[^)]*") for o in $output_paths; do - echo "$o" | grep -v "${bazel_out}/cfg/bin" \ + echo "$o" | grep -Ev "${bazel_out}/cfg/bin|${bazel_out}/cfg/genfiles" \ && fail "expected all \"${bazel_out}\" paths to start with " \ - "\"${bazel_out}/cfg/bin.*\": $o" + "\"${bazel_out}/cfg/bin.*\" or \"${bazel_out}/cfg/genfiles.*\": $o" if [[ "$o" == *"$identifying_action_output"* ]]; then found_identifying_output=1 fi @@ -64,9 +64,9 @@ function assert_paths_stripped() { fi param_file_paths=$(grep "${bazel_out}" $local_path || true) for k in $param_file_paths; do - echo "$k" | grep -v "${bazel_out}/cfg/bin" \ + echo "$k" | grep -Ev "${bazel_out}/cfg/bin|${bazel_out}/cfg/genfiles" \ && fail "$local_path: expected all \"${bazel_out}\" paths to start " \ - "with \"${bazel_out}/cfg/bin.*\": $k" + "with \"${bazel_out}/cfg/bin.*\" or \"${bazel_out}/cfg/genfiles.*\": $k" if [[ "$k" == *"$identifying_action_output"* ]]; then found_identifying_output=1 fi diff --git a/src/test/shell/integration/config_stripped_outputs_test.sh b/src/test/shell/integration/config_stripped_outputs_test.sh index da111c4ec0eab5..ef29f4735d764e 100755 --- a/src/test/shell/integration/config_stripped_outputs_test.sh +++ b/src/test/shell/integration/config_stripped_outputs_test.sh @@ -379,4 +379,153 @@ EOF assert_contains_no_stripped_path "${bazel_bin}/$pkg/java/hello/libd-hjar.jdeps" } +function test_builtin_cc_support() { + local -r pkg="third_party/${FUNCNAME[0]}" + mkdir -p "$pkg" + cat > "$pkg/BUILD" < "$pkg/main.cc" < +#include "$pkg/lib1/lib1.h" +#include "lib2.h" + +int main() { + std::cout << GetLib1Greeting() << std::endl; + std::cout << GetLib2Greeting() << std::endl; + return 0; +} +EOF + + mkdir -p "$pkg"/lib1 + cat > "$pkg/lib1/BUILD" < "$pkg/lib1/lib1.h" < + +std::string GetLib1Greeting(); + +#endif +EOF + cat > "$pkg/lib1/lib1.cc" < "$pkg/lib2/BUILD" < "$pkg/lib2/lib2.h.tpl" < + +std::string GetLib2Greeting(); + +#endif +EOF + cat > "$pkg/lib2/lib2.cc.tpl" < "$pkg/common/utils/BUILD" < "$pkg/common/utils/utils.h.tpl" < + +std::string AsGreeting(const std::string& name); +#endif +EOF + cat > "$pkg/common/utils/utils.cc.tpl" <"$TEST_log" || fail "Expected success" + + # Verify that all paths are stripped as expected. + # The extension can be .pic.o or .o depending on the platform. + assert_paths_stripped "$TEST_log" "$pkg/common/utils/_objs/utils/utils." + assert_paths_stripped "$TEST_log" "$pkg/lib1/_objs/lib1/lib1." + assert_paths_stripped "$TEST_log" "$pkg/lib2/_objs/lib2/lib2." + assert_paths_stripped "$TEST_log" "$pkg/_objs/main/main." +} + run_suite "Tests stripping config prefixes from output paths for better action caching"