From b19184751bbd63e36fbf847cb0fe4f019f573c47 Mon Sep 17 00:00:00 2001 From: Juuz <6596629+Juuxel@users.noreply.github.com> Date: Sat, 18 Feb 2023 20:53:49 +0200 Subject: [PATCH 1/4] Add toString to ModSettings and RemapConfigurationSettings (#831) This helps a bit with debugging code that uses them as you don't have to open the object to see which mod/remap configuration we're investigating. --- src/main/java/net/fabricmc/loom/api/ModSettings.java | 5 +++++ .../net/fabricmc/loom/api/RemapConfigurationSettings.java | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/src/main/java/net/fabricmc/loom/api/ModSettings.java b/src/main/java/net/fabricmc/loom/api/ModSettings.java index 03cdb070..7df3134c 100644 --- a/src/main/java/net/fabricmc/loom/api/ModSettings.java +++ b/src/main/java/net/fabricmc/loom/api/ModSettings.java @@ -122,4 +122,9 @@ public abstract class ModSettings implements Named { @Inject public abstract Project getProject(); + + @Override + public String toString() { + return "ModSettings '" + getName() + "'"; + } } diff --git a/src/main/java/net/fabricmc/loom/api/RemapConfigurationSettings.java b/src/main/java/net/fabricmc/loom/api/RemapConfigurationSettings.java index 8eb0aeeb..a4fe759a 100644 --- a/src/main/java/net/fabricmc/loom/api/RemapConfigurationSettings.java +++ b/src/main/java/net/fabricmc/loom/api/RemapConfigurationSettings.java @@ -142,4 +142,9 @@ public abstract class RemapConfigurationSettings implements Named { private Provider defaultDependencyTransforms() { return getSourceSet().map(sourceSet -> sourceSet.getName().equals(SourceSet.MAIN_SOURCE_SET_NAME) || sourceSet.getName().equals("client")); } + + @Override + public String toString() { + return "RemapConfigurationSettings '" + getName() + "'"; + } } From ab114b5d7a65f934762476c636a3850004d04a04 Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Sat, 18 Feb 2023 18:54:24 +0000 Subject: [PATCH 2/4] Fallback to HTTP 1.1 on the last retry when downloading. (#829) --- .../fabricmc/loom/util/download/Download.java | 37 +++++++++++++++---- .../loom/util/download/DownloadBuilder.java | 34 +++++++++++------ .../unit/download/DownloadFileTest.groovy | 5 ++- 3 files changed, 56 insertions(+), 20 deletions(-) diff --git a/src/main/java/net/fabricmc/loom/util/download/Download.java b/src/main/java/net/fabricmc/loom/util/download/Download.java index 785ac7ac..efbde99e 100644 --- a/src/main/java/net/fabricmc/loom/util/download/Download.java +++ b/src/main/java/net/fabricmc/loom/util/download/Download.java @@ -42,6 +42,7 @@ import java.nio.charset.StandardCharsets; import java.nio.file.FileSystems; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.StandardOpenOption; import java.nio.file.attribute.BasicFileAttributeView; import java.nio.file.attribute.FileTime; import java.time.Duration; @@ -58,7 +59,7 @@ import org.slf4j.LoggerFactory; import net.fabricmc.loom.util.AttributeHelper; import net.fabricmc.loom.util.Checksum; -public class Download { +public final class Download { private static final String E_TAG = "ETag"; private static final Logger LOGGER = LoggerFactory.getLogger(Download.class); @@ -73,8 +74,10 @@ public class Download { private final boolean offline; private final Duration maxAge; private final DownloadProgressListener progressListener; + private final HttpClient.Version httpVersion; + private final int downloadAttempt; - Download(URI url, String expectedHash, boolean useEtag, boolean forceDownload, boolean offline, Duration maxAge, DownloadProgressListener progressListener) { + Download(URI url, String expectedHash, boolean useEtag, boolean forceDownload, boolean offline, Duration maxAge, DownloadProgressListener progressListener, HttpClient.Version httpVersion, int downloadAttempt) { this.url = url; this.expectedHash = expectedHash; this.useEtag = useEtag; @@ -82,6 +85,8 @@ public class Download { this.offline = offline; this.maxAge = maxAge; this.progressListener = progressListener; + this.httpVersion = httpVersion; + this.downloadAttempt = downloadAttempt; } private HttpClient getHttpClient() throws DownloadException { @@ -97,12 +102,14 @@ public class Download { private HttpRequest getRequest() { return HttpRequest.newBuilder(url) + .version(httpVersion) .GET() .build(); } private HttpRequest getETagRequest(String etag) { return HttpRequest.newBuilder(url) + .version(httpVersion) .GET() .header("If-None-Match", etag) .build(); @@ -148,7 +155,7 @@ public class Download { doDownload(output); } catch (Throwable throwable) { tryCleanup(output); - throw error(throwable, "Failed to download (%s) to (%s)", url, output); + throw error(throwable, "Failed to download file from (%s) to (%s)", url, output); } finally { progressListener.onEnd(); } @@ -194,7 +201,7 @@ public class Download { final long length = Long.parseLong(response.headers().firstValue("Content-Length").orElse("-1")); AtomicLong totalBytes = new AtomicLong(0); - try (OutputStream outputStream = Files.newOutputStream(output)) { + try (OutputStream outputStream = Files.newOutputStream(output, StandardOpenOption.CREATE_NEW)) { copyWithCallback(decodeOutput(response), outputStream, value -> { if (length < 0) { return; @@ -203,12 +210,26 @@ public class Download { progressListener.onProgress(totalBytes.addAndGet(value), length); }); } catch (IOException e) { - tryCleanup(output); throw error(e, "Failed to decode and write download output"); } + + if (Files.notExists(output)) { + throw error("No file was downloaded"); + } + + if (length > 0) { + try { + final long actualLength = Files.size(output); + + if (actualLength != length) { + throw error("Unexpected file length of %d bytes, expected %d bytes".formatted(actualLength, length)); + } + } catch (IOException e) { + throw error(e); + } + } } else { - tryCleanup(output); - throw error("HTTP request to (%s) returned unsuccessful status (%d)", url, statusCode); + throw error("HTTP request returned unsuccessful status (%d)", statusCode); } if (useEtag) { @@ -261,7 +282,7 @@ public class Download { } private boolean requiresDownload(Path output) throws DownloadException { - if (getAndResetLock(output)) { + if (getAndResetLock(output) & downloadAttempt == 1) { LOGGER.warn("Forcing downloading {} as existing lock file was found. This may happen if the gradle build was forcefully canceled.", output); return true; } diff --git a/src/main/java/net/fabricmc/loom/util/download/DownloadBuilder.java b/src/main/java/net/fabricmc/loom/util/download/DownloadBuilder.java index 0d3949a2..c2fd6679 100644 --- a/src/main/java/net/fabricmc/loom/util/download/DownloadBuilder.java +++ b/src/main/java/net/fabricmc/loom/util/download/DownloadBuilder.java @@ -27,6 +27,7 @@ package net.fabricmc.loom.util.download; import java.io.IOException; import java.net.URI; import java.net.URISyntaxException; +import java.net.http.HttpClient; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; @@ -46,6 +47,7 @@ public class DownloadBuilder { private DownloadProgressListener progressListener = DownloadProgressListener.NONE; private int maxRetries = 3; private boolean allowInsecureProtocol = false; + private HttpClient.Version httpVersion = HttpClient.Version.HTTP_2; private DownloadBuilder(URI url) { this.url = url; @@ -100,12 +102,17 @@ public class DownloadBuilder { return this; } - private Download build() { + public DownloadBuilder httpVersion(HttpClient.Version httpVersion) { + this.httpVersion = httpVersion; + return this; + } + + private Download build(int downloadAttempt) { if (!allowInsecureProtocol && !isSecureUrl(url)) { throw new IllegalArgumentException("Cannot create download for url (%s) with insecure protocol".formatted(url.toString())); } - return new Download(this.url, this.expectedHash, this.useEtag, this.forceDownload, this.offline, maxAge, progressListener); + return new Download(this.url, this.expectedHash, this.useEtag, this.forceDownload, this.offline, maxAge, progressListener, httpVersion, downloadAttempt); } public void downloadPathAsync(Path path, DownloadExecutor executor) { @@ -113,19 +120,19 @@ public class DownloadBuilder { } public void downloadPath(Path path) throws DownloadException { - withRetries(() -> { - build().downloadPath(path); + withRetries((download) -> { + download.downloadPath(path); return null; }); } public String downloadString() throws DownloadException { - return withRetries(() -> build().downloadString()); + return withRetries(Download::downloadString); } public String downloadString(Path cache) throws DownloadException { - return withRetries(() -> { - build().downloadPath(cache); + return withRetries((download) -> { + download.downloadPath(cache); try { return Files.readString(cache, StandardCharsets.UTF_8); @@ -141,10 +148,15 @@ public class DownloadBuilder { }); } - private T withRetries(DownloadSupplier supplier) throws DownloadException { + private T withRetries(DownloadFunction supplier) throws DownloadException { for (int i = 1; i <= maxRetries; i++) { try { - return supplier.get(); + if (i == maxRetries) { + // Last ditch attempt, try over HTTP 1.1 + httpVersion(HttpClient.Version.HTTP_1_1); + } + + return supplier.get(build(i)); } catch (DownloadException e) { if (i == maxRetries) { throw new DownloadException(String.format(Locale.ENGLISH, "Failed download after %d attempts", maxRetries), e); @@ -166,7 +178,7 @@ public class DownloadBuilder { } @FunctionalInterface - private interface DownloadSupplier { - T get() throws DownloadException; + private interface DownloadFunction { + T get(Download download) throws DownloadException; } } diff --git a/src/test/groovy/net/fabricmc/loom/test/unit/download/DownloadFileTest.groovy b/src/test/groovy/net/fabricmc/loom/test/unit/download/DownloadFileTest.groovy index d0c08942..39af96ce 100644 --- a/src/test/groovy/net/fabricmc/loom/test/unit/download/DownloadFileTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/unit/download/DownloadFileTest.groovy @@ -30,13 +30,16 @@ import net.fabricmc.loom.util.download.Download import net.fabricmc.loom.util.download.DownloadException import net.fabricmc.loom.util.download.DownloadExecutor import net.fabricmc.loom.util.download.DownloadProgressListener +import spock.lang.IgnoreIf + import java.nio.file.Files -import java.nio.file.attribute.FileTime import java.nio.file.Paths +import java.nio.file.attribute.FileTime import java.time.Duration import java.time.Instant class DownloadFileTest extends DownloadTest { + @IgnoreIf({ os.windows }) // Requires admin on windows. def "Directory: Symlink"() { setup: server.get("/symlinkFile") { From 4a11cbae07aca9653c749e0e55a7ece9a950c2a8 Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Sat, 18 Feb 2023 18:55:39 +0000 Subject: [PATCH 3/4] Test against Gradle 8 (#828) * Test against Gradle 8 * Update --- .../groovy/net/fabricmc/loom/test/LoomTestConstants.groovy | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/groovy/net/fabricmc/loom/test/LoomTestConstants.groovy b/src/test/groovy/net/fabricmc/loom/test/LoomTestConstants.groovy index 888bc32e..020b321d 100644 --- a/src/test/groovy/net/fabricmc/loom/test/LoomTestConstants.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/LoomTestConstants.groovy @@ -27,13 +27,13 @@ package net.fabricmc.loom.test import org.gradle.util.GradleVersion class LoomTestConstants { - private final static String NIGHTLY_VERSION = "8.1-20230119104422+0000" + private final static String NIGHTLY_VERSION = "8.1-20230217231705+0000" private final static boolean NIGHTLY_EXISTS = nightlyExists(NIGHTLY_VERSION) // Test against the version of Gradle being used to build loom public final static String DEFAULT_GRADLE = GradleVersion.current().getVersion() // Test against Gradle 8 - public final static String GRADLE_8 = "8.0-rc-2" + public final static String GRADLE_8 = "8.0.1" // Tests that depend specifically on the nightly will run on the current version when the nightly is not available. public final static String PRE_RELEASE_GRADLE = NIGHTLY_EXISTS ? NIGHTLY_VERSION : DEFAULT_GRADLE // Randomly sorted to ensure that all versions can run with a clean gradle home. From 83b968df64845b3a1183e51cb9bbed5579089585 Mon Sep 17 00:00:00 2001 From: Caoimhe <71222289+cbyrneee@users.noreply.github.com> Date: Sat, 18 Feb 2023 18:56:05 +0000 Subject: [PATCH 4/4] Warn when using `clientOnlyMinecraftJar()` on Minecraft versions newer than 1.3 (#824) Single JARs break things like source generation on newer versions and pretty much have no difference there, so we should log a warning, similar to what we do for merged JARs not being supported on 1.2.5 or older. --- .../providers/minecraft/SingleJarMinecraftProvider.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/SingleJarMinecraftProvider.java b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/SingleJarMinecraftProvider.java index e63a5ac2..50e4cdd9 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/SingleJarMinecraftProvider.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/SingleJarMinecraftProvider.java @@ -68,6 +68,11 @@ public final class SingleJarMinecraftProvider extends MinecraftProvider { public void provide() throws Exception { super.provide(); + // Server only JARs are supported on any version, client only JARs are pretty much useless after 1.3. + if (provideClient() && getVersionInfo().isVersionOrNewer("2012-07-25T22:00:00+00:00" /* 1.3 release date */)) { + getProject().getLogger().warn("Using `clientOnlyMinecraftJar()` is not recommended for Minecraft versions 1.3 or newer."); + } + boolean requiresRefresh = getExtension().refreshDeps() || Files.notExists(minecraftEnvOnlyJar); if (!requiresRefresh) {