Skip to content

Commit

Permalink
Automated rollback of commit 5fb1245.
Browse files Browse the repository at this point in the history
*** Reason for rollback ***

Breaks execution_phase_tests.

See https://buildkite.com/bazel/bazel-bazel/builds/13269 for an example in postsubmit.

RELNOTES: None
PiperOrigin-RevId: 321109644
  • Loading branch information
meisterT authored and copybara-github committed Jul 14, 2020
1 parent d917bf2 commit d0a74ff
Show file tree
Hide file tree
Showing 12 changed files with 13 additions and 69 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.syntax.Printer;
import com.google.devtools.build.lib.syntax.Sequence;
import com.google.devtools.build.lib.vfs.BulkDeleter;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.Root;
import com.google.devtools.build.lib.vfs.Symlinks;
Expand Down Expand Up @@ -351,15 +350,8 @@ public void repr(Printer printer) {
* directory recursively removes the contents of the directory.
*
* @param execRoot the exec root in which this action is executed
* @param bulkDeleter a helper to bulk delete outputs to avoid delegating to the filesystem
*/
protected void deleteOutputs(Path execRoot, @Nullable BulkDeleter bulkDeleter)
throws IOException, InterruptedException {
if (bulkDeleter != null) {
bulkDeleter.bulkDelete(Artifact.asPathFragments(getOutputs()));
return;
}

protected void deleteOutputs(Path execRoot) throws IOException {
for (Artifact output : getOutputs()) {
deleteOutput(output.getPath(), output.getRoot());
}
Expand Down Expand Up @@ -457,9 +449,8 @@ protected void checkOutputsForDirectories(ActionExecutionContext actionExecution
}

@Override
public void prepare(Path execRoot, @Nullable BulkDeleter bulkDeleter)
throws IOException, InterruptedException {
deleteOutputs(execRoot, bulkDeleter);
public void prepare(Path execRoot) throws IOException {
deleteOutputs(execRoot);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import com.google.devtools.build.lib.actions.extra.ExtraActionInfo;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ConditionallyThreadCompatible;
import com.google.devtools.build.lib.vfs.BulkDeleter;
import com.google.devtools.build.lib.vfs.Path;
import java.io.IOException;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -83,10 +82,8 @@ public interface Action extends ActionExecutionMetadata {
* or the permissions should be changed, so that they can be safely overwritten by the action.
*
* @throws IOException if there is an error deleting the outputs.
* @throws InterruptedException if the execution is interrupted
*/
void prepare(Path execRoot, @Nullable BulkDeleter bulkDeleter)
throws IOException, InterruptedException;
void prepare(Path execRoot) throws IOException;

/**
* Executes this action. This method <i>unconditionally does the work of the Action</i>, although
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@
import com.google.devtools.build.lib.util.Fingerprint;
import com.google.devtools.build.lib.util.LoggingUtil;
import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.vfs.BulkDeleter;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
Expand Down Expand Up @@ -478,9 +477,8 @@ protected String getRawProgressMessage() {
* the test log base name with arbitrary prefix and extension.
*/
@Override
protected void deleteOutputs(Path execRoot, @Nullable BulkDeleter bulkDeleter)
throws IOException, InterruptedException {
super.deleteOutputs(execRoot, bulkDeleter);
protected void deleteOutputs(Path execRoot) throws IOException {
super.deleteOutputs(execRoot);

// We do not rely on globs, as it causes quadratic behavior in --runs_per_test and test
// shard count.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ private static byte[] printStatusMap(Map<String, String> map) {
return s.getBytes(StandardCharsets.UTF_8);
}

@Override
public void prepare(Path execRoot) throws IOException {
// The default implementation of this method deletes all output files; override it to keep
// the old stableStatus around. This way we can reuse the existing file (preserving its mtime)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.util.DetailedExitCode;
import com.google.devtools.build.lib.util.Fingerprint;
import com.google.devtools.build.lib.vfs.BulkDeleter;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.Symlinks;
import java.io.IOException;
Expand Down Expand Up @@ -65,12 +64,11 @@ public CreateIncSymlinkAction(
}

@Override
public void prepare(Path execRoot, @Nullable BulkDeleter bulkDeleter)
throws IOException, InterruptedException {
public void prepare(Path execRoot) throws IOException {
if (includePath.isDirectory(Symlinks.NOFOLLOW)) {
includePath.deleteTree();
}
super.prepare(execRoot, bulkDeleter);
super.prepare(execRoot);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -633,7 +633,7 @@ public ActionContinuationOrResult execute()
// Fall back to running with the full classpath. This requires first deleting potential
// artifacts generated by the reduced action and clearing the metadata caches.
try {
deleteOutputs(actionExecutionContext.getExecRoot(), /* bulkDeleter= */ null);
deleteOutputs(actionExecutionContext.getExecRoot());
} catch (IOException e) {
throw toActionExecutionException(
new EnvironmentalExecException(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -430,17 +430,12 @@ public BlazeCommandResult exec(CommandEnvironment env, OptionsParsingResult opti
workingDir = env.getExecRoot();

try {
testAction.prepare(env.getExecRoot(), /* bulkDeleter= */ null);
testAction.prepare(env.getExecRoot());
} catch (IOException e) {
return reportAndCreateFailureResult(
env,
"Error while setting up test: " + e.getMessage(),
Code.TEST_ENVIRONMENT_SETUP_FAILURE);
} catch (InterruptedException e) {
return reportAndCreateFailureResult(
env,
"Error while setting up test: " + e.getMessage(),
Code.TEST_ENVIRONMENT_SETUP_INTERRUPTED);
}

try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -930,9 +930,7 @@ public ActionStepOrResult run(Environment env) throws InterruptedException {
// This call generally deletes any files at locations that are declared outputs of the
// action, although some actions perform additional work, while others intentionally
// keep previous outputs in place.
action.prepare(
actionExecutionContext.getExecRoot(),
outputService != null ? outputService.bulkDeleter() : null);
action.prepare(actionExecutionContext.getExecRoot());
} catch (IOException e) {
logger.atWarning().withCause(e).log(
"failed to delete output files before executing action: '%s'", action);
Expand Down
28 changes: 0 additions & 28 deletions src/main/java/com/google/devtools/build/lib/vfs/BulkDeleter.java

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -208,9 +208,4 @@ default ArtifactPathResolver createPathResolverForArtifactValues(
throws IOException {
throw new IllegalStateException("Path resolver not supported by this class");
}

@Nullable
default BulkDeleter bulkDeleter() {
return null;
}
}
1 change: 0 additions & 1 deletion src/main/protobuf/failure_details.proto
Original file line number Diff line number Diff line change
Expand Up @@ -864,7 +864,6 @@ message RunCommand {
SCRIPT_WRITE_FAILURE = 12 [(metadata) = { exit_code: 6 }];
RUNFILES_DIRECTORIES_CREATION_FAILURE = 13 [(metadata) = { exit_code: 36 }];
RUNFILES_SYMLINKS_CREATION_FAILURE = 14 [(metadata) = { exit_code: 36 }];
TEST_ENVIRONMENT_SETUP_INTERRUPTED = 15 [(metadata) = { exit_code: 8 }];
}

Code code = 1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ public void testFileRemoved() throws Exception {
Path extra = rootDirectory.getRelative("out/extra");
FileSystemUtils.createEmptyFile(extra);
assertThat(extra.exists()).isTrue();
action.prepare(rootDirectory, /*bulkDeleter=*/ null);
action.prepare(rootDirectory);
assertThat(extra.exists()).isFalse();
}

Expand Down

0 comments on commit d0a74ff

Please sign in to comment.