Skip to content

Commit

Permalink
Split createLocalOutput with convenience named symlink into its own m…
Browse files Browse the repository at this point in the history
…ethod

PiperOrigin-RevId: 691983613
Change-Id: I8801eab9c5faee440dd2a8b3384214352ecbec9f
  • Loading branch information
yuyue730 authored and copybara-github committed Nov 1, 2024
1 parent aa7317f commit 796ed0e
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -855,7 +855,6 @@ private ExplanationHandler installExplanationHandler(
getWorkspace().getRelative(explanationPath),
env,
getReporter(),
/* convenienceName= */ null,
/* append= */ null,
/* internal= */ null);
handler = new ExplanationHandler(instrumentationOutput.createOutputStream(), allOptions);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -367,21 +367,24 @@ ProfilerStartedEvent initProfiler(
profile =
instrumentationOutputFactory.createBuildEventArtifactInstrumentationOutput(
profileName, newUploader(env, bepOptions.buildEventUploadStrategy));
} else {
var profilePath =
manageProfiles(
workspace.getOutputBase(),
env.getCommandId().toString(),
commandOptions.profilesToRetain);
} else if (commandOptions.redirectLocalInstrumentationOutputWrites) {
profile =
instrumentationOutputFactory.createInstrumentationOutput(
profileName,
profilePath,
workspace.getOutputBase().getRelative(profileName),
env,
eventHandler,
/* convenienceName= */ profileName,
/* append= */ null,
/* internal= */ null);
} else {
var profilePath =
manageProfiles(
workspace.getOutputBase(),
env.getCommandId().toString(),
commandOptions.profilesToRetain);
profile =
instrumentationOutputFactory.createLocalOutputWithConvenientName(
profileName, profilePath, /* convenienceName= */ profileName);
}
} else {
format =
Expand All @@ -397,7 +400,6 @@ ProfilerStartedEvent initProfiler(
profilePath,
env,
eventHandler,
/* convenienceName= */ null,
/* append= */ false,
/* internal= */ true);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,26 @@ private InstrumentationOutputFactory(
redirectInstrumentationOutputBuilderSupplier;
}

/**
* Creates a {@link LocalInstrumentationOutput} located at {@code path}, which could future call
* {@link LocalInstrumentationOutput#makeConvenienceLink()} to make a symlink with the simplified
* {@code convenienceName} pointing to the local output. The symlink locates under the same
* directory as the output.
*
* <p>Should only be used when an output MUST be written locally or is otherwise incompatible with
* the flexible destinations supported by the preferred generic {@link
* #createInstrumentationOutput}.
*/
public LocalInstrumentationOutput createLocalOutputWithConvenientName(
String name, Path path, String convenienceName) {
return localInstrumentationOutputBuilderSupplier
.get()
.setName(name)
.setPath(path)
.setConvenienceName(convenienceName)
.build();
}

/**
* Creates {@link LocalInstrumentationOutput} or an {@link InstrumentationOutput} object
* redirecting outputs to be written on a different machine.
Expand All @@ -55,24 +75,14 @@ private InstrumentationOutputFactory(
* --redirect_local_instrumentation_output_writes} is set, this method will default to return
* {@link LocalInstrumentationOutput}.
*
* <p>For {@link LocalInstrumentationOutput}, there are two additional considerations:
*
* <ul>
* <li>When the name of the instrumentation output is complicated, an optional {@code
* convenienceName} parameter can be passed in so that a symlink pointing to the output with
* such a simpler name is created. See {@link
* LocalInstrumentationOutput.Builder#setConvenienceName}.
* <li>User can also pass in the optional {@code append} and {@code internal} {@code Boolean}s
* to control how {@code path} creates the {@link OutputStream}. See {@link
* LocalInstrumentationOutput#createOutputStream} for more details.
* </ul>
* @param append Whether to open the {@link LocalInstrumentationOutput} file in append mode
* @param internal Whether the {@link LocalInstrumentationOutput} file is a Bazel internal file.
*/
public InstrumentationOutput createInstrumentationOutput(
String name,
Path path,
CommandEnvironment env,
EventHandler eventHandler,
@Nullable String convenienceName,
@Nullable Boolean append,
@Nullable Boolean internal) {
boolean isRedirect =
Expand All @@ -97,7 +107,6 @@ public InstrumentationOutput createInstrumentationOutput(
.get()
.setName(name)
.setPath(path)
.setConvenienceName(convenienceName)
.setAppend(append)
.setInternal(internal)
.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,29 @@ public void testInstrumentationOutputFactory_cannotCreateFactorIfBepSupplierUnse
factoryBuilder::build);
}

@Test
public void testInstrumentationOutputFactory_successfullyCreateLocalOutputWithConvenientLink()
throws Exception {
InstrumentationOutputFactory.Builder factoryBuilder =
new InstrumentationOutputFactory.Builder();
factoryBuilder.setLocalInstrumentationOutputBuilderSupplier(
LocalInstrumentationOutput.Builder::new);
factoryBuilder.setBuildEventArtifactInstrumentationOutputBuilderSupplier(
BuildEventArtifactInstrumentationOutput.Builder::new);
InstrumentationOutputFactory outputFactory = factoryBuilder.build();

CommandEnvironment env = runtimeWrapper.newCommand();
InstrumentationOutput output =
outputFactory.createLocalOutputWithConvenientName(
/* name= */ "output",
env.getWorkspace().getRelative("output-file"),
/* convenienceName= */ "link-to-output");
assertThat(output).isInstanceOf(LocalInstrumentationOutput.class);

((LocalInstrumentationOutput) output).makeConvenienceLink();
assertThat(env.getWorkspace().getRelative("link-to-output").isSymbolicLink()).isTrue();
}

@Test
public void testInstrumentationOutputFactory_successfulFactoryCreation(
@TestParameter boolean injectRedirectOutputBuilderSupplier,
Expand Down Expand Up @@ -116,7 +139,6 @@ public void handle(Event event) {
new InMemoryFileSystem(DigestHashFunction.SHA256).getPath("/file"),
env,
eventHandler,
/* convenienceName= */ null,
/* append= */ null,
/* internal= */ null);

Expand Down

0 comments on commit 796ed0e

Please sign in to comment.