Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[7.4.0] Support layering_check with C++ path mapping #23671

Merged
merged 1 commit into from
Sep 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -581,8 +581,8 @@ void addAdditionalInputs(NestedSetBuilder<Artifact> builder) {
}
}

/** @return modules maps from direct dependencies. */
public Iterable<Artifact> getDirectModuleMaps() {
/** Returns modules maps from direct dependencies. */
public ImmutableList<Artifact> getDirectModuleMaps() {
return directModuleMaps;
}

Expand All @@ -595,8 +595,8 @@ DerivedArtifact getSeparateHeaderModule(boolean usePic) {
}

/**
* @return all declared headers of the current module if the current target is compiled as a
* module.
* Returns all declared headers of the current module if the current target is compiled as a
* module.
*/
ImmutableList<Artifact> getHeaderModuleSrcs(boolean separateModule) {
if (separateModule) {
Expand Down Expand Up @@ -651,7 +651,7 @@ public static CcCompilationContext createWithExtraHeaderTokens(
headerTokens.build());
}

/** @return the C++ module map of the owner. */
/** Returns the C++ module map of the owner. */
public CppModuleMap getCppModuleMap() {
return cppModuleMap;
}
Expand Down Expand Up @@ -1154,7 +1154,7 @@ public ImmutableList<E> getMergedResult() {
* Gathers data about the PIC and no-PIC .pcm files belonging to this context and the associated
* information about the headers, e.g. modular vs. textual headers and pre-grepped header files.
*
* <p>This also implements a data structure very similar to NestedSet, but chosing slightly
* <p>This also implements a data structure very similar to NestedSet, but choosing slightly
* different trade-offs to account for the specific data stored in here, specifically, we know
* that there is going to be a single entry in every node of the DAG. Contrary to NestedSet, we
* reuse memoization data from dependencies to conserve both runtime and memory. Experiments have
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@
import com.google.devtools.build.docgen.annot.DocCategory;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.CommandLineExpansionException;
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.analysis.actions.ActionConstructionContext;
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.starlark.StarlarkActionFactory;
import com.google.devtools.build.lib.analysis.starlark.StarlarkRuleContext;
Expand Down Expand Up @@ -610,6 +612,7 @@ public void createModuleMapAction(
Boolean generateSubmodules,
Boolean withoutExternDependencies)
throws EvalException {
RuleContext ruleContext = actions.getRuleContext();
ActionConstructionContext actionConstructionContext = actions.getActionConstructionContext();
actions
.asActionRegistry(actions)
Expand All @@ -629,6 +632,10 @@ public void createModuleMapAction(
compiledModule,
moduleMapHomeIsCwd,
generateSubmodules,
withoutExternDependencies));
withoutExternDependencies,
PathMappers.getOutputPathsMode(ruleContext.getConfiguration()),
ruleContext
.getConfiguration()
.modifiedExecutionInfo(ImmutableMap.of(), CppModuleMapAction.MNEMONIC)));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1159,6 +1159,54 @@ public int hashCode() {
}
}

@Immutable
private static final class ArtifactSetSequence extends VariableValueAdapter {
private final NestedSet<Artifact> values;

private ArtifactSetSequence(NestedSet<Artifact> values) {
Preconditions.checkNotNull(values);
this.values = values;
}

@Override
public ImmutableList<VariableValue> getSequenceValue(
String variableName, PathMapper pathMapper) {
ImmutableList<Artifact> valuesList = values.toList();
ImmutableList.Builder<VariableValue> sequences =
ImmutableList.builderWithExpectedSize(valuesList.size());
for (Artifact value : valuesList) {
sequences.add(new StringValue(pathMapper.getMappedExecPathString(value)));
}
return sequences.build();
}

@Override
public String getVariableTypeName() {
return Sequence.SEQUENCE_VARIABLE_TYPE_NAME;
}

@Override
public boolean isTruthy() {
return !values.isEmpty();
}

@Override
public boolean equals(Object other) {
if (this == other) {
return true;
}
if (!(other instanceof ArtifactSetSequence otherArtifacts)) {
return false;
}
return values.shallowEquals(otherArtifacts.values);
}

@Override
public int hashCode() {
return values.shallowHashCode();
}
}

/**
* Single structure value. Be careful not to create sequences of single structures, as the memory
* overhead is prohibitively big.
Expand Down Expand Up @@ -1470,6 +1518,19 @@ public Builder addPathFragmentSequenceVariable(String name, NestedSet<PathFragme
return this;
}

/**
* Add a sequence variable that expands {@code name} to {@link Artifact} {@code values}.
*
* <p>Accepts values as NestedSet. Nested set is stored directly, not cloned, not flattened.
*/
@CanIgnoreReturnValue
public Builder addArtifactSequenceVariable(String name, NestedSet<Artifact> values) {
checkVariableNotPresentAlready(name);
Preconditions.checkNotNull(values, "Cannot set null as a value for variable '%s'", name);
variablesMap.put(name, new ArtifactSetSequence(values));
return this;
}

/**
* Add a variable built using {@code VariableValueBuilder} api that expands {@code name} to the
* value returned by the {@code builder}.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ public static CcToolchainVariables setupVariablesOrReportRuleError(
Artifact diagnosticsFile,
ImmutableList<VariablesExtension> variablesExtensions,
ImmutableMap<String, String> additionalBuildVariables,
Iterable<Artifact> directModuleMaps,
ImmutableList<Artifact> directModuleMaps,
NestedSet<PathFragment> includeDirs,
NestedSet<PathFragment> quoteIncludeDirs,
NestedSet<PathFragment> systemIncludeDirs,
Expand Down Expand Up @@ -237,7 +237,7 @@ public static CcToolchainVariables setupVariablesOrThrowEvalException(
Artifact diagnosticsFile,
ImmutableList<VariablesExtension> variablesExtensions,
ImmutableMap<String, String> additionalBuildVariables,
Iterable<Artifact> directModuleMaps,
ImmutableList<Artifact> directModuleMaps,
NestedSet<String> includeDirs,
NestedSet<String> quoteIncludeDirs,
NestedSet<String> systemIncludeDirs,
Expand Down Expand Up @@ -303,7 +303,7 @@ private static CcToolchainVariables setupVariables(
Artifact diagnosticsFile,
ImmutableList<VariablesExtension> variablesExtensions,
ImmutableMap<String, String> additionalBuildVariables,
Iterable<Artifact> directModuleMaps,
ImmutableList<Artifact> directModuleMaps,
NestedSet<PathFragment> includeDirs,
NestedSet<PathFragment> quoteIncludeDirs,
NestedSet<PathFragment> systemIncludeDirs,
Expand Down Expand Up @@ -475,7 +475,7 @@ public static void setupCommonVariables(
String fdoStamp,
List<VariablesExtension> variablesExtensions,
Map<String, String> additionalBuildVariables,
Iterable<Artifact> directModuleMaps,
ImmutableList<Artifact> directModuleMaps,
ImmutableList<PathFragment> includeDirs,
ImmutableList<PathFragment> quoteIncludeDirs,
ImmutableList<PathFragment> systemIncludeDirs,
Expand Down Expand Up @@ -509,7 +509,7 @@ private static void setupCommonVariablesInternal(
String fdoStamp,
List<VariablesExtension> variablesExtensions,
Map<String, String> additionalBuildVariables,
Iterable<Artifact> directModuleMaps,
ImmutableList<Artifact> directModuleMaps,
NestedSet<PathFragment> includeDirs,
NestedSet<PathFragment> quoteIncludeDirs,
NestedSet<PathFragment> systemIncludeDirs,
Expand All @@ -528,9 +528,9 @@ private static void setupCommonVariablesInternal(
buildVariables.addStringVariable(MODULE_NAME.getVariableName(), cppModuleMap.getName());
buildVariables.addArtifactVariable(
MODULE_MAP_FILE.getVariableName(), cppModuleMap.getArtifact());
buildVariables.addStringSequenceVariable(
buildVariables.addArtifactSequenceVariable(
DEPENDENT_MODULE_MAP_FILES.getVariableName(),
Iterables.transform(directModuleMaps, Artifact::getExecPathString));
NestedSetBuilder.wrap(Order.STABLE_ORDER, directModuleMaps));
}
if (featureConfiguration.isEnabled(CppRuleClasses.USE_HEADER_MODULES)) {
// Module inputs will be set later when the action is executed.
Expand Down
Loading
Loading