diff options
4 files changed, 68 insertions, 23 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/MavenDownloader.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/MavenDownloader.java index 06eccccb0e..574b44afab 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/MavenDownloader.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/MavenDownloader.java @@ -21,6 +21,8 @@ import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.bazel.repository.cache.RepositoryCache; import com.google.devtools.build.lib.bazel.repository.cache.RepositoryCache.KeyType; import com.google.devtools.build.lib.bazel.repository.downloader.HttpDownloader; +import com.google.devtools.build.lib.events.Event; +import com.google.devtools.build.lib.events.ExtendedEventHandler; import com.google.devtools.build.lib.rules.repository.WorkspaceAttributeMapper; import com.google.devtools.build.lib.syntax.EvalException; import com.google.devtools.build.lib.syntax.Type; @@ -56,8 +58,13 @@ public class MavenDownloader extends HttpDownloader { * Download the Maven artifact to the output directory. Returns the path to the jar (and the * srcjar if available). */ - public JarPaths download(String name, WorkspaceAttributeMapper mapper, Path outputDirectory, - MavenServerValue serverValue) throws IOException, EvalException { + public JarPaths download( + String name, + WorkspaceAttributeMapper mapper, + Path outputDirectory, + MavenServerValue serverValue, + ExtendedEventHandler eventHandler) + throws IOException, EvalException { String url = serverValue.getUrl(); Server server = serverValue.getServer(); @@ -80,14 +87,20 @@ public class MavenDownloader extends HttpDownloader { if (isCaching) { Path downloadPath = getDownloadDestination(outputDirectory, artifact); - Path cachedDestination = repositoryCache.get(sha1, downloadPath, KeyType.SHA1); - if (cachedDestination != null) { - Path cachedDestinationSrc = null; - if (sha1Src != null) { - Path downloadPathSrc = getDownloadDestination(outputDirectory, artifactWithSrcs); - cachedDestinationSrc = repositoryCache.get(sha1Src, downloadPathSrc, KeyType.SHA1); + try { + Path cachedDestination = repositoryCache.get(sha1, downloadPath, KeyType.SHA1); + if (cachedDestination != null) { + Path cachedDestinationSrc = null; + if (sha1Src != null) { + Path downloadPathSrc = getDownloadDestination(outputDirectory, artifactWithSrcs); + cachedDestinationSrc = repositoryCache.get(sha1Src, downloadPathSrc, KeyType.SHA1); + } + return new JarPaths(cachedDestination, Optional.fromNullable(cachedDestinationSrc)); } - return new JarPaths(cachedDestination, Optional.fromNullable(cachedDestinationSrc)); + } catch (IOException e) { + eventHandler.handle( + Event.debug("RepositoryCache entry " + sha1 + " is invalid, replacing it...")); + // Ignore error trying to get. We'll just download again. } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/MavenJarFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/MavenJarFunction.java index 903ac93ac6..bb15e606e5 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/MavenJarFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/MavenJarFunction.java @@ -20,6 +20,7 @@ import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.analysis.RuleDefinition; import com.google.devtools.build.lib.bazel.repository.MavenDownloader.JarPaths; import com.google.devtools.build.lib.bazel.rules.workspace.MavenJarRule; +import com.google.devtools.build.lib.events.ExtendedEventHandler; import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue; import com.google.devtools.build.lib.rules.repository.WorkspaceAttributeMapper; @@ -101,11 +102,15 @@ public class MavenJarFunction extends HttpArchiveFunction { } Path outputDir = getExternalRepositoryDirectory(directories).getRelative(rule.getName()); - return createOutputTree(rule, outputDir, serverValue); + return createOutputTree(rule, outputDir, serverValue, env.getListener()); } - private RepositoryDirectoryValue.Builder createOutputTree(Rule rule, Path outputDirectory, - MavenServerValue serverValue) throws RepositoryFunctionException { + private RepositoryDirectoryValue.Builder createOutputTree( + Rule rule, + Path outputDirectory, + MavenServerValue serverValue, + ExtendedEventHandler eventHandler) + throws RepositoryFunctionException { Preconditions.checkState(downloader instanceof MavenDownloader); MavenDownloader mavenDownloader = (MavenDownloader) downloader; @@ -115,7 +120,7 @@ public class MavenJarFunction extends HttpArchiveFunction { try { repositoryJars = mavenDownloader.download( - name, WorkspaceAttributeMapper.of(rule), outputDirectory, serverValue); + name, WorkspaceAttributeMapper.of(rule), outputDirectory, serverValue, eventHandler); } catch (IOException e) { throw new RepositoryFunctionException(e, Transience.TRANSIENT); } catch (EvalException e) { diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpDownloader.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpDownloader.java index d2d3798bce..4e7ccb3cca 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpDownloader.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpDownloader.java @@ -183,10 +183,14 @@ public class HttpDownloader { if (repositoryCache.isEnabled()) { isCachingByProvidedSha256 = true; - Path cachedDestination = repositoryCache.get(sha256, destination, KeyType.SHA256); - if (cachedDestination != null) { - // Cache hit! - return cachedDestination; + try { + Path cachedDestination = repositoryCache.get(sha256, destination, KeyType.SHA256); + if (cachedDestination != null) { + // Cache hit! + return cachedDestination; + } + } catch (IOException e) { + // Ignore error trying to get. We'll just download again. } } diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/MavenJarFunctionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/MavenJarFunctionTest.java index 8b90c612df..9cc82dc27b 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/repository/MavenJarFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/MavenJarFunctionTest.java @@ -16,18 +16,22 @@ package com.google.devtools.build.lib.bazel.repository; import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.fail; +import static org.mockito.Mockito.mock; +import com.google.common.eventbus.EventBus; import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; import com.google.devtools.build.lib.bazel.repository.cache.RepositoryCache; +import com.google.devtools.build.lib.events.EventHandler; +import com.google.devtools.build.lib.events.ExtendedEventHandler; +import com.google.devtools.build.lib.events.Reporter; import com.google.devtools.build.lib.packages.Rule; - import com.google.devtools.build.lib.rules.repository.WorkspaceAttributeMapper; +import java.io.IOException; import org.apache.maven.settings.Server; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; import org.mockito.Mockito; -import java.io.IOException; /** * Tests for {@link MavenJarFunction}. @@ -37,6 +41,9 @@ import java.io.IOException; public class MavenJarFunctionTest extends BuildViewTestCase { private static final MavenServerValue TEST_SERVER = new MavenServerValue( "server", "http://example.com", new Server(), new byte[]{}); + private final EventHandler eventHandler = mock(EventHandler.class); + private final ExtendedEventHandler extendedEventHandler = + new Reporter(new EventBus(), eventHandler); @Test public void testInvalidSha1() throws Exception { @@ -49,7 +56,11 @@ public class MavenJarFunctionTest extends BuildViewTestCase { MavenDownloader downloader = new MavenDownloader(Mockito.mock(RepositoryCache.class)); try { downloader.download( - "foo", WorkspaceAttributeMapper.of(rule), scratch.dir("/whatever"), TEST_SERVER); + "foo", + WorkspaceAttributeMapper.of(rule), + scratch.dir("/whatever"), + TEST_SERVER, + extendedEventHandler); fail("Invalid sha1 should have thrown."); } catch (IOException expected) { assertThat(expected.getMessage()).contains("Invalid SHA-1 for maven_jar foo"); @@ -68,7 +79,11 @@ public class MavenJarFunctionTest extends BuildViewTestCase { MavenDownloader downloader = new MavenDownloader(Mockito.mock(RepositoryCache.class)); try { downloader.download( - "foo", WorkspaceAttributeMapper.of(rule), scratch.dir("/whatever"), TEST_SERVER); + "foo", + WorkspaceAttributeMapper.of(rule), + scratch.dir("/whatever"), + TEST_SERVER, + extendedEventHandler); fail("Expected failure to fetch artifact because of nonexistent server and not due to " + "the existence of a valid SHA"); } catch (IOException expected) { @@ -86,7 +101,11 @@ public class MavenJarFunctionTest extends BuildViewTestCase { MavenDownloader downloader = new MavenDownloader(Mockito.mock(RepositoryCache.class)); try { downloader.download( - "foo", WorkspaceAttributeMapper.of(rule), scratch.dir("/whatever"), TEST_SERVER); + "foo", + WorkspaceAttributeMapper.of(rule), + scratch.dir("/whatever"), + TEST_SERVER, + extendedEventHandler); fail("Expected failure to fetch artifact because of nonexistent server and not due to " + "lack of SHA."); } catch (IOException expected) { @@ -106,7 +125,11 @@ public class MavenJarFunctionTest extends BuildViewTestCase { MavenDownloader downloader = new MavenDownloader(cache); try { downloader.download( - "foo", WorkspaceAttributeMapper.of(rule), scratch.dir("/whatever"), TEST_SERVER); + "foo", + WorkspaceAttributeMapper.of(rule), + scratch.dir("/whatever"), + TEST_SERVER, + extendedEventHandler); fail("Expected failure to fetch artifact because of nonexistent server."); } catch (IOException expected) { assertThat(expected.getMessage()).contains("Failed to fetch Maven dependency:"); |