From 679026ef31d619409187a564bc0623891f5979e6 Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Thu, 4 Mar 2021 23:50:02 +0000 Subject: [PATCH] Minor cleanup/optimisations to downloading, should help a little bit more with asset downloading. Closes #359 --- .../providers/MinecraftProvider.java | 4 +- .../mappings/MojangMappingsDependency.java | 10 ++--- .../assets/MinecraftAssetsProvider.java | 44 ++++++++++--------- .../net/fabricmc/loom/util/DownloadUtil.java | 3 +- .../loom/util/HashedDownloadUtil.java | 25 ++++++----- 5 files changed, 48 insertions(+), 38 deletions(-) diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/MinecraftProvider.java b/src/main/java/net/fabricmc/loom/configuration/providers/MinecraftProvider.java index e25b8ca..508e8c3 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/MinecraftProvider.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/MinecraftProvider.java @@ -102,8 +102,8 @@ public class MinecraftProvider extends DependencyProvider { try { mergeJars(getProject().getLogger()); } catch (ZipError e) { - DownloadUtil.delete(minecraftClientJar); - DownloadUtil.delete(minecraftServerJar); + HashedDownloadUtil.delete(minecraftClientJar); + HashedDownloadUtil.delete(minecraftServerJar); getProject().getLogger().error("Could not merge JARs! Deleting source JARs - please re-run the command and move on.", e); throw new RuntimeException(); diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/MojangMappingsDependency.java b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/MojangMappingsDependency.java index 96c4c30..6287945 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/MojangMappingsDependency.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/MojangMappingsDependency.java @@ -56,7 +56,7 @@ import org.zeroturnaround.zip.ZipUtil; import net.fabricmc.loom.LoomGradleExtension; import net.fabricmc.loom.LoomGradlePlugin; import net.fabricmc.loom.configuration.providers.minecraft.MinecraftVersionMeta; -import net.fabricmc.loom.util.DownloadUtil; +import net.fabricmc.loom.util.HashedDownloadUtil; import net.fabricmc.lorenztiny.TinyMappingsReader; import net.fabricmc.mapping.tree.TinyMappingFactory; @@ -127,11 +127,11 @@ public class MojangMappingsDependency implements SelfResolvingDependency { throw new RuntimeException("Failed to find official mojang mappings for " + getVersion()); } - String clientMappingsUrl = versionInfo.getDownload(MANIFEST_CLIENT_MAPPINGS).getUrl(); - String serverMappingsUrl = versionInfo.getDownload(MANIFEST_CLIENT_MAPPINGS).getUrl(); + MinecraftVersionMeta.Download clientMappingsDownload = versionInfo.getDownload(MANIFEST_CLIENT_MAPPINGS); + MinecraftVersionMeta.Download serverMappingsDownload = versionInfo.getDownload(MANIFEST_CLIENT_MAPPINGS); - DownloadUtil.downloadIfChanged(new URL(clientMappingsUrl), clientMappings.toFile(), project.getLogger()); - DownloadUtil.downloadIfChanged(new URL(serverMappingsUrl), serverMappings.toFile(), project.getLogger()); + HashedDownloadUtil.downloadIfInvalid(new URL(clientMappingsDownload.getUrl()), clientMappings.toFile(), clientMappingsDownload.getSha1(), project.getLogger(), false); + HashedDownloadUtil.downloadIfInvalid(new URL(serverMappingsDownload.getUrl()), serverMappings.toFile(), clientMappingsDownload.getSha1(), project.getLogger(), false); MappingSet mappings = MappingSet.create(); diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/assets/MinecraftAssetsProvider.java b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/assets/MinecraftAssetsProvider.java index 64722c6..97e8503 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/assets/MinecraftAssetsProvider.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/assets/MinecraftAssetsProvider.java @@ -105,35 +105,39 @@ public class MinecraftAssetsProvider { } } else { executor.execute(() -> { - ProgressLogger progressLogger; - - if (loggers.isEmpty()) { - //Create a new logger if we need one - progressLogger = ProgressLogger.getProgressFactory(project, MinecraftAssetsProvider.class.getName()); - progressLogger.start("Downloading assets...", "assets"); - } else { - // use a free logger if we can - progressLogger = loggers.pop(); - } - - String assetName = entry.getKey(); - int end = assetName.lastIndexOf("/") + 1; + final String[] assetName = {entry.getKey()}; + int end = assetName[0].lastIndexOf("/") + 1; if (end > 0) { - assetName = assetName.substring(end); + assetName[0] = assetName[0].substring(end); } - project.getLogger().debug(":downloading asset " + assetName); - progressLogger.progress(String.format("%-30.30s", assetName) + " - " + sha1); + project.getLogger().debug("validating asset " + assetName[0]); + + final ProgressLogger[] progressLogger = new ProgressLogger[1]; try { - HashedDownloadUtil.downloadIfInvalid(new URL(Constants.RESOURCES_BASE + sha1.substring(0, 2) + "/" + sha1), file, sha1, project.getLogger(), true); + HashedDownloadUtil.downloadIfInvalid(new URL(Constants.RESOURCES_BASE + sha1.substring(0, 2) + "/" + sha1), file, sha1, project.getLogger(), true, () -> { + if (loggers.isEmpty()) { + //Create a new logger if we need one + progressLogger[0] = ProgressLogger.getProgressFactory(project, MinecraftAssetsProvider.class.getName()); + progressLogger[0].start("Downloading assets...", "assets"); + } else { + // use a free logger if we can + progressLogger[0] = loggers.pop(); + } + + project.getLogger().debug("downloading asset " + assetName[0]); + progressLogger[0].progress(String.format("%-30.30s", assetName[0]) + " - " + sha1); + }); } catch (IOException e) { - throw new RuntimeException("Failed to download: " + assetName, e); + throw new RuntimeException("Failed to download: " + assetName[0], e); } - //Give this logger back - loggers.add(progressLogger); + if (progressLogger[0] != null) { + //Give this logger back if we used it + loggers.add(progressLogger[0]); + } }); } } diff --git a/src/main/java/net/fabricmc/loom/util/DownloadUtil.java b/src/main/java/net/fabricmc/loom/util/DownloadUtil.java index 9bb2b7e..90131b4 100644 --- a/src/main/java/net/fabricmc/loom/util/DownloadUtil.java +++ b/src/main/java/net/fabricmc/loom/util/DownloadUtil.java @@ -89,6 +89,7 @@ public class DownloadUtil { if ((code < 200 || code > 299) && code != HttpURLConnection.HTTP_NOT_MODIFIED) { //Didn't get what we expected + delete(to); throw new IOException(connection.getResponseMessage() + " for " + from); } @@ -111,7 +112,7 @@ public class DownloadUtil { try { // Try download to the output FileUtils.copyInputStreamToFile(connection.getInputStream(), to); } catch (IOException e) { - to.delete(); // Probably isn't good if it fails to copy/save + delete(to); // Probably isn't good if it fails to copy/save throw e; } diff --git a/src/main/java/net/fabricmc/loom/util/HashedDownloadUtil.java b/src/main/java/net/fabricmc/loom/util/HashedDownloadUtil.java index da294b8..86cf80a 100644 --- a/src/main/java/net/fabricmc/loom/util/HashedDownloadUtil.java +++ b/src/main/java/net/fabricmc/loom/util/HashedDownloadUtil.java @@ -25,6 +25,7 @@ package net.fabricmc.loom.util; import java.io.File; +import java.io.FileNotFoundException; import java.io.IOException; import java.net.HttpURLConnection; import java.net.URL; @@ -41,19 +42,23 @@ import net.fabricmc.loom.LoomGradlePlugin; public class HashedDownloadUtil { public static void downloadIfInvalid(URL from, File to, String expectedHash, Logger logger, boolean quiet) throws IOException { + downloadIfInvalid(from, to, expectedHash, logger, quiet, () -> { }); + } + + public static void downloadIfInvalid(URL from, File to, String expectedHash, Logger logger, boolean quiet, Runnable startDownload) throws IOException { if (LoomGradlePlugin.refreshDeps) { delete(to); } - if (to.exists()) { - String sha1 = getSha1(to, logger); + String sha1 = getSha1(to, logger); - if (expectedHash.equals(sha1)) { - // The hash in the sha1 file matches - return; - } + if (expectedHash.equals(sha1)) { + // The hash in the sha1 file matches + return; } + startDownload.run(); + HttpURLConnection connection = (HttpURLConnection) from.openConnection(); connection.setRequestProperty("Accept-Encoding", "gzip"); connection.connect(); @@ -62,6 +67,7 @@ public class HashedDownloadUtil { if ((code < 200 || code > 299) && code != HttpURLConnection.HTTP_NOT_MODIFIED) { //Didn't get what we expected + delete(to); throw new IOException(connection.getResponseMessage() + " for " + from); } @@ -96,12 +102,11 @@ public class HashedDownloadUtil { private static String getSha1(File to, Logger logger) { File sha1File = getSha1File(to); - if (!sha1File.exists()) { - return null; - } - try { return Files.asCharSource(sha1File, StandardCharsets.UTF_8).read(); + } catch (FileNotFoundException ignored) { + // Quicker to catch this than do an exists check before. + return null; } catch (IOException e) { logger.warn("Error reading sha1 file '{}'.", sha1File); return null;