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

.github/workflows/test-push.yml

@@ -66,7 +66,32 @@ jobs:
           TEST_WARNING_MODE: fail
 
       - uses: actions/upload-artifact@v2
-        if: ${{ always() }}
+        if: ${{ failure() }}
         with:
           name: ${{ matrix.test }} (${{ matrix.java }}) Results
+          path: build/reports/
+
+  # Special case this test to run across all os's
+  reproducible_build_test:
+    needs: build
+
+    strategy:
+      fail-fast: false
+      matrix:
+        java: [ 1.8, 11, 15 ]
+        os: [ windows-2019, ubuntu-20.04, macos-10.15 ]
+
+    runs-on: ${{ matrix.os }}
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions/setup-java@v1
+        with:
+          java-version: ${{ matrix.java }}
+
+      - run: ./gradlew test --tests *ReproducibleBuildTest --stacktrace
+
+      - uses: actions/upload-artifact@v2
+        if: ${{ failure() }}
+        with:
+          name: Reproducible Build ${{ matrix.os }} (${{ matrix.java }}) Results
           path: build/reports/

build.gradle

@@ -211,6 +211,11 @@ task writeActionsTestMatrix() {
 					return
 				}
 
+				if (it.name.endsWith("ReproducibleBuildTest.groovy")) {
+					// This test gets a special case to run across all os's
+					return
+				}
+
 				testMatrix.add(it.name.replace(".groovy", ""))
 			}
 		}

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]);

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);
+						}
+					}
 				});
 	}
 

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 boolean preserveFileTimestamps = true;
-	private final Property<Boolean> archiveReproducibleFileOrder;
+	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"));
+			SourceRemapper.remapSources(getProject(), getInput(), getOutput(), direction.equals("named"), reproducibleFileOrder, preserveFileTimestamps);
-			ZipReprocessorUtil.reprocessZip(getOutput(), archivePreserveFileTimestamps.getOrElse(true), archiveReproducibleFileOrder.getOrElse(false));
 		} 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;
+	}
 }

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");
 

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 {

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;

src/test/groovy/net/fabricmc/loom/ReproducibleBuildTest.groovy

@@ -28,13 +28,11 @@ import com.google.common.hash.HashCode
 import com.google.common.hash.Hashing
 import com.google.common.io.Files
 import net.fabricmc.loom.util.ProjectTestTrait
-import spock.lang.IgnoreIf
 import spock.lang.Specification
 import spock.lang.Unroll
 
 import static org.gradle.testkit.runner.TaskOutcome.SUCCESS
 
-@IgnoreIf({ os.windows }) // Linux and mac create the same files, im unsure why windows is different. Let me know if you have any ideas?
 class ReproducibleBuildTest extends Specification implements ProjectTestTrait {
 	@Override
 	String name() {
@@ -48,11 +46,11 @@ class ReproducibleBuildTest extends Specification implements ProjectTestTrait {
 		then:
 			result.task(":build").outcome == SUCCESS
 			getOutputHash("fabric-example-mod-1.0.0.jar") == modHash
-			getOutputHash("fabric-example-mod-1.0.0-sources.jar") == sourceHash
+			getOutputHash("fabric-example-mod-1.0.0-sources.jar") in sourceHash // Done for different line endings.
 		where:
 			gradle 				| modHash								| sourceHash
-			'6.8.3' 			| "ccd6aaff1b06df01e4dd8c08625b82c9"	| "8bd590dc03b7dd0de3a4a7aeb431d4e8"
+			'6.8.3' 			| "6132ffb4117adb7e258f663110552952"	| ["be31766e6cafbe4ae3bca9e35ba63169", "7348b0bd87d36d7ec6f3bca9c2b66062"]
-			'7.0-milestone-2'	| "ccd6aaff1b06df01e4dd8c08625b82c9"	| "8bd590dc03b7dd0de3a4a7aeb431d4e8"
+			'7.0-milestone-2'	| "6132ffb4117adb7e258f663110552952"	| ["be31766e6cafbe4ae3bca9e35ba63169", "7348b0bd87d36d7ec6f3bca9c2b66062"]
 	}
 
 	String getOutputHash(String name) {

src/test/groovy/net/fabricmc/loom/util/ProjectTestTrait.groovy

@@ -26,7 +26,6 @@ package net.fabricmc.loom.util
 
 import org.gradle.testkit.runner.BuildResult
 import org.gradle.testkit.runner.GradleRunner
-import spock.lang.Shared
 
 trait ProjectTestTrait {
 	static File gradleHome = File.createTempDir()

src/test/resources/projects/reproducible/build.gradle

@@ -14,7 +14,6 @@ dependencies {
 	minecraft "com.mojang:minecraft:${project.minecraft_version}"
 	mappings "net.fabricmc:yarn:${project.yarn_mappings}:v2"
 	modImplementation "net.fabricmc:fabric-loader:${project.loader_version}"
-	modImplementation "net.fabricmc.fabric-api:fabric-api:${project.fabric_version}"
 }
 
 processResources {
@@ -48,4 +47,9 @@ jar {
 tasks.withType(AbstractArchiveTask) {
 	preserveFileTimestamps = false
 	reproducibleFileOrder = true
+}
+
+remapSourcesJar {
+	preserveFileTimestamps = false
+	reproducibleFileOrder = true
 }

src/test/resources/projects/reproducible/gradle.properties

@@ -11,7 +11,3 @@ org.gradle.jvmargs=-Xmx1G
 	mod_version = 1.0.0
 	maven_group = com.example
 	archives_base_name = fabric-example-mod
-
-# Dependencies
-	# currently not on the main fabric site, check on the maven: https://maven.fabricmc.net/net/fabricmc/fabric-api/fabric-api
-	fabric_version=0.31.0+1.16