From ce1f0b2ef926dc08d661757e484b5b8ad6a2ba5b Mon Sep 17 00:00:00 2001 From: Sebastian Hartte Date: Wed, 6 Dec 2023 02:39:53 +0100 Subject: [PATCH 1/2] Fixes #66 by using upToDateWhen to indicate the launcher manifest download task is never up-to-date. To mitigate the re-download, use If-Modified-Since in all HTTP download requests if the target file already exists. --- .../common/tasks/FileCacheProviding.java | 4 +- .../MinecraftLauncherFileCacheProvider.java | 5 +- .../common/util/FileDownloadingUtils.java | 99 ++++++++++++++++--- 3 files changed, 93 insertions(+), 15 deletions(-) diff --git a/common/src/main/java/net/neoforged/gradle/common/tasks/FileCacheProviding.java b/common/src/main/java/net/neoforged/gradle/common/tasks/FileCacheProviding.java index 67fa30734..37cf636fb 100644 --- a/common/src/main/java/net/neoforged/gradle/common/tasks/FileCacheProviding.java +++ b/common/src/main/java/net/neoforged/gradle/common/tasks/FileCacheProviding.java @@ -56,7 +56,9 @@ protected FileCacheProviding() { protected void downloadJsonTo(String url) { final File output = getOutput().get().getAsFile(); - FileDownloadingUtils.downloadThrowing(getIsOffline().get(), new FileDownloadingUtils.DownloadInfo(url, null, "json", null, null), output); + FileDownloadingUtils.DownloadInfo info = new FileDownloadingUtils.DownloadInfo(url, null, "json", null, null); + boolean didWork = FileDownloadingUtils.downloadThrowing(getIsOffline().get(), info, output); + setDidWork(didWork); } protected void doDownloadVersionDownloadToCache(final String artifact, final String potentialError, File versionManifest) { diff --git a/common/src/main/java/net/neoforged/gradle/common/tasks/MinecraftLauncherFileCacheProvider.java b/common/src/main/java/net/neoforged/gradle/common/tasks/MinecraftLauncherFileCacheProvider.java index 894bace8c..7812210aa 100644 --- a/common/src/main/java/net/neoforged/gradle/common/tasks/MinecraftLauncherFileCacheProvider.java +++ b/common/src/main/java/net/neoforged/gradle/common/tasks/MinecraftLauncherFileCacheProvider.java @@ -3,13 +3,12 @@ import net.neoforged.gradle.dsl.common.util.CacheFileSelector; import net.neoforged.gradle.util.UrlConstants; import org.gradle.api.tasks.TaskAction; -import org.gradle.work.DisableCachingByDefault; -@DisableCachingByDefault(because = "This can change at any point in time.") public abstract class MinecraftLauncherFileCacheProvider extends FileCacheProviding { - public MinecraftLauncherFileCacheProvider() { getSelector().set(CacheFileSelector.launcherMetadata()); + // This can change at any point in time. + getOutputs().upToDateWhen(element -> false); } @TaskAction diff --git a/common/src/main/java/net/neoforged/gradle/common/util/FileDownloadingUtils.java b/common/src/main/java/net/neoforged/gradle/common/util/FileDownloadingUtils.java index 5ecae445e..a9fb795b2 100644 --- a/common/src/main/java/net/neoforged/gradle/common/util/FileDownloadingUtils.java +++ b/common/src/main/java/net/neoforged/gradle/common/util/FileDownloadingUtils.java @@ -1,14 +1,21 @@ package net.neoforged.gradle.common.util; import net.neoforged.gradle.util.HashFunction; -import org.apache.commons.io.FileUtils; -import org.gradle.api.Project; import org.gradle.api.tasks.Input; import org.gradle.api.tasks.Optional; import javax.annotation.Nullable; -import java.io.*; +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.io.Serializable; +import java.net.HttpURLConnection; import java.net.URL; +import java.nio.file.AtomicMoveNotSupportedException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.nio.file.attribute.FileTime; public final class FileDownloadingUtils { @@ -17,33 +24,103 @@ private FileDownloadingUtils() { } - public static void downloadThrowing(boolean isOffline, FileDownloadingUtils.DownloadInfo info, File file) { + public static boolean downloadThrowing(boolean isOffline, FileDownloadingUtils.DownloadInfo info, File file) { try { - downloadTo(isOffline, info, file); + return downloadTo(isOffline, info, file); } catch (IOException e) { - throw new RuntimeException(String.format("Failed to download the file from: %s to: %s", info, file), e); + throw new RuntimeException(String.format("Failed to download the file from: %s to: %s", info.url, file), e); } } - public static void downloadTo(boolean isOffline, DownloadInfo info, File file) throws IOException { + /** + * @return True if a file was downloaded, false if the file was already up-to-date. + */ + public static boolean downloadTo(boolean isOffline, DownloadInfo info, File file) throws IOException { // Check if file exists in local installer cache if (info.type.equals("jar") && info.side.equals("client")) { File localPath = new File(getMCDir() + File.separator + "versions" + File.separator + info.version + File.separator + info.version + ".jar"); if (localPath.exists() && HashFunction.SHA1.hash(localPath).equalsIgnoreCase(info.hash)) { org.apache.commons.io.FileUtils.copyFile(localPath, file); - return; + return true; } } if (!isOffline) { - FileUtils.copyURLToFile(new URL(info.url), file); + return copyURLToFileIfNewer(new URL(info.url), file.toPath()); } else if (!file.exists()) { throw new RuntimeException("Could not find the file: " + file + " and we are offline."); + } else { + return false; // We're offline and the file exists } } - public static File getMCDir() - { + /** + * Downloads a file, but attempts to make a conditional request to only re-download if the file has been + * changed on the remote-server. + */ + private static boolean copyURLToFileIfNewer(URL url, Path target) throws IOException { + HttpURLConnection urlConnection = (HttpURLConnection) url.openConnection(); + + try { + // Do a Conditional If-Modified-Since request + if (Files.isRegularFile(target)) { + FileTime lastModified = Files.getLastModifiedTime(target); + urlConnection.setIfModifiedSince(lastModified.toMillis()); + + // Accessing the response code will cause the request to be sent + if (urlConnection.getResponseCode() == HttpURLConnection.HTTP_NOT_MODIFIED) { + // Double-Check here -> If the server also returns a last-modified date, + // and that is different from our local date, re-download! + // This could occur if the local file was modified and is now newer than the original. + if (urlConnection.getLastModified() != 0 && urlConnection.getLastModified() != urlConnection.getIfModifiedSince()) { + urlConnection.disconnect(); + urlConnection = (HttpURLConnection) url.openConnection(); + } else { + return false; + } + } + } + + if (urlConnection.getResponseCode() != 200) { + throw new IOException("Failed to download " + url + ", HTTP-Status: " + + urlConnection.getResponseCode()); + } + + // Resolve a relative path to get a proper parent directory + if (target.getParent() == null) { + target = target.toAbsolutePath(); + } + + // Always download to a temp-file to avoid partially downloaded files persisting a VM crash/shutdown + Path tempFile = Files.createTempFile(target.getParent(), target.getFileName().toString(), ".download"); + + try { + try (InputStream stream = urlConnection.getInputStream()) { + Files.copy(stream, tempFile, StandardCopyOption.REPLACE_EXISTING); + } + + try { + Files.move(tempFile, target, StandardCopyOption.ATOMIC_MOVE); + } catch (AtomicMoveNotSupportedException e) { + // Atomic moves within the same directory should have worked. + // We fall back to the inferior normal move. We should log this issue, but there is no logger here. + Files.move(tempFile, target, StandardCopyOption.REPLACE_EXISTING); + } + + if (urlConnection.getLastModified() != 0) { + Files.setLastModifiedTime(target, FileTime.fromMillis(urlConnection.getLastModified())); + } + + return true; + } finally { + Files.deleteIfExists(tempFile); + } + } finally { + urlConnection.disconnect(); + } + } + + public static File getMCDir() { switch (VersionJson.OS.getCurrent()) { case OSX: return new File(System.getProperty("user.home") + "/Library/Application Support/minecraft"); From a6a4475c648d1ac81170aa1dc41d8ab285b32536 Mon Sep 17 00:00:00 2001 From: Sebastian Hartte Date: Wed, 6 Dec 2023 02:51:29 +0100 Subject: [PATCH 2/2] Create the download directory if it doesn't exist. --- .../net/neoforged/gradle/common/util/FileDownloadingUtils.java | 1 + 1 file changed, 1 insertion(+) diff --git a/common/src/main/java/net/neoforged/gradle/common/util/FileDownloadingUtils.java b/common/src/main/java/net/neoforged/gradle/common/util/FileDownloadingUtils.java index a9fb795b2..d418519cb 100644 --- a/common/src/main/java/net/neoforged/gradle/common/util/FileDownloadingUtils.java +++ b/common/src/main/java/net/neoforged/gradle/common/util/FileDownloadingUtils.java @@ -92,6 +92,7 @@ private static boolean copyURLToFileIfNewer(URL url, Path target) throws IOExcep } // Always download to a temp-file to avoid partially downloaded files persisting a VM crash/shutdown + Files.createDirectories(target.getParent()); Path tempFile = Files.createTempFile(target.getParent(), target.getFileName().toString(), ".download"); try {