diff options
author | tomlu <tomlu@google.com> | 2018-08-10 11:51:20 -0700 |
---|---|---|
committer | Copybara-Service <copybara-piper@google.com> | 2018-08-10 11:52:48 -0700 |
commit | 9efbc25845c5ec4e6eece8540079fb33669e39e5 (patch) | |
tree | 572f4aa6f5db382adf18e950f08de7e8b35cc127 /src/main/java/com | |
parent | 6a118a6c1ccbe3e436e8a87340a8cc554c9cc0a0 (diff) |
Do not crash when attempting to upload a tree artifact.
These show up as directories. Filter these out and return null from the path converter, which should cause omission of those files from any build events.
RELNOTES: None
PiperOrigin-RevId: 208244910
Diffstat (limited to 'src/main/java/com')
3 files changed, 137 insertions, 41 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEventArtifactUploader.java b/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEventArtifactUploader.java index 284ce6de67..67a53142dd 100644 --- a/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEventArtifactUploader.java +++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEventArtifactUploader.java @@ -13,30 +13,14 @@ // limitations under the License. package com.google.devtools.build.lib.buildeventstream; -import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; import com.google.devtools.build.lib.buildeventstream.BuildEvent.LocalFile; -import com.google.devtools.build.lib.buildeventstream.PathConverter.FileUriPathConverter; import com.google.devtools.build.lib.vfs.Path; import java.util.Map; /** Uploads artifacts referenced by the Build Event Protocol (BEP). */ public interface BuildEventArtifactUploader { - BuildEventArtifactUploader LOCAL_FILES_UPLOADER = - new BuildEventArtifactUploader() { - private final ListenableFuture<PathConverter> completedPathConverter = - Futures.immediateFuture(new FileUriPathConverter()); - - @Override - public ListenableFuture<PathConverter> upload(Map<Path, LocalFile> files) { - return completedPathConverter; - } - - @Override - public void shutdown() { - // Intentionally left empty. - } - }; + BuildEventArtifactUploader LOCAL_FILES_UPLOADER = new LocalFilesArtifactUploader(); /** * Asynchronously uploads a set of files referenced by the protobuf representation of a {@link diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/LocalFilesArtifactUploader.java b/src/main/java/com/google/devtools/build/lib/buildeventstream/LocalFilesArtifactUploader.java new file mode 100644 index 0000000000..2cbe93d3f7 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/LocalFilesArtifactUploader.java @@ -0,0 +1,89 @@ +// Copyright 2018 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.buildeventstream; + +import com.google.common.collect.ImmutableMap; +import com.google.common.util.concurrent.Futures; +import com.google.common.util.concurrent.ListenableFuture; +import com.google.common.util.concurrent.ListeningExecutorService; +import com.google.common.util.concurrent.MoreExecutors; +import com.google.devtools.build.lib.buildeventstream.BuildEvent.LocalFile; +import com.google.devtools.build.lib.vfs.Path; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.concurrent.Executors; +import javax.annotation.Nullable; + +/** An uploader that simply turns paths into local file URIs. */ +class LocalFilesArtifactUploader implements BuildEventArtifactUploader { + private final ListeningExecutorService uploadExecutor = + MoreExecutors.listeningDecorator(Executors.newCachedThreadPool()); + + @Override + public ListenableFuture<PathConverter> upload(Map<Path, LocalFile> files) { + List<ListenableFuture<PathLookupResult>> lookups = new ArrayList<>(); + for (Path path : files.keySet()) { + lookups.add(uploadExecutor.submit(() -> new PathLookupResult(path, path.isDirectory()))); + } + return Futures.transform( + Futures.allAsList(lookups), + lookupList -> { + ImmutableMap.Builder<Path, PathLookupResult> pathLookups = ImmutableMap.builder(); + for (PathLookupResult lookup : lookupList) { + pathLookups.put(lookup.path, lookup); + } + return new PathConverterImpl(pathLookups.build()); + }, + MoreExecutors.directExecutor()); + } + + @Override + public void shutdown() { + // Intentionally left empty + } + + private static class PathLookupResult { + final Path path; + final boolean isDirectory; + + private PathLookupResult(Path path, boolean isDirectory) { + this.path = path; + this.isDirectory = isDirectory; + } + } + + private static class PathConverterImpl implements PathConverter { + private static final FileUriPathConverter FILE_URI_PATH_CONVERTER = new FileUriPathConverter(); + private final Map<Path, PathLookupResult> pathLookups; + + private PathConverterImpl(Map<Path, PathLookupResult> pathLookups) { + this.pathLookups = pathLookups; + } + + @Nullable + @Override + public String apply(Path path) { + PathLookupResult result = pathLookups.get(path); + if (result == null) { + // We should throw here, the file wasn't declared in BuildEvent#referencedLocalFiles + return null; + } + if (result.isDirectory) { + return null; + } + return FILE_URI_PATH_CONVERTER.apply(result.path); + } + } +} diff --git a/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploader.java b/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploader.java index d44bfcf12e..2cbe3e6575 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploader.java +++ b/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploader.java @@ -15,8 +15,10 @@ package com.google.devtools.build.lib.remote; import com.google.common.base.Preconditions; import com.google.common.base.Strings; +import com.google.common.collect.ImmutableSet; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; +import com.google.common.util.concurrent.ListeningExecutorService; import com.google.common.util.concurrent.MoreExecutors; import com.google.devtools.build.lib.buildeventstream.BuildEvent.LocalFile; import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploader; @@ -25,11 +27,12 @@ import com.google.devtools.build.lib.remote.util.DigestUtil; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.remoteexecution.v1test.Digest; import io.grpc.Context; -import java.io.IOException; import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; +import java.util.concurrent.Executors; import java.util.concurrent.atomic.AtomicBoolean; import javax.annotation.Nullable; @@ -38,6 +41,8 @@ import javax.annotation.Nullable; */ class ByteStreamBuildEventArtifactUploader implements BuildEventArtifactUploader { + private final ListeningExecutorService uploadExecutor = + MoreExecutors.listeningDecorator(Executors.newCachedThreadPool()); private final Context ctx; private final ByteStreamUploader uploader; private final String remoteServerInstanceName; @@ -63,28 +68,35 @@ class ByteStreamBuildEventArtifactUploader implements BuildEventArtifactUploader } List<ListenableFuture<PathDigestPair>> uploads = new ArrayList<>(files.size()); - Context prevCtx = ctx.attach(); - try { for (Path file : files.keySet()) { - DigestUtil digestUtil = new DigestUtil(file.getFileSystem().getDigestFunction()); - Digest digest = digestUtil.compute(file); - Chunker chunker = Chunker.builder(digestUtil).setInput(digest, file).build(); - ListenableFuture<PathDigestPair> upload = - Futures.transform( - uploader.uploadBlobAsync(chunker, /*forceUpload=*/false), - unused -> new PathDigestPair(file, digest), - MoreExecutors.directExecutor()); - uploads.add(upload); + ListenableFuture<Boolean> isDirectoryFuture = uploadExecutor.submit(() -> file.isDirectory()); + ListenableFuture<PathDigestPair> digestFuture = + Futures.transformAsync( + isDirectoryFuture, + isDirectory -> { + if (isDirectory) { + return Futures.immediateFuture(new PathDigestPair(file, null)); + } + DigestUtil digestUtil = new DigestUtil(file.getFileSystem().getDigestFunction()); + Digest digest = digestUtil.compute(file); + Chunker chunker = Chunker.builder(digestUtil).setInput(digest, file).build(); + final ListenableFuture<Void> upload; + Context prevCtx = ctx.attach(); + try { + upload = uploader.uploadBlobAsync(chunker, /*forceUpload=*/ false); + } finally { + ctx.detach(prevCtx); + } + return Futures.transform( + upload, unused -> new PathDigestPair(file, digest), uploadExecutor); + }, + MoreExecutors.directExecutor()); + uploads.add(digestFuture); } - - return Futures.transform(Futures.allAsList(uploads), - (uploadsDone) -> new PathConverterImpl(remoteServerInstanceName, uploadsDone), - MoreExecutors.directExecutor()); - } catch (IOException e) { - return Futures.immediateFailedFuture(e); - } finally { - ctx.detach(prevCtx); - } + return Futures.transform( + Futures.allAsList(uploads), + pathDigestPairs -> new PathConverterImpl(remoteServerInstanceName, pathDigestPairs), + MoreExecutors.directExecutor()); } @Override @@ -99,15 +111,23 @@ class ByteStreamBuildEventArtifactUploader implements BuildEventArtifactUploader private final String remoteServerInstanceName; private final Map<Path, Digest> pathToDigest; + private final Set<Path> skippedPaths; - PathConverterImpl(String remoteServerInstanceName, - List<PathDigestPair> uploads) { + PathConverterImpl(String remoteServerInstanceName, List<PathDigestPair> uploads) { Preconditions.checkNotNull(uploads); this.remoteServerInstanceName = remoteServerInstanceName; pathToDigest = new HashMap<>(uploads.size()); + ImmutableSet.Builder<Path> skippedPaths = ImmutableSet.builder(); for (PathDigestPair pair : uploads) { - pathToDigest.put(pair.getPath(), pair.getDigest()); + Path path = pair.getPath(); + Digest digest = pair.getDigest(); + if (digest != null) { + pathToDigest.put(path, digest); + } else { + skippedPaths.add(path); + } } + this.skippedPaths = skippedPaths.build(); } @Override @@ -115,6 +135,9 @@ class ByteStreamBuildEventArtifactUploader implements BuildEventArtifactUploader Preconditions.checkNotNull(path); Digest digest = pathToDigest.get(path); if (digest == null) { + if (skippedPaths.contains(path)) { + return null; + } // It's a programming error to reference a file that has not been uploaded. throw new IllegalStateException( String.format("Illegal file reference: '%s'", path.getPathString())); |