Skip to content

Commit

Permalink
Replace more mentions of the term "middleman" with "runfiles tree".
Browse files Browse the repository at this point in the history
This removes all obviously-replaceable ones; what's left requires thinking (at least for me)

RELNOTES: None.
PiperOrigin-RevId: 6960009
Change-Id: Ic6ae3f11220d4457d63664180c5c4c190edbee58
  • Loading branch information
lberki authored and copybara-github committed Nov 13, 2024
1 parent 0f9d547 commit c1cc547
Show file tree
Hide file tree
Showing 31 changed files with 82 additions and 166 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,8 @@ public enum RootType {
ExternalSource,
Output,
// Sibling root types are in effect when --experimental_sibling_repository_layout is activated.
// These will eventually replace the above Output and Middleman types when the flag becomes
// the default option and then removed.
// These will eventually replace the above Output types when the flag becomes the default option
// and then removed.
SiblingMainOutput,
SiblingExternalOutput,
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,10 @@
/**
* A value that represents a file for the purposes of up-to-dateness checks of actions.
*
* <p>It always stands for an actual file. In particular, tree artifacts and middlemen do not have a
* corresponding {@link FileArtifactValue}. However, the file is not necessarily present in the file
* system; this happens when intermediate build outputs are not downloaded (and maybe when an input
* artifact of an action is missing?)
* <p>It always stands for an actual file. In particular, tree artifacts and runfiles trees do not
* have a corresponding {@link FileArtifactValue}. However, the file is not necessarily present in
* the file system; this happens when intermediate build outputs are not downloaded (and maybe when
* an input artifact of an action is missing?)
*
* <p>It makes its main appearance in {@code ActionExecutionValue.artifactData}. It has two main
* uses:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,8 @@ public interface Spawn extends DescribableExecutionUnit {
/**
* Returns the list of files that this command may read.
*
* <p>This method explicitly does not expand middleman artifacts. Pass the result to an
* appropriate utility method on {@link com.google.devtools.build.lib.actions.Artifact} to expand
* the middlemen.
* <p>This method explicitly does not expand runfiles trees. Pass the result to an appropriate
* utility method on {@link com.google.devtools.build.lib.actions.Artifact} to expand them.
*
* <p>This is for use with remote execution, so we can ship inputs before starting the command.
* Order stability across multiple calls should be upheld for performance reasons.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ private Set<String> getPaths(
TreeSet<String> paths = Sets.newTreeSet();
for (Artifact artifact : artifacts) {
PathFragment path = getPath(artifact, workspaceRunfilesDirectory);
if (path != null) { // omit middlemen etc
if (path != null) {
paths.add(path.getCallablePathString());
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -406,8 +406,8 @@ public static void collectTransitiveValidationOutputGroups(

/**
* Compute the artifacts to put into the {@link FilesToRunProvider} for this target. These are the
* filesToBuild, any artifacts added by the rule with {@link #addFilesToRun}, and the runfiles'
* middlemen if they exists.
* filesToBuild, any artifacts added by the rule with {@link #addFilesToRun}, and the runfiles
* tree of the rule if it exists.
*/
private NestedSet<Artifact> buildFilesToRun(
NestedSet<Artifact> runfilesTrees, NestedSet<Artifact> filesToBuild) {
Expand Down Expand Up @@ -483,7 +483,7 @@ private TestProvider initializeTestProvider(FilesToRunProvider filesToRunProvide

/**
* Add files required to run the target. Artifacts from {@link #setFilesToBuild} and the runfiles
* middleman, if any, are added automatically.
* tree, if any, are added automatically.
*/
@CanIgnoreReturnValue
public RuleConfiguredTargetBuilder addFilesToRun(NestedSet<Artifact> files) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,10 @@ public void fingerprint(Fingerprint fp) {
/**
* The artifacts that should be present in the runfiles directory.
*
* <p>This collection may not include any middlemen. These artifacts will be placed at a location
* that corresponds to the output-dir-relative path of each artifact. It's possible for several
* artifacts to have the same output-dir-relative path, in which case the last one will win.
* <p>This collection may not include any runfiles trees. These artifacts will be placed at a
* location that corresponds to the output-dir-relative path of each artifact. It's possible for
* several artifacts to have the same output-dir-relative path, in which case the last one will
* win.
*/
private final NestedSet<Artifact> artifacts;

Expand Down Expand Up @@ -619,7 +620,7 @@ public void put(Map<PathFragment, Artifact> map, PathFragment path, Artifact art
return;
}
// TODO(b/304440811): Check if this assertion is reached.
// test_fail_on_middleman_in_transitive_runfiles_for_executable with the renaming halfway
// test_fail_on_runfiles_tree_in_transitive_runfiles_for_executable with the renaming halfway
// done indicated that it is; check the exit code in that test.
Preconditions.checkArgument(artifact == null || !artifact.isRunfilesTree(), "%s", artifact);
if (policy != ConflictPolicy.IGNORE && map.containsKey(path)) {
Expand Down Expand Up @@ -715,7 +716,7 @@ public Runfiles build() {
public Builder addArtifact(Artifact artifact) {
Preconditions.checkNotNull(artifact);
Preconditions.checkArgument(
!artifact.isRunfilesTree(), "unexpected middleman artifact: %s", artifact);
!artifact.isRunfilesTree(), "unexpected runfiles tree artifact: %s", artifact);
artifactsBuilder.add(artifact);
return this;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ private static CoverageReportAction generateCoverageReportAction(
coverageDir.getRelative("_coverage_report.dat"), root, args.artifactOwner());
Artifact reportGeneratorExec = args.reportGenerator().getExecutable();
RunfilesSupport runfilesSupport = args.reportGenerator().getRunfilesSupport();
Artifact runfilesMiddleman =
Artifact runfilesTree =
runfilesSupport != null ? runfilesSupport.getRunfilesTreeArtifact() : null;
args = CoverageArgs.createCopyWithCoverageDirAndLcovOutput(args, coverageDir, lcovOutput);
ImmutableList<String> actionArgs = argsFunc.apply(args);
Expand All @@ -287,8 +287,8 @@ private static CoverageReportAction generateCoverageReportAction(
.addAll(args.coverageArtifacts())
.add(reportGeneratorExec)
.add(args.lcovArtifact());
if (runfilesMiddleman != null) {
inputsBuilder.add(runfilesMiddleman);
if (runfilesTree != null) {
inputsBuilder.add(runfilesTree);
}
return new CoverageReportAction(
ACTION_OWNER,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ private static ArrayList<Artifact> getArtifactsToPrint(
ProviderCollection target, TopLevelArtifactContext context) {
var artifacts = new ArrayList<Artifact>();
// For up-to-date targets report generated artifacts, but only if they have associated action
// and not middleman artifacts.
// and not runfiles trees.
for (Artifact artifact :
TopLevelArtifactHelper.getAllArtifactsToBuild(target, context)
.getImportantArtifacts()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -723,12 +723,9 @@ public static class Builder {
private Builder() {}

/**
* Overrides the purpose of this context. This is useful if a Target needs more than one
* CcCompilationContext. (The purpose is used to construct the name of the prerequisites
* middleman for the context, and all artifacts for a given Target must have distinct names.)
* Overrides the purpose of this context.
*
* @param purpose must be a string which is suitable for use as a filename. A single rule may
* have many middlemen with distinct purposes.
* @param a string suitable for use as a filename.
*/
@CanIgnoreReturnValue
public Builder setPurpose(String purpose) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -712,8 +712,8 @@ public CcCompilationContext createCcCompilationContext(
Sequence<?> directPrivateHdrs,
Object purposeNoneable,
Object moduleMap,
Object actionFactoryForMiddlemanOwnerAndConfiguration,
Object labelForMiddlemanNameObject,
Object unused1,
Object unused2,
Object externalIncludes,
Object virtualToOriginalHeaders,
Sequence<?> dependentCcCompilationContexts,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -269,8 +269,6 @@ public class CppCompileAction extends AbstractAction implements IncludeScannable
this.sourceFile = sourceFile;
this.shareable = shareable;
this.configuration = configuration;
// We do not need to include the middleman artifact since it is a generated artifact and will
// definitely exist prior to this action execution.
this.mandatoryInputs = mandatoryInputs;
this.mandatorySpawnInputs = mandatorySpawnInputs;
this.additionalPrunableHeaders = additionalPrunableHeaders;
Expand Down Expand Up @@ -1104,7 +1102,7 @@ void verifyActionIncludePaths(
}

/**
* Recalculates this action's live input collection, including sources, middlemen.
* Recalculates this action's live input collection.
*
* <p>Can only be called if {@link #discoversInputs}, and must be called after execution in that
* case.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -584,7 +584,7 @@ private void maybeAddEdges(
// for Spawns and Actions and the runfiles on a Spawn can be a subset of the runfiles of the
// action during whose execution it was created.
if ((input instanceof Artifact) && ((Artifact) input).isRunfilesTree()) {
// This is a runfiles middleman. Collect the artifacts in it into
// This is a runfiles tree. Collect the artifacts in it into
// runfilesArtifactsBuilder.
RunfilesTree runfilesTree =
inputMetadataProvider.getRunfilesMetadata(input).getRunfilesTree();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -579,8 +579,8 @@ private SkyFunction.Reset handleLostInputs(
* ownership information from {@code inputDeps}.
*
* <p>This compensates for how the ownership information in {@code e.getOwners()} is potentially
* incomplete. E.g., it may lack knowledge of a runfiles middleman owning a fileset, even if it
* knows that fileset owns a lost input.
* incomplete. E.g., it may lack knowledge of a runfiles tree owning a fileset, even if it knows
* that fileset owns a lost input.
*/
private ActionInputDepOwners createAugmentedInputDepOwners(
LostInputsActionExecutionException e,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ public FileArtifactValue getOutputMetadata(ActionInput actionInput)
}

if (artifact.isRunfilesTree()) {
// A middleman artifact's data has the default middleman value.
// Runfiles trees get a placeholder value, see the Javadoc of RUNFILES_TREE_MARKER as to why
value = artifactData.get(artifact);
if (value != null) {
return checkExists(value, artifact);
Expand Down Expand Up @@ -478,8 +478,8 @@ private FileArtifactValue constructFileArtifactValue(
}

/**
* Constructs a {@link FileArtifactValue} for a regular (non-tree, non-middleman) artifact for the
* purpose of determining whether an existing {@link FileArtifactValue} is still valid.
* Constructs a {@link FileArtifactValue} for a regular (non-tree, non-runfiles tree) artifact for
* the purpose of determining whether an existing {@link FileArtifactValue} is still valid.
*
* <p>The returned metadata may be compared with metadata present in an {@link
* ActionExecutionValue} using {@link FileArtifactValue#couldBeModifiedSince} to check for
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,9 @@

/**
* A builder of values for {@link Artifact} keys when the key is not a simple generated artifact. To
* save memory, ordinary generated artifacts (non-middleman, non-tree) have their metadata accessed
* save memory, ordinary generated artifacts (non-runfiles, non-tree) have their metadata accessed
* directly from the corresponding {@link ActionExecutionValue}. This SkyFunction is therefore only
* usable for source, middleman, and tree artifacts.
* usable for source, runfiles trees and tree artifacts.
*/
public final class ArtifactFunction implements SkyFunction {

Expand Down Expand Up @@ -188,7 +188,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
Action action =
Preconditions.checkNotNull(
artifactDependencies.actionLookupValue.getAction(generatingActionKey.getActionIndex()),
"Null middleman action? %s",
"Null runfiles tree action? %s",
artifactDependencies);

return createRunfilesArtifactValue(artifact, (RunfilesTreeAction) action, env);
Expand Down Expand Up @@ -410,7 +410,7 @@ private static RunfilesArtifactValue createRunfilesArtifactValue(
trees.add(input);
treeValues.add((TreeArtifactValue) inputValue);
} else {
// We do not recurse in middleman artifacts.
// We do not recurse into runfiles tree artifacts.
Preconditions.checkState(
!(inputValue instanceof RunfilesArtifactValue),
"%s %s %s",
Expand Down Expand Up @@ -492,8 +492,6 @@ private static String constructErrorMessage(Artifact artifact, String error) {
}

/** Describes dependencies of derived artifacts. */
// TODO(b/19539699): extend this to comprehensively support all special artifact types (e.g.
// middleman, etc).
public static final class ArtifactDependencies {
private final DerivedArtifact artifact;
private final ActionLookupValue actionLookupValue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1750,8 +1750,8 @@ CompilationContextT createCcCompilationContext(
Sequence<?> directPrivateHdrs,
Object purpose,
Object moduleMap,
Object actionFactoryForMiddlemanOwnerAndConfiguration,
Object labelForMiddlemanNameObject,
Object unused1,
Object unused2,
Object externalIncludes,
Object virtualToOriginalHeaders,
Sequence<?> dependentCcCompilationContexts,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -452,8 +452,8 @@ public String getMnemonic() {
}

/**
* A mocked action containing the inputs and outputs of the action and determines whether or not
* the action is a middleman. Used for tests that do not need to execute the action.
* A mocked action containing the inputs and outputs of the action. Used for tests that do not
* need to execute the action.
*/
public static class MockAction extends AbstractAction {
private final boolean isShareable;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ public String getMnemonic() {
return "Test";
}

/** No-op action that has exactly one output, and can be a middleman action. */
/** No-op action that has exactly one output. */
@AutoCodec
public static class DummyAction extends TestAction {
@AutoCodec.Instantiator
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,7 @@
/** Tests to verify that Bazel actions don't poison any output cache. */
@RunWith(JUnit4.class)
public class CachingTest extends BuildViewTestCase {
/**
* Regression test for bugs #2317593 and #2284024: Don't expand runfile middlemen.
*/
/** Regression test for bugs #2317593 and #2284024 */
@Test
public void testRunfilesManifestNotAnInput() throws Exception {
scratch.file(
Expand All @@ -59,22 +57,22 @@ public void testRunfilesManifestNotAnInput() throws Exception {
}

boolean lookedAtAnyAction = false;
boolean foundRunfilesMiddlemanSoRunfilesAreCorrectlyStaged = false;
boolean foundRunfilesTreeSoRunfilesAreCorrectlyStaged = false;
for (Action action : actions) {
if (action instanceof SpawnAction) {
for (ActionInput string :
((SpawnAction) action).getSpawnForTesting().getInputFiles().toList()) {
lookedAtAnyAction = true;
if (string.getExecPathString().endsWith("tool.runfiles")
|| string.getExecPathString().endsWith("tool.exe.runfiles")) {
foundRunfilesMiddlemanSoRunfilesAreCorrectlyStaged = true;
foundRunfilesTreeSoRunfilesAreCorrectlyStaged = true;
} else {
assertThat(string.getExecPathString().endsWith(".runfiles/MANIFEST")).isFalse();
}
}
}
}
assertThat(lookedAtAnyAction).isTrue();
assertThat(foundRunfilesMiddlemanSoRunfilesAreCorrectlyStaged).isTrue();
assertThat(foundRunfilesTreeSoRunfilesAreCorrectlyStaged).isTrue();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1000,8 +1000,9 @@ protected final Action getGeneratingAction(Artifact artifact) {
}

protected RunfilesTree runfilesTreeFor(TestRunnerAction testRunnerAction) throws Exception {
Artifact middleman = testRunnerAction.getRunfilesTree();
RunfilesTreeAction runfilesTreeAction = (RunfilesTreeAction) getGeneratingAction(middleman);
Artifact runfilesTreeArtifact = testRunnerAction.getRunfilesTree();
RunfilesTreeAction runfilesTreeAction =
(RunfilesTreeAction) getGeneratingAction(runfilesTreeArtifact);
return runfilesTreeAction.getRunfilesTree();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ public void testKeepGoingAfterTargetParsingErrors() throws Exception {
}

@Test
public void testKeepGoingAfterSchedulingDependencyMiddlemanFailure() throws Exception {
public void testKeepGoingAfterSchedulingDependencyFailure() throws Exception {
write("foo/foo.cc", "int main() { return 0; }");
write(
"foo/BUILD",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -246,8 +246,7 @@ public void testDiscardAnalysisCache() throws Exception {
)
""");
buildTarget("//sh:sh");
// We test with dep because target completion middleman actions keep references to the
// top-level configured targets.
// We test with dep because we may keep references to the top-level configured targets.
ConfiguredTarget ct = getConfiguredTarget("//sh:dep");
addOptions("--discard_analysis_cache");
buildTarget("//sh:sh");
Expand Down
Loading

0 comments on commit c1cc547

Please sign in to comment.