Skip to content

Commit

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

    Probably the culprit that broke Windows builds with remote caching: bazelbuild/bazel#9072

    *** Original change description ***

    Streamline uploading of stdout and stderr

    This change removes extra upload logic for stdout/stderr. Besides
    the streamlined code this also saves two round trips uploading an
    action that produced both stdout and stderr.

    Closes #9025.

    PiperOrigin-RevId: 261700869
  • Loading branch information
Luca Di Grazia committed Sep 4, 2022
1 parent 94bd48a commit 254d5f9
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -640,10 +640,8 @@ static class UploadManifest {
private final Path execRoot;
private final boolean allowSymlinks;
private final boolean uploadSymlinks;
private final Map<Digest, Path> digestToFile = new HashMap<>();
private final Map<Digest, Chunker> digestToChunkers = new HashMap<>();
private Digest stderrDigest;
private Digest stdoutDigest;
private final Map<Digest, Path> digestToFile;
private final Map<Digest, Chunker> digestToChunkers;

/**
* Create an UploadManifest from an ActionResult builder and an exec root. The ActionResult
Expand All @@ -660,17 +658,9 @@ public UploadManifest(
this.execRoot = execRoot;
this.uploadSymlinks = uploadSymlinks;
this.allowSymlinks = allowSymlinks;
}

public void setStdoutStderr(FileOutErr outErr) throws IOException {
if (outErr.getErrorPath().exists()) {
stderrDigest = digestUtil.compute(outErr.getErrorPath());
addFile(stderrDigest, outErr.getErrorPath());
}
if (outErr.getOutputPath().exists()) {
stdoutDigest = digestUtil.compute(outErr.getOutputPath());
addFile(stdoutDigest, outErr.getOutputPath());
}
this.digestToFile = new HashMap<>();
this.digestToChunkers = new HashMap<>();
}

/**
Expand Down Expand Up @@ -757,16 +747,6 @@ public Map<Digest, Chunker> getDigestToChunkers() {
return digestToChunkers;
}

@Nullable
public Digest getStdoutDigest() {
return stdoutDigest;
}

@Nullable
public Digest getStderrDigest() {
return stderrDigest;
}

private void addFileSymbolicLink(Path file, PathFragment target) throws IOException {
result
.addOutputFileSymlinksBuilder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import com.google.common.base.Ascii;
import com.google.common.base.Preconditions;
import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Maps;
Expand Down Expand Up @@ -418,7 +419,6 @@ void upload(
options.incompatibleRemoteSymlinks,
options.allowSymlinkUpload);
manifest.addFiles(files);
manifest.setStdoutStderr(outErr);
manifest.addAction(actionKey, action, command);

Map<HashCode, Chunker> filesToUpload = Maps.newHashMap();
Expand Down Expand Up @@ -449,13 +449,34 @@ void upload(
uploader.uploadBlobs(filesToUpload, /*forceUpload=*/true);
}

if (manifest.getStderrDigest() != null) {
result.setStderrDigest(manifest.getStderrDigest());
// TODO(olaola): inline small stdout/stderr here.
if (outErr.getErrorPath().exists()) {
Digest stderr = uploadFileContents(outErr.getErrorPath());
result.setStderrDigest(stderr);
}
if (manifest.getStdoutDigest() != null) {
result.setStdoutDigest(manifest.getStdoutDigest());
if (outErr.getOutputPath().exists()) {
Digest stdout = uploadFileContents(outErr.getOutputPath());
result.setStdoutDigest(stdout);
}
}

/**
* Put the file contents cache if it is not already in it. No-op if the file is already stored in
* cache. The given path must be a full absolute path.
*
* @return The key for fetching the file contents blob from cache.
*/
private Digest uploadFileContents(Path file) throws IOException, InterruptedException {
Digest digest = digestUtil.compute(file);
ImmutableSet<Digest> missing = getMissingDigests(ImmutableList.of(digest));
if (!missing.isEmpty()) {
uploader.uploadBlob(
HashCode.fromString(digest.getHash()),
Chunker.builder().setInput(digest.getSizeBytes(), file).build(),
/* forceUpload=*/ true);
}
return digest;
}

// Execution Cache API

Expand Down

0 comments on commit 254d5f9

Please sign in to comment.