Ensure outputs are reproducable across all OS's. (#363)

6 files changed, 58 insertions, 23 deletions

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 97e8503c..931e8672 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
@@ -118,13 +118,15 @@ public class MinecraftAssetsProvider {
 
 					try {
 						HashedDownloadUtil.downloadIfInvalid(new URL(Constants.RESOURCES_BASE + sha1.substring(0, 2) + "/" + sha1), file, sha1, project.getLogger(), true, () -> {
-							if (loggers.isEmpty()) {
+							ProgressLogger logger = loggers.pollFirst();
+
+							if (logger == null) {
 								//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();
+								progressLogger[0] = logger;
 							}
 
 							project.getLogger().debug("downloading asset " + assetName[0]);
diff --git a/src/main/java/net/fabricmc/loom/task/RemapJarTask.java b/src/main/java/net/fabricmc/loom/task/RemapJarTask.java
index 2c0a3d6a..2e69f12a 100644
--- a/src/main/java/net/fabricmc/loom/task/RemapJarTask.java
+++ b/src/main/java/net/fabricmc/loom/task/RemapJarTask.java
@@ -62,6 +62,7 @@ import net.fabricmc.loom.configuration.providers.mappings.MappingsProvider;
 import net.fabricmc.loom.util.Constants;
 import net.fabricmc.loom.util.TinyRemapperMappingsHelper;
 import net.fabricmc.loom.util.gradle.GradleSupport;
+import net.fabricmc.loom.util.ZipReprocessorUtil;
 import net.fabricmc.stitch.util.Pair;
 import net.fabricmc.tinyremapper.TinyRemapper;
 import net.fabricmc.tinyremapper.TinyUtils;
@@ -173,6 +174,14 @@ public class RemapJarTask extends Jar {
 						boolean replaced = ZipUtil.replaceEntry(data.output.toFile(), accessWidener.getLeft(), accessWidener.getRight());
 						Preconditions.checkArgument(replaced, "Failed to remap access widener");
 					}
+
+					if (isReproducibleFileOrder() || !isPreserveFileTimestamps()) {
+						try {
+							ZipReprocessorUtil.reprocessZip(output.toFile(), isReproducibleFileOrder(), isPreserveFileTimestamps());
+						} catch (IOException e) {
+							throw new RuntimeException("Failed to re-process jar", e);
+						}
+					}
 				});
 	}
 
diff --git a/src/main/java/net/fabricmc/loom/task/RemapSourcesJarTask.java b/src/main/java/net/fabricmc/loom/task/RemapSourcesJarTask.java
index 2d388fba..9548ad7f 100644
--- a/src/main/java/net/fabricmc/loom/task/RemapSourcesJarTask.java
+++ b/src/main/java/net/fabricmc/loom/task/RemapSourcesJarTask.java
@@ -26,8 +26,6 @@ package net.fabricmc.loom.task;
 
 import java.io.File;
 
-import org.gradle.api.model.ObjectFactory;
-import org.gradle.api.provider.Property;
 import org.gradle.api.tasks.Input;
 import org.gradle.api.tasks.InputFile;
 import org.gradle.api.tasks.Internal;
@@ -35,29 +33,24 @@ import org.gradle.api.tasks.OutputFile;
 import org.gradle.api.tasks.TaskAction;
 
 import net.fabricmc.loom.util.SourceRemapper;
-import net.fabricmc.loom.util.ZipReprocessorUtil;
 
 public class RemapSourcesJarTask extends AbstractLoomTask {
 	private Object input;
 	private Object output;
 	private String direction = "intermediary";
 	private SourceRemapper sourceRemapper = null;
-	private final Property<Boolean> archivePreserveFileTimestamps;
-	private final Property<Boolean> archiveReproducibleFileOrder;
+	private boolean preserveFileTimestamps = true;
+	private boolean reproducibleFileOrder = false;
 
 	public RemapSourcesJarTask() {
-		ObjectFactory objectFactory = getProject().getObjects();
-		archivePreserveFileTimestamps = objectFactory.property(Boolean.class);
-		archiveReproducibleFileOrder = objectFactory.property(Boolean.class);
 	}
 
 	@TaskAction
 	public void remap() throws Exception {
 		if (sourceRemapper == null) {
-			SourceRemapper.remapSources(getProject(), getInput(), getOutput(), direction.equals("named"));
-			ZipReprocessorUtil.reprocessZip(getOutput(), archivePreserveFileTimestamps.getOrElse(true), archiveReproducibleFileOrder.getOrElse(false));
+			SourceRemapper.remapSources(getProject(), getInput(), getOutput(), direction.equals("named"), reproducibleFileOrder, preserveFileTimestamps);
 		} else {
-			sourceRemapper.scheduleRemapSources(getInput(), getOutput(), archivePreserveFileTimestamps.getOrElse(true), archiveReproducibleFileOrder.getOrElse(false));
+			sourceRemapper.scheduleRemapSources(getInput(), getOutput(), reproducibleFileOrder, preserveFileTimestamps);
 		}
 	}
 
@@ -97,4 +90,22 @@ public class RemapSourcesJarTask extends AbstractLoomTask {
 	public void setTargetNamespace(String value) {
 		this.direction = value;
 	}
+
+	@Input
+	public boolean isPreserveFileTimestamps() {
+		return preserveFileTimestamps;
+	}
+
+	public void setPreserveFileTimestamps(boolean preserveFileTimestamps) {
+		this.preserveFileTimestamps = preserveFileTimestamps;
+	}
+
+	@Input
+	public boolean isReproducibleFileOrder() {
+		return reproducibleFileOrder;
+	}
+
+	public void setReproducibleFileOrder(boolean reproducibleFileOrder) {
+		this.reproducibleFileOrder = reproducibleFileOrder;
+	}
 }
diff --git a/src/main/java/net/fabricmc/loom/util/OperatingSystem.java b/src/main/java/net/fabricmc/loom/util/OperatingSystem.java
index 2f69bcb8..db622790 100644
--- a/src/main/java/net/fabricmc/loom/util/OperatingSystem.java
+++ b/src/main/java/net/fabricmc/loom/util/OperatingSystem.java
@@ -49,6 +49,10 @@ public class OperatingSystem {
 		return System.getProperty("sun.arch.data.model").contains("64");
 	}
 
+	public static boolean isWindows() {
+		return getOS().equals("windows");
+	}
+
 	public static boolean isCIBuild() {
 		String loomProperty = System.getProperty("fabric.loom.ci");
 
diff --git a/src/main/java/net/fabricmc/loom/util/SourceRemapper.java b/src/main/java/net/fabricmc/loom/util/SourceRemapper.java
index 873b2e09..e673d0a7 100644
--- a/src/main/java/net/fabricmc/loom/util/SourceRemapper.java
+++ b/src/main/java/net/fabricmc/loom/util/SourceRemapper.java
@@ -62,17 +62,12 @@ public class SourceRemapper {
 		this.toNamed = toNamed;
 	}
 
-	public static void remapSources(Project project, File input, File output, boolean named) throws Exception {
+	public static void remapSources(Project project, File input, File output, boolean named, boolean reproducibleFileOrder, boolean preserveFileTimestamps) {
 		SourceRemapper sourceRemapper = new SourceRemapper(project, named);
-		sourceRemapper.scheduleRemapSources(input, output, false, true);
+		sourceRemapper.scheduleRemapSources(input, output, reproducibleFileOrder, preserveFileTimestamps);
 		sourceRemapper.remapAll();
 	}
 
-	@Deprecated
-	public void scheduleRemapSources(File source, File destination) throws Exception {
-		scheduleRemapSources(source, destination, false, true); // Not reproducable by default, old behavior
-	}
-
 	public void scheduleRemapSources(File source, File destination, boolean reproducibleFileOrder, boolean preserveFileTimestamps) {
 		remapTasks.add((logger) -> {
 			try {
diff --git a/src/main/java/net/fabricmc/loom/util/ZipReprocessorUtil.java b/src/main/java/net/fabricmc/loom/util/ZipReprocessorUtil.java
index 662acf82..735b3346 100644
--- a/src/main/java/net/fabricmc/loom/util/ZipReprocessorUtil.java
+++ b/src/main/java/net/fabricmc/loom/util/ZipReprocessorUtil.java
@@ -29,11 +29,20 @@ import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import java.nio.file.attribute.FileTime;
+import java.util.Calendar;
+import java.util.Comparator;
+import java.util.GregorianCalendar;
 import java.util.zip.ZipEntry;
 import java.util.zip.ZipFile;
 import java.util.zip.ZipOutputStream;
 
 public class ZipReprocessorUtil {
+	/**
+	 * See {@link org.gradle.api.internal.file.archive.ZipCopyAction} about this.
+	 */
+	private static final long CONSTANT_TIME_FOR_ZIP_ENTRIES = new GregorianCalendar(1980, Calendar.FEBRUARY, 1, 0, 0, 0).getTimeInMillis();
+
 	private ZipReprocessorUtil() { }
 
 	public static void reprocessZip(File file, boolean reproducibleFileOrder, boolean preserveFileTimestamps) throws IOException {
@@ -45,7 +54,7 @@ public class ZipReprocessorUtil {
 			ZipEntry[] entries;
 
 			if (reproducibleFileOrder) {
-				entries = zipFile.stream().sorted((a, b) -> a.getName().compareTo(b.getName())).toArray(ZipEntry[]::new);
+				entries = zipFile.stream().sorted(Comparator.comparing(ZipEntry::getName)).toArray(ZipEntry[]::new);
 			} else {
 				entries = zipFile.stream().toArray(ZipEntry[]::new);
 			}
@@ -54,11 +63,16 @@ public class ZipReprocessorUtil {
 
 			try (ZipOutputStream zipOutputStream = new ZipOutputStream(outZip)) {
 				for (ZipEntry entry : entries) {
+					ZipEntry newEntry = entry;
+
 					if (!preserveFileTimestamps) {
-						entry.setTime(0);
+						newEntry = new ZipEntry(entry.getName());
+						newEntry.setTime(CONSTANT_TIME_FOR_ZIP_ENTRIES);
+						newEntry.setLastModifiedTime(FileTime.fromMillis(CONSTANT_TIME_FOR_ZIP_ENTRIES));
+						newEntry.setLastAccessTime(FileTime.fromMillis(CONSTANT_TIME_FOR_ZIP_ENTRIES));
 					}
 
-					zipOutputStream.putNextEntry(entry);
+					zipOutputStream.putNextEntry(newEntry);
 					InputStream inputStream = zipFile.getInputStream(entry);
 					byte[] buf = new byte[1024];
 					int length;