diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java index f135348c807829..1dc8e1ef1df341 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java @@ -71,9 +71,7 @@ import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.RepositoryOverride; import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.WorkerForRepoFetching; import com.google.devtools.build.lib.bazel.repository.cache.RepositoryCache; -import com.google.devtools.build.lib.bazel.repository.downloader.DelegatingDownloader; import com.google.devtools.build.lib.bazel.repository.downloader.DownloadManager; -import com.google.devtools.build.lib.bazel.repository.downloader.HttpDownloader; import com.google.devtools.build.lib.bazel.repository.downloader.UrlRewriter; import com.google.devtools.build.lib.bazel.repository.downloader.UrlRewriterParseException; import com.google.devtools.build.lib.bazel.repository.starlark.StarlarkRepositoryFunction; @@ -150,11 +148,6 @@ public class BazelRepositoryModule extends BlazeModule { private final AtomicBoolean isFetch = new AtomicBoolean(false); private final StarlarkRepositoryFunction starlarkRepositoryFunction; private final RepositoryCache repositoryCache = new RepositoryCache(); - private final HttpDownloader httpDownloader = new HttpDownloader(); - private final DelegatingDownloader delegatingDownloader = - new DelegatingDownloader(httpDownloader); - private final DownloadManager downloadManager = - new DownloadManager(repositoryCache, delegatingDownloader); private final MutableSupplier> clientEnvironmentSupplier = new MutableSupplier<>(); private ImmutableMap overrides = ImmutableMap.of(); @@ -174,12 +167,16 @@ public class BazelRepositoryModule extends BlazeModule { private boolean disableNativeRepoRules; private SingleExtensionEvalFunction singleExtensionEvalFunction; + private final VendorCommand vendorCommand = new VendorCommand(clientEnvironmentSupplier); + private final RegistryFactoryImpl registryFactory = + new RegistryFactoryImpl(clientEnvironmentSupplier); + @Nullable private CredentialModule credentialModule; private ImmutableMap builtinModules = null; public BazelRepositoryModule() { - this.starlarkRepositoryFunction = new StarlarkRepositoryFunction(downloadManager); + this.starlarkRepositoryFunction = new StarlarkRepositoryFunction(); this.repositoryHandlers = repositoryRules(); } @@ -228,7 +225,7 @@ public void serverInit(OptionsParsingResult startupOptions, ServerBuilder builde builder.addCommands(new FetchCommand()); builder.addCommands(new ModCommand()); builder.addCommands(new SyncCommand()); - builder.addCommands(new VendorCommand(downloadManager, clientEnvironmentSupplier)); + builder.addCommands(vendorCommand); builder.addInfoItems(new RepositoryCacheInfoItem(repositoryCache)); } @@ -252,7 +249,7 @@ public void workspaceInit( directories, BazelSkyframeExecutorConstants.EXTERNAL_PACKAGE_HELPER); singleExtensionEvalFunction = - new SingleExtensionEvalFunction(directories, clientEnvironmentSupplier, downloadManager); + new SingleExtensionEvalFunction(directories, clientEnvironmentSupplier); if (builtinModules == null) { builtinModules = ModuleFileFunction.getBuiltinModules(directories.getEmbeddedBinariesRoot()); @@ -278,9 +275,7 @@ SkyFunctions.BAZEL_LOCK_FILE, new BazelLockFileFunction(directories.getWorkspace .addSkyFunction(SkyFunctions.SINGLE_EXTENSION_USAGES, new SingleExtensionUsagesFunction()) .addSkyFunction( SkyFunctions.REGISTRY, - new RegistryFunction( - new RegistryFactoryImpl(downloadManager, clientEnvironmentSupplier), - directories.getWorkspace())) + new RegistryFunction(registryFactory, directories.getWorkspace())) .addSkyFunction(SkyFunctions.REPO_SPEC, new RepoSpecFunction()) .addSkyFunction(SkyFunctions.YANKED_VERSIONS, new YankedVersionsFunction()) .addSkyFunction( @@ -314,6 +309,12 @@ public void initializeRuleClasses(ConfiguredRuleClassProvider.Builder builder) { @Override public void beforeCommand(CommandEnvironment env) throws AbruptExitException { + DownloadManager downloadManager = + new DownloadManager(repositoryCache, env.getDownloaderDelegate(), env.getHttpDownloader()); + this.starlarkRepositoryFunction.setDownloadManager(downloadManager); + this.vendorCommand.setDownloadManager(downloadManager); + this.registryFactory.setDownloadManager(downloadManager); + clientEnvironmentSupplier.set(env.getRepoEnv()); PackageOptions pkgOptions = env.getOptions().getOptions(PackageOptions.class); isFetch.set(pkgOptions != null && pkgOptions.fetch); @@ -323,6 +324,7 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException { starlarkRepositoryFunction.setProcessWrapper(processWrapper); starlarkRepositoryFunction.setSyscallCache(env.getSyscallCache()); singleExtensionEvalFunction.setProcessWrapper(processWrapper); + singleExtensionEvalFunction.setDownloadManager(downloadManager); RepositoryOptions repoOptions = env.getOptions().getOptions(RepositoryOptions.class); if (repoOptions != null) { @@ -453,16 +455,6 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException { downloadManager.setDistdir(ImmutableList.of()); } - if (repoOptions.httpTimeoutScaling > 0) { - httpDownloader.setTimeoutScaling((float) repoOptions.httpTimeoutScaling); - } else { - env.getReporter() - .handle(Event.warn("Ignoring request to scale http timeouts by a non-positive factor")); - httpDownloader.setTimeoutScaling(1.0f); - } - httpDownloader.setMaxAttempts(repoOptions.httpConnectorAttempts); - httpDownloader.setMaxRetryTimeout(repoOptions.httpConnectorRetryMaxTimeout); - if (repoOptions.repositoryOverrides != null) { // To get the usual latest-wins semantics, we need a mutable map, as the builder // of an immutable map does not allow redefining the values of existing keys. @@ -568,7 +560,6 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException { } starlarkRepositoryFunction.setRepositoryRemoteExecutor(remoteExecutor); singleExtensionEvalFunction.setRepositoryRemoteExecutor(remoteExecutor); - delegatingDownloader.setDelegate(env.getRuntime().getDownloaderSupplier().get()); clock = env.getClock(); try { diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RegistryFactoryImpl.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RegistryFactoryImpl.java index e26468f4890528..c15c847771327d 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RegistryFactoryImpl.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RegistryFactoryImpl.java @@ -26,18 +26,21 @@ import java.util.Map; import java.util.Optional; import java.util.function.Supplier; +import javax.annotation.Nullable; /** Prod implementation of {@link RegistryFactory}. */ public class RegistryFactoryImpl implements RegistryFactory { - private final DownloadManager downloadManager; + @Nullable private DownloadManager downloadManager; private final Supplier> clientEnvironmentSupplier; - public RegistryFactoryImpl( - DownloadManager downloadManager, Supplier> clientEnvironmentSupplier) { - this.downloadManager = downloadManager; + public RegistryFactoryImpl(Supplier> clientEnvironmentSupplier) { this.clientEnvironmentSupplier = clientEnvironmentSupplier; } + public void setDownloadManager(DownloadManager downloadManager) { + this.downloadManager = downloadManager; + } + @Override public Registry createRegistry( String url, diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java index e452dda9e67ee8..1e69f4aa2da16e 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java @@ -101,18 +101,19 @@ public class SingleExtensionEvalFunction implements SkyFunction { private final BlazeDirectories directories; private final Supplier> clientEnvironmentSupplier; - private final DownloadManager downloadManager; private double timeoutScaling = 1.0; @Nullable private ProcessWrapper processWrapper = null; @Nullable private RepositoryRemoteExecutor repositoryRemoteExecutor = null; + @Nullable private DownloadManager downloadManager = null; public SingleExtensionEvalFunction( - BlazeDirectories directories, - Supplier> clientEnvironmentSupplier, - DownloadManager downloadManager) { + BlazeDirectories directories, Supplier> clientEnvironmentSupplier) { this.directories = directories; this.clientEnvironmentSupplier = clientEnvironmentSupplier; + } + + public void setDownloadManager(DownloadManager downloadManager) { this.downloadManager = downloadManager; } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/commands/VendorCommand.java b/src/main/java/com/google/devtools/build/lib/bazel/commands/VendorCommand.java index cde398ce3dea07..64a6e7f2dcf5f2 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/commands/VendorCommand.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/commands/VendorCommand.java @@ -109,16 +109,18 @@ public final class VendorCommand implements BlazeCommand { public static final String NAME = "vendor"; - private final DownloadManager downloadManager; private final Supplier> clientEnvironmentSupplier; @Nullable private VendorManager vendorManager = null; + @Nullable private DownloadManager downloadManager; - public VendorCommand( - DownloadManager downloadManager, Supplier> clientEnvironmentSupplier) { - this.downloadManager = downloadManager; + public VendorCommand(Supplier> clientEnvironmentSupplier) { this.clientEnvironmentSupplier = clientEnvironmentSupplier; } + public void setDownloadManager(DownloadManager downloadManager) { + this.downloadManager = downloadManager; + } + @Override public void editOptions(OptionsParser optionsParser) { // We only need to inject these options with fetch target (when there is a residue) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/RepositoryOptions.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/RepositoryOptions.java index efa007f629b418..d6bf6656f362e4 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/RepositoryOptions.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/RepositoryOptions.java @@ -27,7 +27,6 @@ import com.google.devtools.common.options.OptionMetadataTag; import com.google.devtools.common.options.OptionsBase; import com.google.devtools.common.options.OptionsParsingException; -import java.time.Duration; import java.util.List; /** Command-line options for repositories. */ @@ -133,31 +132,7 @@ public class RepositoryOptions extends OptionsBase { + "to download them.") public List experimentalDistdir; - @Option( - name = "http_timeout_scaling", - defaultValue = "1.0", - documentationCategory = OptionDocumentationCategory.BAZEL_CLIENT_OPTIONS, - effectTags = {OptionEffectTag.BAZEL_INTERNAL_CONFIGURATION}, - help = "Scale all timeouts related to http downloads by the given factor") - public double httpTimeoutScaling; - - @Option( - name = "http_connector_attempts", - defaultValue = "8", - documentationCategory = OptionDocumentationCategory.BAZEL_CLIENT_OPTIONS, - effectTags = {OptionEffectTag.BAZEL_INTERNAL_CONFIGURATION}, - help = "The maximum number of attempts for http downloads.") - public int httpConnectorAttempts; - @Option( - name = "http_connector_retry_max_timeout", - defaultValue = "0s", - documentationCategory = OptionDocumentationCategory.BAZEL_CLIENT_OPTIONS, - effectTags = {OptionEffectTag.BAZEL_INTERNAL_CONFIGURATION}, - help = - "The maximum timeout for http download retries. With a value of 0, no timeout maximum is" - + " defined.") - public Duration httpConnectorRetryMaxTimeout; @Option( name = "override_repository", diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/DownloadManager.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/DownloadManager.java index 0b5064cc8dbcd6..0626f1e9e9c930 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/DownloadManager.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/DownloadManager.java @@ -58,9 +58,10 @@ */ public class DownloadManager { private final RepositoryCache repositoryCache; - private List distdir = ImmutableList.of(); + private ImmutableList distdir = ImmutableList.of(); private UrlRewriter rewriter; private final Downloader downloader; + private final HttpDownloader bzlmodHttpDownloader; private boolean disableDownload = false; private int retries = 0; @Nullable private Credentials netrcCreds; @@ -71,9 +72,19 @@ public interface CredentialFactory { Credentials create(Map>> authHeaders); } - public DownloadManager(RepositoryCache repositoryCache, Downloader downloader) { + /** + * Creates a new {@link DownloadManager}. + * + * @param downloader The (delegating) downloader to use to download files. Is either a + * HttpDownloader, or a GrpcRemoteDownloader. + * @param bzlmodHttpDownloader The downloader to use for downloading files from the bzlmod + * registry. + */ + public DownloadManager( + RepositoryCache repositoryCache, Downloader downloader, HttpDownloader bzlmodHttpDownloader) { this.repositoryCache = repositoryCache; this.downloader = downloader; + this.bzlmodHttpDownloader = bzlmodHttpDownloader; } public void setDistdir(List distdir) { @@ -425,12 +436,11 @@ public byte[] downloadAndReadOneUrlForBzlmod( throw new IOException(getRewriterBlockedAllUrlsMessage(ImmutableList.of(originalUrl))); } - HttpDownloader httpDownloader = new HttpDownloader(); byte[] content; for (int attempt = 0; ; ++attempt) { try { content = - httpDownloader.downloadAndReadOneUrl( + bzlmodHttpDownloader.downloadAndReadOneUrl( rewrittenUrls.get(0), credentialFactory.create(authHeaders), checksum, 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 796ac03f631a54..0651ab8c261308 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 @@ -46,31 +46,29 @@ * *

This class uses {@link HttpConnectorMultiplexer} to connect to HTTP mirrors and then reads the * file to disk. + * + *

This class is (outside of tests) a singleton instance, living in `BazelRepositoryModule`. */ public class HttpDownloader implements Downloader { - static final int MAX_PARALLEL_DOWNLOADS = 8; - - private static final Semaphore SEMAPHORE = new Semaphore(MAX_PARALLEL_DOWNLOADS, true); private static final Clock CLOCK = new JavaClock(); private static final Sleeper SLEEPER = new JavaSleeper(); private static final Locale LOCALE = Locale.getDefault(); - private float timeoutScaling = 1.0f; - private int maxAttempts = 0; - private Duration maxRetryTimeout = Duration.ZERO; - - public HttpDownloader() {} + private final Semaphore semaphore; + private final float timeoutScaling; + private final int maxAttempts; + private final Duration maxRetryTimeout; - public void setTimeoutScaling(float timeoutScaling) { - this.timeoutScaling = timeoutScaling; - } - - public void setMaxAttempts(int maxAttempts) { + public HttpDownloader( + int maxAttempts, Duration maxRetryTimeout, int maxParallelDownloads, float timeoutScaling) { this.maxAttempts = maxAttempts; + this.maxRetryTimeout = maxRetryTimeout; + semaphore = new Semaphore(maxParallelDownloads, true); + this.timeoutScaling = timeoutScaling; } - public void setMaxRetryTimeout(Duration maxRetryTimeout) { - this.maxRetryTimeout = maxRetryTimeout; + public HttpDownloader() { + this(0, Duration.ZERO, 8, 1.0f); } @Override @@ -94,7 +92,7 @@ public void download( List ioExceptions = ImmutableList.of(); for (URL url : urls) { - SEMAPHORE.acquire(); + semaphore.acquire(); try (HttpStream payload = multiplexer.connect(url, checksum, headers, credentials, type); OutputStream out = destination.getOutputStream()) { @@ -119,7 +117,7 @@ public void download( Event.warn("Download from " + url + " failed: " + e.getClass() + " " + e.getMessage())); continue; } finally { - SEMAPHORE.release(); + semaphore.release(); eventHandler.post(new FetchEvent(url.toString(), success)); } } @@ -154,7 +152,7 @@ public byte[] downloadAndReadOneUrl( HttpConnectorMultiplexer multiplexer = setUpConnectorMultiplexer(eventHandler, clientEnv); ByteArrayOutputStream out = new ByteArrayOutputStream(); - SEMAPHORE.acquire(); + semaphore.acquire(); try (HttpStream payload = multiplexer.connect(url, checksum, ImmutableMap.of(), credentials, Optional.empty())) { ByteStreams.copy(payload, out); @@ -166,7 +164,7 @@ public byte[] downloadAndReadOneUrl( } catch (InterruptedIOException e) { throw new InterruptedException(e.getMessage()); } finally { - SEMAPHORE.release(); + semaphore.release(); // TODO(wyv): Do we need to report any event here? } return out.toByteArray(); diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryFunction.java index 042f3003eb7e33..b546dabc2f79ab 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryFunction.java @@ -73,21 +73,23 @@ /** A repository function to delegate work done by Starlark remote repositories. */ public final class StarlarkRepositoryFunction extends RepositoryFunction { - private final DownloadManager downloadManager; private double timeoutScaling = 1.0; private boolean useWorkers; + @Nullable private DownloadManager downloadManager; @Nullable private ProcessWrapper processWrapper = null; @Nullable private RepositoryRemoteExecutor repositoryRemoteExecutor; @Nullable private SyscallCache syscallCache; - public StarlarkRepositoryFunction(DownloadManager downloadManager) { - this.downloadManager = downloadManager; - } + public StarlarkRepositoryFunction() {} public void setTimeoutScaling(double timeoutScaling) { this.timeoutScaling = timeoutScaling; } + public void setDownloadManager(DownloadManager downloadManager) { + this.downloadManager = downloadManager; + } + public void setProcessWrapper(@Nullable ProcessWrapper processWrapper) { this.processWrapper = processWrapper; } diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java index cc14e0b676a2a5..02ca1595a955d7 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java @@ -45,7 +45,6 @@ import com.google.devtools.build.lib.authandtls.credentialhelper.CredentialModule; import com.google.devtools.build.lib.authandtls.credentialhelper.GetCredentialsResponse; import com.google.devtools.build.lib.bazel.repository.downloader.Downloader; -import com.google.devtools.build.lib.bazel.repository.downloader.HttpDownloader; import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploader; import com.google.devtools.build.lib.buildeventstream.LocalFilesArtifactUploader; import com.google.devtools.build.lib.buildtool.BuildRequest; @@ -89,7 +88,6 @@ import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; import com.google.devtools.build.lib.server.FailureDetails.RemoteExecution; import com.google.devtools.build.lib.server.FailureDetails.RemoteExecution.Code; -import com.google.devtools.build.lib.skyframe.MutableSupplier; import com.google.devtools.build.lib.util.AbruptExitException; import com.google.devtools.build.lib.util.DetailedExitCode; import com.google.devtools.build.lib.util.ExitCode; @@ -165,7 +163,7 @@ public ManagedChannel newChannel( private final RepositoryRemoteExecutorFactoryDelegate repositoryRemoteExecutorFactoryDelegate = new RepositoryRemoteExecutorFactoryDelegate(); - private final MutableSupplier remoteDownloaderSupplier = new MutableSupplier<>(); + private Downloader remoteDownloader; private CredentialModule credentialModule; @@ -174,7 +172,6 @@ public void serverInit(OptionsParsingResult startupOptions, ServerBuilder builde builder.addBuildEventArtifactUploaderFactory( buildEventArtifactUploaderFactoryDelegate, "remote"); builder.setRepositoryRemoteExecutorFactory(repositoryRemoteExecutorFactoryDelegate); - builder.setDownloaderSupplier(remoteDownloaderSupplier); } /** Returns whether remote execution should be available. */ @@ -723,9 +720,9 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException { Downloader fallbackDownloader = null; if (remoteOptions.remoteDownloaderLocalFallback) { - fallbackDownloader = new HttpDownloader(); + fallbackDownloader = env.getHttpDownloader(); } - remoteDownloaderSupplier.set( + remoteDownloader = new GrpcRemoteDownloader( buildRequestId, invocationId, @@ -736,8 +733,9 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException { digestUtil.getDigestFunction(), remoteOptions, verboseFailures, - fallbackDownloader)); + fallbackDownloader); downloaderChannel.release(); + env.getDownloaderDelegate().setDelegate(remoteDownloader); } } @@ -927,7 +925,7 @@ public void afterCommand() { buildEventArtifactUploaderFactoryDelegate.reset(); repositoryRemoteExecutorFactoryDelegate.reset(); - remoteDownloaderSupplier.set(null); + remoteDownloader = null; actionContextProvider = null; actionInputFetcher = null; remoteOptions = null; @@ -1226,7 +1224,7 @@ static Credentials createCredentials( } @VisibleForTesting - MutableSupplier getRemoteDownloaderSupplier() { - return remoteDownloaderSupplier; + Downloader getRemoteDownloader() { + return remoteDownloader; } } diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java index 2e00120e1fc66c..487e0d966c70b8 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java @@ -39,7 +39,6 @@ import com.google.devtools.build.lib.analysis.ServerDirectories; import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.analysis.test.CoverageReportActionFactory; -import com.google.devtools.build.lib.bazel.repository.downloader.Downloader; import com.google.devtools.build.lib.bugreport.BugReport; import com.google.devtools.build.lib.bugreport.BugReporter; import com.google.devtools.build.lib.bugreport.Crash; @@ -135,7 +134,6 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Function; -import java.util.function.Supplier; import java.util.logging.Handler; import java.util.logging.LogRecord; import java.util.logging.Logger; @@ -188,7 +186,6 @@ public final class BlazeRuntime implements BugReport.BlazeRuntimeInterface { private final ActionKeyContext actionKeyContext; private final ImmutableMap authHeadersProviderMap; @Nullable private final RepositoryRemoteExecutorFactory repositoryRemoteExecutorFactory; - private final Supplier downloaderSupplier; // Workspace state (currently exactly one workspace per server) private BlazeWorkspace workspace; @@ -215,8 +212,7 @@ private BlazeRuntime( String productName, BuildEventArtifactUploaderFactoryMap buildEventArtifactUploaderFactoryMap, ImmutableMap authHeadersProviderMap, - RepositoryRemoteExecutorFactory repositoryRemoteExecutorFactory, - Supplier downloaderSupplier) { + RepositoryRemoteExecutorFactory repositoryRemoteExecutorFactory) { // Server state this.fileSystem = fileSystem; this.blazeModules = blazeModules; @@ -246,7 +242,6 @@ private BlazeRuntime( this.authHeadersProviderMap = Preconditions.checkNotNull(authHeadersProviderMap, "authHeadersProviderMap"); this.repositoryRemoteExecutorFactory = repositoryRemoteExecutorFactory; - this.downloaderSupplier = downloaderSupplier; } public BlazeWorkspace initWorkspace(BlazeDirectories directories, BinTools binTools) @@ -1430,10 +1425,6 @@ public RepositoryRemoteExecutorFactory getRepositoryRemoteExecutorFactory() { return repositoryRemoteExecutorFactory; } - public Supplier getDownloaderSupplier() { - return downloaderSupplier; - } - /** * A builder for {@link BlazeRuntime} objects. The only required fields are the {@link * BlazeDirectories}, and the {@link com.google.devtools.build.lib.packages.RuleClassProvider} @@ -1560,8 +1551,7 @@ public BlazeRuntime build() throws AbruptExitException { productName, serverBuilder.getBuildEventArtifactUploaderMap(), serverBuilder.getAuthHeadersProvidersMap(), - serverBuilder.getRepositoryRemoteExecutorFactory(), - serverBuilder.getDownloaderSupplier()); + serverBuilder.getRepositoryRemoteExecutorFactory()); AutoProfiler.setClock(runtime.getClock()); BugReport.setRuntime(runtime); return runtime; diff --git a/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java b/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java index 28cb21b1e45c8f..9ffaf2d99a2bdf 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java @@ -30,10 +30,13 @@ import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.analysis.BuildInfoEvent; import com.google.devtools.build.lib.analysis.config.CoreOptions; +import com.google.devtools.build.lib.bazel.repository.downloader.DelegatingDownloader; +import com.google.devtools.build.lib.bazel.repository.downloader.HttpDownloader; import com.google.devtools.build.lib.buildtool.BuildRequestOptions; import com.google.devtools.build.lib.clock.Clock; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.concurrent.QuiescingExecutors; +import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.Reporter; import com.google.devtools.build.lib.exec.SingleBuildFileCache; import com.google.devtools.build.lib.pkgcache.PackageManager; @@ -44,6 +47,8 @@ import com.google.devtools.build.lib.profiler.SilentCloseable; import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.InvocationPolicy; import com.google.devtools.build.lib.server.FailureDetails; +import com.google.devtools.build.lib.server.FailureDetails.ExternalRepository; +import com.google.devtools.build.lib.server.FailureDetails.ExternalRepository.Code; import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; import com.google.devtools.build.lib.skyframe.BuildResultListener; import com.google.devtools.build.lib.skyframe.SkyframeBuildView; @@ -89,8 +94,8 @@ public class CommandEnvironment { private final BlazeWorkspace workspace; private final BlazeDirectories directories; - private final UUID commandId; // Unique identifier for the command being run - private final String buildRequestId; // Unique identifier for the build being run + private final UUID commandId; // Unique identifier for the command being run + private final String buildRequestId; // Unique identifier for the build being run private final Reporter reporter; private final EventBus eventBus; private final BlazeModule.ModuleEnvironment blazeModuleEnvironment; @@ -115,6 +120,8 @@ public class CommandEnvironment { private final Consumer shutdownReasonConsumer; private final BuildResultListener buildResultListener; private final CommandLinePathFactory commandLinePathFactory; + private final HttpDownloader httpDownloader; + private final DelegatingDownloader delegatingDownloader; private boolean mergedAnalysisAndExecution; @@ -245,6 +252,31 @@ public void exit(AbruptExitException exception) { } workspace.getSkyframeExecutor().setEventBus(eventBus); eventBus.register(this); + float httpTimeoutScaling = (float) commandOptions.httpTimeoutScaling; + if (commandOptions.httpTimeoutScaling <= 0) { + reporter.handle( + Event.warn("Ignoring request to scale http timeouts by a non-positive factor")); + httpTimeoutScaling = 1.0f; + } + if (commandOptions.httpMaxParallelDownloads <= 0) { + this.blazeModuleEnvironment.exit( + new AbruptExitException( + DetailedExitCode.of( + FailureDetail.newBuilder() + .setMessage( + "The maximum number of parallel downloads needs to be a positive number") + .setExternalRepository( + ExternalRepository.newBuilder().setCode(Code.BAD_DOWNLOADER_CONFIG)) + .build()))); + } + + this.httpDownloader = + new HttpDownloader( + commandOptions.httpConnectorAttempts, + commandOptions.httpConnectorRetryMaxTimeout, + commandOptions.httpMaxParallelDownloads, + httpTimeoutScaling); + this.delegatingDownloader = new DelegatingDownloader(httpDownloader); ClientOptions clientOptions = Preconditions.checkNotNull( @@ -391,9 +423,7 @@ public PathPackageLocator getPackageLocator() { return packageLocator; } - /** - * Returns the reporter for events. - */ + /** Returns the reporter for events. */ public Reporter getReporter() { return reporter; } @@ -417,6 +447,7 @@ public ImmutableMap getClientEnv() { public Command getCommand() { return command; } + public String getCommandName() { return command.name(); } @@ -582,9 +613,8 @@ public boolean inWorkspace() { } /** - * Returns the output base directory associated with this Blaze server - * process. This is the base directory for shared Blaze state as well as tool - * and strategy specific subdirectories. + * Returns the output base directory associated with this Blaze server process. This is the base + * directory for shared Blaze state as well as tool and strategy specific subdirectories. */ public Path getOutputBase() { return getDirectories().getOutputBase(); @@ -704,8 +734,8 @@ public AbruptExitException getPendingException() { * Throws the exception currently queued by a Blaze module. * *

This should be called as often as is practical so that errors are reported as soon as - * possible. Ideally, we'd not need this, but the event bus swallows exceptions so we raise - * the exception this way. + * possible. Ideally, we'd not need this, but the event bus swallows exceptions so we raise the + * exception this way. */ public void throwPendingException() throws AbruptExitException { AbruptExitException exception = getPendingException(); @@ -914,4 +944,12 @@ public void ensureBuildInfoPosted() { void gotBuildInfo(BuildInfoEvent event) { buildInfoPosted = true; } + + public HttpDownloader getHttpDownloader() { + return httpDownloader; + } + + public DelegatingDownloader getDownloaderDelegate() { + return delegatingDownloader; + } } diff --git a/src/main/java/com/google/devtools/build/lib/runtime/CommonCommandOptions.java b/src/main/java/com/google/devtools/build/lib/runtime/CommonCommandOptions.java index 9657df3e263d68..7eb1434c82731b 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/CommonCommandOptions.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/CommonCommandOptions.java @@ -31,6 +31,7 @@ import com.google.devtools.common.options.OptionsBase; import com.google.devtools.common.options.OptionsParsingException; import com.google.devtools.common.options.TriState; +import java.time.Duration; import java.util.List; import java.util.Map; import java.util.UUID; @@ -532,6 +533,40 @@ public String getTypeDescription() { + " them.") public boolean heuristicallyDropNodes; + @Option( + name = "http_timeout_scaling", + defaultValue = "1.0", + documentationCategory = OptionDocumentationCategory.BAZEL_CLIENT_OPTIONS, + effectTags = {OptionEffectTag.BAZEL_INTERNAL_CONFIGURATION}, + help = "Scale all timeouts related to http downloads by the given factor") + public double httpTimeoutScaling; + + @Option( + name = "http_connector_attempts", + defaultValue = "8", + documentationCategory = OptionDocumentationCategory.BAZEL_CLIENT_OPTIONS, + effectTags = {OptionEffectTag.BAZEL_INTERNAL_CONFIGURATION}, + help = "The maximum number of attempts for http downloads.") + public int httpConnectorAttempts; + + @Option( + name = "http_connector_retry_max_timeout", + defaultValue = "0s", + documentationCategory = OptionDocumentationCategory.BAZEL_CLIENT_OPTIONS, + effectTags = {OptionEffectTag.BAZEL_INTERNAL_CONFIGURATION}, + help = + "The maximum timeout for http download retries. With a value of 0, no timeout maximum is" + + " defined.") + public Duration httpConnectorRetryMaxTimeout; + + @Option( + name = "http_max_parallel_downloads", + defaultValue = "8", + documentationCategory = OptionDocumentationCategory.BAZEL_CLIENT_OPTIONS, + effectTags = {OptionEffectTag.BAZEL_INTERNAL_CONFIGURATION}, + help = "The maximum number parallel http downloads.") + public int httpMaxParallelDownloads; + /** The option converter to check that the user can only specify legal profiler tasks. */ public static class ProfilerTaskConverter extends EnumConverter { public ProfilerTaskConverter() { diff --git a/src/main/java/com/google/devtools/build/lib/runtime/ServerBuilder.java b/src/main/java/com/google/devtools/build/lib/runtime/ServerBuilder.java index 4874940cd27cbd..f9634eb57b2749 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/ServerBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/ServerBuilder.java @@ -17,13 +17,11 @@ import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import com.google.devtools.build.lib.bazel.repository.downloader.Downloader; import com.google.devtools.build.lib.query2.QueryEnvironmentFactory; import com.google.devtools.build.lib.query2.engine.QueryEnvironment.QueryFunction; import com.google.devtools.build.lib.query2.query.output.OutputFormatter; import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.InvocationPolicy; import com.google.errorprone.annotations.CanIgnoreReturnValue; -import java.util.function.Supplier; /** * Builder class to create a {@link BlazeRuntime} instance. This class is part of the module API, @@ -42,7 +40,6 @@ public final class ServerBuilder { private final ImmutableMap.Builder authHeadersProvidersMap = ImmutableMap.builder(); private RepositoryRemoteExecutorFactory repositoryRemoteExecutorFactory; - private Supplier downloaderSupplier = () -> null; @VisibleForTesting public ServerBuilder() {} @@ -82,10 +79,6 @@ public RepositoryRemoteExecutorFactory getRepositoryRemoteExecutorFactory() { return repositoryRemoteExecutorFactory; } - public Supplier getDownloaderSupplier() { - return downloaderSupplier; - } - /** * Merges the given invocation policy into the per-server invocation policy. While this can accept * any number of policies, the end result is order-dependent if multiple policies attempt to @@ -177,12 +170,6 @@ public ServerBuilder setRepositoryRemoteExecutorFactory( return this; } - @CanIgnoreReturnValue - public ServerBuilder setDownloaderSupplier(Supplier downloaderSupplier) { - this.downloaderSupplier = downloaderSupplier; - return this; - } - /** * Register a provider of authentication headers that blaze modules can use. See {@link * AuthHeadersProvider} for more details. diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/packages/BazelPackageLoader.java b/src/main/java/com/google/devtools/build/lib/skyframe/packages/BazelPackageLoader.java index 36e72039fdcb95..067333494e99b2 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/packages/BazelPackageLoader.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/packages/BazelPackageLoader.java @@ -22,7 +22,6 @@ import com.google.devtools.build.lib.bazel.bzlmod.BazelLockFileFunction; import com.google.devtools.build.lib.bazel.bzlmod.BazelModuleResolutionFunction; import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileFunction; -import com.google.devtools.build.lib.bazel.bzlmod.RegistryFactory; import com.google.devtools.build.lib.bazel.bzlmod.RegistryFactoryImpl; import com.google.devtools.build.lib.bazel.bzlmod.RegistryFunction; import com.google.devtools.build.lib.bazel.bzlmod.RepoSpecFunction; @@ -137,9 +136,11 @@ public BazelPackageLoader buildImpl() { // Set up SkyFunctions and PrecomputedValues needed to make local repositories work correctly. RepositoryCache repositoryCache = new RepositoryCache(); HttpDownloader httpDownloader = new HttpDownloader(); - DownloadManager downloadManager = new DownloadManager(repositoryCache, httpDownloader); - RegistryFactory registryFactory = - new RegistryFactoryImpl(downloadManager, Suppliers.ofInstance(ImmutableMap.of())); + DownloadManager downloadManager = + new DownloadManager(repositoryCache, httpDownloader, httpDownloader); + RegistryFactoryImpl registryFactory = + new RegistryFactoryImpl(Suppliers.ofInstance(ImmutableMap.of())); + registryFactory.setDownloadManager(downloadManager); // Allow tests to override the following functions to use fake registry or custom built-in // modules @@ -163,6 +164,8 @@ public BazelPackageLoader buildImpl() { SkyFunctions.REGISTRY, new RegistryFunction(registryFactory, directories.getWorkspace()))); } + StarlarkRepositoryFunction starlarkRepositoryFunction = new StarlarkRepositoryFunction(); + starlarkRepositoryFunction.setDownloadManager(downloadManager); addExtraSkyFunctions( ImmutableMap.builder() @@ -181,7 +184,7 @@ public BazelPackageLoader buildImpl() { SkyFunctions.REPOSITORY_DIRECTORY, new RepositoryDelegatorFunction( BazelRepositoryModule.repositoryRules(), - new StarlarkRepositoryFunction(downloadManager), + starlarkRepositoryFunction, isFetch, ImmutableMap::of, directories, diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisMock.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisMock.java index c7a16c496bf52e..0e5775d103c90a 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisMock.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisMock.java @@ -36,7 +36,6 @@ import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.BazelCompatibilityMode; import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.CheckDirectDepsMode; import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.LockfileMode; -import com.google.devtools.build.lib.bazel.repository.downloader.DownloadManager; import com.google.devtools.build.lib.bazel.repository.starlark.StarlarkRepositoryFunction; import com.google.devtools.build.lib.bazel.rules.android.AndroidNdkRepositoryFunction; import com.google.devtools.build.lib.bazel.rules.android.AndroidNdkRepositoryRule; @@ -65,7 +64,6 @@ import java.util.Optional; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; -import org.mockito.Mockito; /** Create a mock client for the analysis phase, as well as a configuration factory. */ public abstract class AnalysisMock extends LoadingMock { @@ -157,14 +155,12 @@ public ImmutableMap getSkyFunctions(BlazeDirectori addExtraRepositoryFunctions(repositoryHandlers); - DownloadManager downloadManager = Mockito.mock(DownloadManager.class); - return ImmutableMap.builder() .put( SkyFunctions.REPOSITORY_DIRECTORY, new RepositoryDelegatorFunction( repositoryHandlers.buildKeepingLast(), - new StarlarkRepositoryFunction(downloadManager), + new StarlarkRepositoryFunction(), new AtomicBoolean(true), ImmutableMap::of, directories, @@ -181,7 +177,7 @@ public ImmutableMap getSkyFunctions(BlazeDirectori .put(SkyFunctions.SINGLE_EXTENSION, new SingleExtensionFunction()) .put( SkyFunctions.SINGLE_EXTENSION_EVAL, - new SingleExtensionEvalFunction(directories, ImmutableMap::of, downloadManager)) + new SingleExtensionEvalFunction(directories, ImmutableMap::of)) .put(SkyFunctions.SINGLE_EXTENSION_USAGES, new SingleExtensionUsagesFunction()) .put( SkyFunctions.REGISTRY, diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunctionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunctionTest.java index 38ee7f4d338e29..ab04bb4c29cf10 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunctionTest.java @@ -31,7 +31,6 @@ import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.BazelCompatibilityMode; import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.CheckDirectDepsMode; import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.LockfileMode; -import com.google.devtools.build.lib.bazel.repository.downloader.DownloadManager; import com.google.devtools.build.lib.bazel.repository.starlark.StarlarkRepositoryFunction; import com.google.devtools.build.lib.bazel.repository.starlark.StarlarkRepositoryModule; import com.google.devtools.build.lib.clock.BlazeClock; @@ -76,7 +75,6 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; -import org.mockito.Mockito; /** Tests for {@link BazelModuleResolutionFunction}. */ @RunWith(JUnit4.class) @@ -121,9 +119,7 @@ public void setup() throws Exception { ConfiguredRuleClassProvider ruleClassProvider = builder.build(); ImmutableMap repositoryHandlers = ImmutableMap.of(LocalRepositoryRule.NAME, new LocalRepositoryFunction()); - DownloadManager downloadManager = Mockito.mock(DownloadManager.class); - StarlarkRepositoryFunction starlarkRepositoryFunction = - new StarlarkRepositoryFunction(downloadManager); + StarlarkRepositoryFunction starlarkRepositoryFunction = new StarlarkRepositoryFunction(); evaluator = new InMemoryMemoizingEvaluator( diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/IndexRegistryTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/IndexRegistryTest.java index f542786737a1cf..1aac36dee9e7c1 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/IndexRegistryTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/IndexRegistryTest.java @@ -78,7 +78,7 @@ public ImmutableMap> getRecordedHashes() { @Rule public final TestHttpServer server = new TestHttpServer(authToken); @Rule public final TemporaryFolder tempFolder = new TemporaryFolder(); - private RegistryFactory registryFactory; + private RegistryFactoryImpl registryFactory; private RepositoryCache repositoryCache; @Before @@ -86,9 +86,10 @@ public void setUp() throws Exception { eventRecorder = new EventRecorder(); eventBus.register(eventRecorder); repositoryCache = new RepositoryCache(); - downloadManager = new DownloadManager(repositoryCache, new HttpDownloader()); - registryFactory = - new RegistryFactoryImpl(downloadManager, Suppliers.ofInstance(ImmutableMap.of())); + HttpDownloader httpDownloader = new HttpDownloader(); + downloadManager = new DownloadManager(repositoryCache, httpDownloader, httpDownloader); + registryFactory = new RegistryFactoryImpl(Suppliers.ofInstance(ImmutableMap.of())); + registryFactory.setDownloadManager(downloadManager); } @Test diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java index e722cae92b417e..582fa9a2a3066a 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java @@ -34,7 +34,6 @@ import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.BazelCompatibilityMode; import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.CheckDirectDepsMode; import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.LockfileMode; -import com.google.devtools.build.lib.bazel.repository.downloader.DownloadManager; import com.google.devtools.build.lib.bazel.repository.starlark.StarlarkRepositoryFunction; import com.google.devtools.build.lib.bazel.repository.starlark.StarlarkRepositoryModule; import com.google.devtools.build.lib.clock.BlazeClock; @@ -106,7 +105,6 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; -import org.mockito.Mockito; /** Tests for module extension resolution. */ @RunWith(JUnit4.class) @@ -163,10 +161,7 @@ public void setup() throws Exception { .getPackageFactoryBuilderForTesting(directories) .build(ruleClassProvider, fileSystem); HashFunction hashFunction = fileSystem.getDigestFunction().getHashFunction(); - - DownloadManager downloadManager = Mockito.mock(DownloadManager.class); - StarlarkRepositoryFunction starlarkRepositoryFunction = - new StarlarkRepositoryFunction(downloadManager); + StarlarkRepositoryFunction starlarkRepositoryFunction = new StarlarkRepositoryFunction(); ImmutableMap repositoryHandlers = ImmutableMap.of(LocalRepositoryRule.NAME, new LocalRepositoryFunction()); @@ -267,7 +262,7 @@ public void setup() throws Exception { .put(SkyFunctions.SINGLE_EXTENSION, new SingleExtensionFunction()) .put( SkyFunctions.SINGLE_EXTENSION_EVAL, - new SingleExtensionEvalFunction(directories, ImmutableMap::of, downloadManager)) + new SingleExtensionEvalFunction(directories, ImmutableMap::of)) .put( SkyFunctions.REGISTRY, new RegistryFunction(registryFactory, directories.getWorkspace())) diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/RegistryFactoryTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/RegistryFactoryTest.java index fb52c7fe216416..d17dafc4ef982f 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/RegistryFactoryTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/RegistryFactoryTest.java @@ -21,9 +21,6 @@ import com.google.common.base.Suppliers; import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.LockfileMode; -import com.google.devtools.build.lib.bazel.repository.cache.RepositoryCache; -import com.google.devtools.build.lib.bazel.repository.downloader.DownloadManager; -import com.google.devtools.build.lib.bazel.repository.downloader.HttpDownloader; import java.net.URISyntaxException; import java.util.Optional; import org.junit.Test; @@ -37,9 +34,7 @@ public class RegistryFactoryTest { @Test public void badSchemes() { RegistryFactory registryFactory = - new RegistryFactoryImpl( - new DownloadManager(new RepositoryCache(), new HttpDownloader()), - Suppliers.ofInstance(ImmutableMap.of())); + new RegistryFactoryImpl(Suppliers.ofInstance(ImmutableMap.of())); Throwable exception = assertThrows( URISyntaxException.class, @@ -67,9 +62,7 @@ public void badSchemes() { @Test public void badPath() { RegistryFactory registryFactory = - new RegistryFactoryImpl( - new DownloadManager(new RepositoryCache(), new HttpDownloader()), - Suppliers.ofInstance(ImmutableMap.of())); + new RegistryFactoryImpl(Suppliers.ofInstance(ImmutableMap.of())); Throwable exception = assertThrows( URISyntaxException.class, diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpDownloaderTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpDownloaderTest.java index 84a862ea85a04e..d998e17a49a678 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpDownloaderTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpDownloaderTest.java @@ -45,6 +45,7 @@ import java.net.SocketTimeoutException; import java.net.URI; import java.net.URL; +import java.time.Duration; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -73,9 +74,10 @@ public class HttpDownloaderTest { @Rule public final Timeout timeout = new Timeout(30, SECONDS); private final RepositoryCache repositoryCache = mock(RepositoryCache.class); - private final HttpDownloader httpDownloader = new HttpDownloader(); + // Scale timeouts down to make test fast. + private final HttpDownloader httpDownloader = new HttpDownloader(0, Duration.ZERO, 8, .1f); private final DownloadManager downloadManager = - new DownloadManager(repositoryCache, httpDownloader); + new DownloadManager(repositoryCache, httpDownloader, httpDownloader); private final ExecutorService executor = Executors.newFixedThreadPool(2); private final ExtendedEventHandler eventHandler = mock(ExtendedEventHandler.class); @@ -83,9 +85,6 @@ public class HttpDownloaderTest { public HttpDownloaderTest() { fs = new JavaIoFileSystem(DigestHashFunction.SHA256); - - // Scale timeouts down to make tests fast. - httpDownloader.setTimeoutScaling(0.1f); } @After @@ -646,7 +645,9 @@ public void downloadAndReadOneUrl_checksumMismatch() throws IOException { @Test public void download_contentLengthMismatch_propagateErrorIfNotRetry() throws Exception { Downloader downloader = mock(Downloader.class); - DownloadManager downloadManager = new DownloadManager(repositoryCache, downloader); + HttpDownloader httpDownloader = mock(HttpDownloader.class); + DownloadManager downloadManager = + new DownloadManager(repositoryCache, downloader, httpDownloader); // do not retry downloadManager.setRetries(0); AtomicInteger times = new AtomicInteger(0); @@ -682,9 +683,11 @@ public void download_contentLengthMismatch_propagateErrorIfNotRetry() throws Exc @Test public void download_contentLengthMismatch_retries() throws Exception { Downloader downloader = mock(Downloader.class); - int retires = 5; - DownloadManager downloadManager = new DownloadManager(repositoryCache, downloader); - downloadManager.setRetries(retires); + HttpDownloader httpDownloader = mock(HttpDownloader.class); + int retries = 5; + DownloadManager downloadManager = + new DownloadManager(repositoryCache, downloader, httpDownloader); + downloadManager.setRetries(retries); AtomicInteger times = new AtomicInteger(0); byte[] data = "content".getBytes(UTF_8); doAnswer( @@ -725,9 +728,11 @@ public void download_contentLengthMismatch_retries() throws Exception { @Test public void download_contentLengthMismatchWithOtherErrors_retries() throws Exception { Downloader downloader = mock(Downloader.class); - int retires = 5; - DownloadManager downloadManager = new DownloadManager(repositoryCache, downloader); - downloadManager.setRetries(retires); + HttpDownloader httpDownloader = mock(HttpDownloader.class); + int retries = 5; + DownloadManager downloadManager = + new DownloadManager(repositoryCache, downloader, httpDownloader); + downloadManager.setRetries(retries); AtomicInteger times = new AtomicInteger(0); byte[] data = "content".getBytes(UTF_8); doAnswer( diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteModuleTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteModuleTest.java index 7026150cbe11f6..8d260d8ad9a507 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteModuleTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteModuleTest.java @@ -148,6 +148,7 @@ private static CommandEnvironment createTestCommandEnvironment( ServerDirectories serverDirectories = new ServerDirectories( scratch.dir("install"), scratch.dir("output"), scratch.dir("user_root")); + BlazeRuntime runtime = new BlazeRuntime.Builder() .setProductName(productName) @@ -237,7 +238,7 @@ public void testVerifyCapabilities_none() throws Exception { beforeCommand(); // Wait for the channel to be connected. - var downloader = (GrpcRemoteDownloader) remoteModule.getRemoteDownloaderSupplier().get(); + var downloader = (GrpcRemoteDownloader) remoteModule.getRemoteDownloader(); var unused = downloader.getChannel().withChannelBlocking(ch -> new Object()); // Remote downloader uses Remote Asset API, and Bazel doesn't have any capability requirement diff --git a/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java b/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java index bc176a5b776d94..7dd7d6159d4108 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java @@ -44,7 +44,6 @@ import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.BazelCompatibilityMode; import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.CheckDirectDepsMode; import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.LockfileMode; -import com.google.devtools.build.lib.bazel.repository.downloader.DownloadManager; import com.google.devtools.build.lib.bazel.repository.starlark.StarlarkRepositoryFunction; import com.google.devtools.build.lib.bazel.repository.starlark.StarlarkRepositoryModule; import com.google.devtools.build.lib.clock.BlazeClock; @@ -108,7 +107,6 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; -import org.mockito.Mockito; /** Tests for {@link RepositoryDelegatorFunction} */ @RunWith(JUnit4.class) @@ -132,14 +130,13 @@ public void setupDelegator() throws Exception { rootPath, /* defaultSystemJavabase= */ null, TestConstants.PRODUCT_NAME); - DownloadManager downloader = Mockito.mock(DownloadManager.class); RepositoryFunction localRepositoryFunction = new LocalRepositoryFunction(); ImmutableMap repositoryHandlers = ImmutableMap.of(LocalRepositoryRule.NAME, localRepositoryFunction); RepositoryDelegatorFunction delegatorFunction = new RepositoryDelegatorFunction( repositoryHandlers, - new StarlarkRepositoryFunction(downloader), + new StarlarkRepositoryFunction(), /* isFetch= */ new AtomicBoolean(true), /* clientEnvironmentSupplier= */ ImmutableMap::of, directories,