From 943ba4ee2d7c7b5c3498ab068a221ce2e750606d Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Sat, 18 May 2024 23:28:29 +0200 Subject: [PATCH] Add PathMapper --- .../build/lib/actions/SimpleSpawn.java | 72 +++++- .../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 | 80 +++++-- .../rules/cpp/CppCompileActionTemplate.java | 12 +- .../build/lib/rules/cpp/CppHelper.java | 3 +- .../build/lib/rules/cpp/HeaderDiscovery.java | 8 +- .../build/lib/rules/cpp/LinkCommandLine.java | 11 +- .../lib/rules/cpp/LtoBackendArtifacts.java | 2 +- .../build/lib/exec/util/SpawnBuilder.java | 45 +--- .../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/HeaderDiscoveryTest.java | 4 +- .../lib/rules/cpp/LibraryToLinkValueTest.java | 61 ++--- .../lib/rules/cpp/LinkBuildVariablesTest.java | 21 +- .../lib/rules/cpp/StarlarkCcCommonTest.java | 42 +++- .../build/lib/rules/objc/ObjcLibraryTest.java | 10 +- src/test/shell/bazel/path_mapping_test.sh | 219 ++++++++++++++++++ .../config_stripped_outputs_test.sh | 149 ++++++++++++ 24 files changed, 774 insertions(+), 223 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 bef6547dd5c869..88d9cc1cbf62b1 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 @@ -41,6 +41,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; @@ -55,7 +56,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); @@ -77,6 +79,7 @@ private SimpleSpawn( this.localResourcesSupplier = localResourcesSupplier; this.localResourcesCached = null; } + this.pathMapper = pathMapper; } public SimpleSpawn( @@ -101,7 +104,8 @@ public SimpleSpawn( outputs, mandatoryOutputs, localResources, - /*localResourcesSupplier=*/ null); + /* localResourcesSupplier= */ null, + PathMapper.NOOP); } @SuppressWarnings("TooManyParameters") @@ -126,8 +130,63 @@ public SimpleSpawn( tools, outputs, mandatoryOutputs, - /*localResources=*/ null, - localResourcesSupplier); + /* localResources= */ null, + localResourcesSupplier, + PathMapper.NOOP); + } + + public SimpleSpawn( + ActionExecutionMetadata owner, + ImmutableList arguments, + ImmutableMap environment, + ImmutableMap executionInfo, + ImmutableMap> filesetMappings, + NestedSet inputs, + NestedSet tools, + Collection outputs, + @Nullable Set mandatoryOutputs, + LocalResourcesSupplier localResourcesSupplier, + PathMapper pathMapper) { + this( + owner, + arguments, + environment, + executionInfo, + filesetMappings, + inputs, + tools, + outputs, + mandatoryOutputs, + null, + localResourcesSupplier, + pathMapper); + } + + public SimpleSpawn( + ActionExecutionMetadata owner, + ImmutableList arguments, + ImmutableMap environment, + ImmutableMap executionInfo, + ImmutableMap> filesetMappings, + NestedSet inputs, + NestedSet tools, + Collection outputs, + @Nullable Set mandatoryOutputs, + ResourceSet localResources, + PathMapper pathMapper) { + this( + owner, + arguments, + environment, + executionInfo, + filesetMappings, + inputs, + tools, + outputs, + mandatoryOutputs, + localResources, + null, + pathMapper); } public SimpleSpawn( @@ -226,6 +285,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 8954f9006aa47f..7fc79cbe53fb24 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; @@ -287,7 +288,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 ef069fec61b8b8..e1ff458481176b 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.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()); } @@ -169,9 +171,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 @@ -250,14 +253,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()); } @@ -370,29 +375,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)) { @@ -418,7 +424,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; } @@ -443,9 +449,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 @@ -565,6 +572,7 @@ private void expandCommandLine( CcToolchainVariables variables, Set enabledFeatureNames, @Nullable ArtifactExpander expander, + PathMapper pathMapper, List commandLine) throws ExpansionException { for (String variable : expandIfAllAvailable) { @@ -579,7 +587,7 @@ private void expandCommandLine( return; } for (FlagGroup flagGroup : flagGroups) { - flagGroup.expandCommandLine(variables, expander, commandLine); + flagGroup.expandCommandLine(variables, expander, pathMapper, commandLine); } } @@ -711,6 +719,7 @@ ImmutableSet getWithFeatureSets() { private void expandEnvironment( String action, CcToolchainVariables variables, + PathMapper pathMapper, Set enabledFeatureNames, ImmutableMap.Builder envBuilder) throws ExpansionException { @@ -721,7 +730,7 @@ private void expandEnvironment( return; } for (EnvEntry envEntry : envEntries) { - envEntry.addEnvEntry(variables, envBuilder); + envEntry.addEnvEntry(variables, envBuilder, pathMapper); } } @@ -830,11 +839,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); } } @@ -844,10 +854,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); } } @@ -1144,11 +1156,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); } } @@ -1348,59 +1361,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 50687774429e1b..12c4d9085e9b93 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 @@ -26,6 +26,7 @@ import com.google.common.collect.Sets.SetView; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.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; @@ -65,7 +66,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(); } @@ -80,7 +81,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 @@ -272,6 +274,7 @@ public interface Expandable { void expand( CcToolchainVariables variables, @Nullable ArtifactExpander expander, + PathMapper pathMapper, List commandLine) throws ExpansionException; } @@ -291,10 +294,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(); } @@ -410,18 +414,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. */ @@ -461,16 +468,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; /** @@ -495,8 +504,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 { @@ -533,7 +543,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, " @@ -542,8 +553,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, " @@ -972,7 +983,8 @@ private Sequence(ImmutableList values) { } @Override - public ImmutableList getSequenceValue(String variableName) { + public ImmutableList getSequenceValue( + String variableName, PathMapper pathMapper) { return values; } @@ -1026,7 +1038,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) { @@ -1086,7 +1099,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()); @@ -1133,12 +1147,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(); } @@ -1236,7 +1251,7 @@ private static final class StringValue extends VariableValueAdapter { } @Override - public String getStringValue(String variableName) { + public String getStringValue(String variableName, PathMapper pathMapper) { return value; } @@ -1283,7 +1298,7 @@ private static BooleanValue of(boolean value) { } @Override - public String getStringValue(String variableName) { + public String getStringValue(String variableName, PathMapper pathMapper) { return value ? "1" : "0"; } @@ -1314,8 +1329,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 b53c81bd43c33b..03c2689c7f677e 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 @@ -450,9 +450,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 81d62cd560990c..c5e4708f8102e7 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 @@ -21,6 +21,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; @@ -62,9 +63,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()); } @@ -85,7 +87,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<>(); @@ -96,7 +100,7 @@ List getArguments( if (parameterFilePath != null) { commandLine.add("@" + parameterFilePath.getSafePathString()); } else { - commandLine.addAll(getCompilerOptions(overwrittenVariables)); + commandLine.addAll(getCompilerOptions(overwrittenVariables, pathMapper)); } return commandLine; } @@ -113,13 +117,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<>(); @@ -131,7 +136,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) { @@ -172,15 +178,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 d1be207862b3d9..c26dd865cc8ef7 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 @@ -51,6 +51,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; @@ -58,7 +59,9 @@ 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.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 +675,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 +696,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 +860,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 +878,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 +896,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 +931,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 +967,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 @@ -1244,7 +1257,7 @@ public void computeKey( actionKeyContext, fp, getEnvironment(), - compileCommandLine.getEnvironment(), + compileCommandLine.getEnvironment(PathMapper.NOOP), executionInfo, getCommandLineKey(), ccCompilationContext.getDeclaredIncludeSrcs(), @@ -1252,7 +1265,9 @@ public void computeKey( mandatorySpawnInputs, additionalPrunableHeaders, builtInIncludeDirectories, - ccCompilationContext.getTransitiveCompilationPrerequisites()); + ccCompilationContext.getTransitiveCompilationPrerequisites(), + getMnemonic(), + PathMappers.getOutputPathsMode(configuration)); } // Separated into a helper method so that it can be called from CppCompileActionTemplate. @@ -1268,7 +1283,9 @@ static void computeKey( NestedSet mandatorySpawnInputs, NestedSet prunableHeaders, List builtInIncludeDirectories, - NestedSet inputsForInvalidation) + NestedSet inputsForInvalidation, + String mnemonic, + OutputPathsMode outputPathsMode) throws CommandLineExpansionException, InterruptedException { fp.addUUID(GUID); env.addTo(fp); @@ -1294,6 +1311,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 { @@ -1319,12 +1338,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); @@ -1360,7 +1381,10 @@ public ActionResult execute(ActionExecutionContext actionExecutionContext) Spawn spawn; try { spawn = - createSpawn(actionExecutionContext.getExecRoot(), actionExecutionContext.getClientEnv()); + createSpawn( + actionExecutionContext.getExecRoot(), + actionExecutionContext.getClientEnv(), + pathMapper); } finally { clearAdditionalInputs(); } @@ -1409,7 +1433,8 @@ public ActionResult execute(ActionExecutionContext actionExecutionContext) scanningContext.getArtifactResolver(), showIncludesFilterForStdout, showIncludesFilterForStderr, - siblingRepositoryLayout); + siblingRepositoryLayout, + pathMapper); updateActionInputs(discoveredInputs); validateInclusions(actionExecutionContext, discoveredInputs); return ActionResult.create(spawnResults); @@ -1427,7 +1452,8 @@ public ActionResult execute(ActionExecutionContext actionExecutionContext) execRoot, scanningContext.getArtifactResolver(), dotDContents, - siblingRepositoryLayout); + siblingRepositoryLayout, + pathMapper); dotDContents = null; // Garbage collect in-memory .d contents. updateActionInputs(discoveredInputs); @@ -1493,7 +1519,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 = @@ -1552,8 +1579,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(), /* filesetMappings= */ ImmutableMap.of(), inputs, @@ -1565,7 +1592,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( @@ -1581,7 +1609,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); @@ -1613,7 +1642,8 @@ private NestedSet discoverInputsFromShowIncludesFilters( getAllowedDerivedInputs(), execRoot, artifactResolver, - siblingRepositoryLayout); + siblingRepositoryLayout, + pathMapper); } @VisibleForTesting @@ -1622,7 +1652,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( @@ -1634,7 +1665,8 @@ public NestedSet discoverInputsFromDotdFiles( getAllowedDerivedInputs(), execRoot, artifactResolver, - siblingRepositoryLayout); + siblingRepositoryLayout, + pathMapper); } private DependencySet processDepset( @@ -1782,7 +1814,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 ce1e1fc9b0ce03..67cb06bb597d19 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.ArtifactExpander; 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; @@ -203,16 +205,20 @@ 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, cppCompileActionBuilder.getPrunableHeaders(), cppCompileActionBuilder.getBuiltinIncludeDirectories(), - cppCompileActionBuilder.getInputsForInvalidation()); + cppCompileActionBuilder.getInputsForInvalidation(), + getMnemonic(), + // TODO(fmeum): Replace this with the actual value once CppCompileActionTemplate supports + // path mapping. + OutputPathsMode.OFF); } catch (EvalException e) { throw new CommandLineExpansionException(e.getMessage()); } 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 2a264e7d4620c3..8c13fc6512611c 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 @@ -22,6 +22,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.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; @@ -290,7 +291,7 @@ public static ImmutableMap getEnvironmentVariables( FeatureConfiguration featureConfiguration, CcToolchainVariables variables, String actionName) throws EvalException { try { - return featureConfiguration.getEnvironmentVariables(actionName, variables); + return featureConfiguration.getEnvironmentVariables(actionName, variables, PathMapper.NOOP); } catch (ExpansionException e) { throw new EvalException(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/LinkCommandLine.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java index 6c12010604738a..d72f6f64cbd3c6 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 @@ -103,13 +103,16 @@ public ImmutableList getParamCommandLine(@Nullable ArtifactExpander expa 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()); @@ -125,7 +128,7 @@ CommandLines getCommandLines() throws EvalException { if (splitCommandLine) { try { Optional formatString = - featureConfiguration.getCommandLine(actionName, variables, null).stream() + featureConfiguration.getCommandLine(actionName, variables, null, PathMapper.NOOP).stream() .filter(s -> s.contains("LINKER_PARAM_FILE_PLACEHOLDER")) .findAny(); if (formatString.isPresent()) { 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 6e156062b45522..3cd23e6db19308 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 @@ -326,7 +326,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 89b4b508939490..80f4afc61f5e6b 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.FilesetOutputSymlink; import com.google.devtools.build.lib.actions.PathMapper; import com.google.devtools.build.lib.actions.ResourceSet; @@ -63,27 +62,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); } @@ -99,18 +77,17 @@ public Spawn build() { platform, execProperties, builtForToolConfiguration); - return new PathStrippableSpawn( - new SimpleSpawn( - owner, - ImmutableList.copyOf(args), - ImmutableMap.copyOf(environment), - ImmutableMap.copyOf(executionInfo), - 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), + ImmutableMap.copyOf(filesetMappings), + inputs.build(), + tools.build(), + ImmutableSet.copyOf(outputs), + mandatoryOutputs, + resourceSet, pathMapper); } 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 2245f20d8634c9..b321b557cf3056 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 @@ -390,6 +390,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 197c03ae354d44..b3d680a5582b6d 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 223fa0529a8b64..9c5855e616329d 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; @@ -60,9 +61,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"); } @@ -77,7 +82,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(); @@ -94,7 +99,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"); } @@ -111,7 +116,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(); } @@ -130,7 +135,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"); @@ -149,7 +154,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"); } @@ -167,7 +172,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"); } @@ -189,7 +194,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"); } @@ -209,7 +214,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(); } @@ -228,7 +234,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(); } @@ -299,7 +306,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(); } @@ -314,7 +322,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"); } @@ -387,7 +396,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/HeaderDiscoveryTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscoveryTest.java index d1c2c0aeb4e225..dadb6a0c3327e6 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; @@ -77,7 +78,8 @@ private void checkHeaderInclusion( includedHeaders, execRoot, artifactResolver, - /*siblingRepositoryLayout=*/ false); + /* 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 c99730e7f2ae34..c89a1edf097b8c 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; @@ -109,7 +110,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 @@ -118,7 +119,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 @@ -127,7 +128,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( @@ -149,7 +150,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 @@ -158,7 +159,7 @@ public void getFieldValue_forVersionedDynamicLibrary() throws Exception { /* field= */ "path", /* expander= */ null, /* throwOnMissingVariable= */ false) - .getStringValue("variable name doesn't matter")) + .getStringValue("variable name doesn't matter", PathMapper.NOOP)) .isEqualTo("foo/bar.so"); assertThat( libraryToLinkValue @@ -167,7 +168,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 @@ -176,7 +177,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( @@ -197,7 +198,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 @@ -206,7 +207,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 @@ -215,7 +216,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( @@ -237,7 +238,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 @@ -246,7 +247,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 @@ -255,7 +256,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( @@ -277,7 +278,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 @@ -286,7 +287,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 @@ -295,7 +296,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( @@ -317,7 +318,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 @@ -326,7 +327,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 @@ -335,7 +336,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( @@ -357,7 +358,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 @@ -366,7 +367,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 @@ -375,7 +376,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( @@ -405,7 +406,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 @@ -414,7 +415,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( @@ -431,8 +432,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"); } @@ -455,7 +456,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 @@ -464,7 +465,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( @@ -481,8 +482,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 c2786154a4877e..ababd1fe4546c7 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.actions.SpawnAction; @@ -95,7 +96,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 @@ -105,7 +106,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"); } @@ -140,7 +141,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() @@ -149,7 +150,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"); } @@ -167,7 +168,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() @@ -176,7 +177,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"); } @@ -194,7 +195,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() @@ -203,7 +204,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"); } @@ -631,7 +632,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(); } @@ -660,7 +661,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 fd83cf62427657..fff46c89c7959d 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; @@ -587,7 +588,7 @@ public void testGetEnvironment() throws Exception { assertThat(environmentVariables) .containsExactlyEntriesIn( featureConfiguration.getEnvironmentVariables( - CppActionNames.CPP_COMPILE, CcToolchainVariables.EMPTY)); + CppActionNames.CPP_COMPILE, CcToolchainVariables.EMPTY, PathMapper.NOOP)); } @Test @@ -7936,9 +7937,18 @@ 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"); + action + .getCompileCommandLine() + .getVariables() + .getSequenceVariable("string_sequence_variable", PathMapper.NOOP); + action + .getCompileCommandLine() + .getVariables() + .getStringVariable("string_variable", PathMapper.NOOP); + action + .getCompileCommandLine() + .getVariables() + .getSequenceVariable("string_depset_variable", PathMapper.NOOP); } @Test @@ -7949,9 +7959,15 @@ public void testVariableExtensionLinkingContextApi() throws Exception { SpawnAction action = (SpawnAction) getGeneratingAction(artifactByPath(getFilesToBuild(target), ".a")); - getLinkCommandLine(action).getBuildVariables().getSequenceVariable("string_sequence_variable"); - getLinkCommandLine(action).getBuildVariables().getStringVariable("string_variable"); - getLinkCommandLine(action).getBuildVariables().getSequenceVariable("string_depset_variable"); + getLinkCommandLine(action) + .getBuildVariables() + .getSequenceVariable("string_sequence_variable", PathMapper.NOOP); + getLinkCommandLine(action) + .getBuildVariables() + .getStringVariable("string_variable", PathMapper.NOOP); + getLinkCommandLine(action) + .getBuildVariables() + .getSequenceVariable("string_depset_variable", PathMapper.NOOP); } @Test @@ -7964,9 +7980,15 @@ public void testVariableExtensionLinkApi() throws Exception { (SpawnAction) getGeneratingAction((Artifact) getMyInfoFromTarget(target).getValue("executable")); - getLinkCommandLine(action).getBuildVariables().getSequenceVariable("string_sequence_variable"); - getLinkCommandLine(action).getBuildVariables().getStringVariable("string_variable"); - getLinkCommandLine(action).getBuildVariables().getSequenceVariable("string_depset_variable"); + getLinkCommandLine(action) + .getBuildVariables() + .getSequenceVariable("string_sequence_variable", PathMapper.NOOP); + getLinkCommandLine(action) + .getBuildVariables() + .getStringVariable("string_variable", PathMapper.NOOP); + getLinkCommandLine(action) + .getBuildVariables() + .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 dca2fef1817eaa..400f00741b791a 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; @@ -1483,7 +1484,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"); } @@ -2339,7 +2345,7 @@ public void testCoptsLocationIsExpanded() throws Exception { "--platforms=" + MockObjcSupport.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..3bca3b15dfa7bd 100755 --- a/src/test/shell/bazel/path_mapping_test.sh +++ b/src/test/shell/bazel/path_mapping_test.sh @@ -325,4 +325,223 @@ 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} \ + "//$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} \ + "//$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_test.sh b/src/test/shell/integration/config_stripped_outputs_test.sh index da111c4ec0eab5..731c2435c1e2a1 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="${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"