From 550dd373af2f13171511c6770e966e1f8600e490 Mon Sep 17 00:00:00 2001 From: mezz Date: Wed, 12 Aug 2026 00:38:57 +0900 Subject: [PATCH] Use absolute paths for source-transform access transformers ## Problem `ApplySourceTransformAction` receives user-provided files from `--access-transformer` and `--validated-access-transformer`. NFRT launches the Java Source Transformer (JST) from a temporary directory for the `transformSources` node, so a relative path such as `config/at.cfg` would be resolved from that directory instead of the directory where NFRT was launched. ## Fix Convert both kinds of user-provided AT paths to absolute paths before formatting the JST command line. Paths inside the node's temporary directory can still be shortened by `ProcessingEnvironment#getPathArgument`, while paths supplied from elsewhere keep their correct location. This change originally came from Technici4n's commits `daa4bd9` and `6acd2cc`. ## Tests Launch the packaged NFRT jar through its normal `run` command against a small offline NeoForm fixture. Pass relative files through both `--access-transformer` and `--validated-access-transformer`, then check the published `gameSources` archive to confirm that the real configured JST applied both files. As a control, removing the two `toAbsolutePath` calls makes the CLI exit with an error when JST reports `NoSuchFileException` for the relative validated AT path. Co-authored-by: Technici4n <13494793+Technici4n@users.noreply.github.com> --- build.gradle | 42 ++- .../runtime/integration/NfrtCommand.java | 319 ++++++++++++++++++ .../integration/RunNeoFormCommandTest.java | 55 +++ .../runtime/integration/package-info.java | 1 + .../actions/ApplySourceTransformAction.java | 4 +- 5 files changed, 415 insertions(+), 6 deletions(-) create mode 100644 src/integrationTest/java/net/neoforged/neoform/runtime/integration/NfrtCommand.java create mode 100644 src/integrationTest/java/net/neoforged/neoform/runtime/integration/RunNeoFormCommandTest.java create mode 100644 src/integrationTest/java/net/neoforged/neoform/runtime/integration/package-info.java diff --git a/build.gradle b/build.gradle index 4c7f9cb..4e86c29 100644 --- a/build.gradle +++ b/build.gradle @@ -60,6 +60,10 @@ jar { AdhocComponentWithVariants javaComponent = (AdhocComponentWithVariants) project.components.getByName("java") +sourceSets { + integrationTest +} + configurations { changelog { canBeResolved = false @@ -84,6 +88,16 @@ configurations { } components.java.addVariantsFromConfiguration(it) {} } + integrationTestTools { + canBeConsumed = false + canBeResolved = true + transitive = false + } +} + +def toolVersions = new java.util.Properties() +file("src/main/resources/tools.properties").withInputStream { + toolVersions.load(it) } dependencies { @@ -104,13 +118,15 @@ dependencies { testImplementation 'org.junit.jupiter:junit-jupiter-params' testImplementation 'org.mockito:mockito-junit-jupiter' testImplementation 'org.assertj:assertj-core' + integrationTestImplementation platform('org.junit:junit-bom:5.13.2') + integrationTestImplementation platform('org.assertj:assertj-bom:3.27.3') + integrationTestImplementation 'org.junit.jupiter:junit-jupiter' + integrationTestRuntimeOnly 'org.junit.platform:junit-platform-launcher' + integrationTestImplementation 'org.assertj:assertj-core' + integrationTestTools toolVersions.getProperty("JAVA_SOURCE_TRANSFORMER") } // Add dependencies to external tools -var toolVersions = new java.util.Properties() -file("src/main/resources/tools.properties").withInputStream { - toolVersions.load(it) -} for (var toolDependency : toolVersions.values()) { dependencies { externalTools(toolDependency) @@ -128,6 +144,24 @@ artifacts { test { useJUnitPlatform() + dependsOn "integrationTest" +} + +tasks.register("integrationTest", Test) { + description = "Runs tests against the packaged NFRT command line" + group = "verification" + testClassesDirs = sourceSets.integrationTest.output.classesDirs + classpath = sourceSets.integrationTest.runtimeClasspath + useJUnitPlatform() + dependsOn shadowJar + inputs.file(shadowJar.archiveFile) + inputs.files(configurations.integrationTestTools) + inputs.property("javaSourceTransformerCoordinate", toolVersions.getProperty("JAVA_SOURCE_TRANSFORMER")) + doFirst { + systemProperty "nfrt.test.executable-jar", shadowJar.archiveFile.get().asFile.absolutePath + systemProperty "nfrt.test.java-source-transformer-coordinate", toolVersions.getProperty("JAVA_SOURCE_TRANSFORMER") + systemProperty "nfrt.test.java-source-transformer-jar", configurations.integrationTestTools.singleFile.absolutePath + } } publishing { diff --git a/src/integrationTest/java/net/neoforged/neoform/runtime/integration/NfrtCommand.java b/src/integrationTest/java/net/neoforged/neoform/runtime/integration/NfrtCommand.java new file mode 100644 index 0000000..69814ac --- /dev/null +++ b/src/integrationTest/java/net/neoforged/neoform/runtime/integration/NfrtCommand.java @@ -0,0 +1,319 @@ +package net.neoforged.neoform.runtime.integration; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.time.Duration; +import java.util.ArrayList; +import java.util.HexFormat; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Properties; +import java.util.TreeMap; +import java.util.concurrent.TimeUnit; +import java.util.zip.ZipEntry; +import java.util.zip.ZipFile; +import java.util.zip.ZipOutputStream; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.fail; + +final class NfrtCommand { + private static final String GAME_SOURCES_RESULT = "gameSources"; + + private final List command; + private final Path workingDirectory; + private final Path testDirectory; + private final Duration timeout; + private final Map results; + + private NfrtCommand(List command, + Path workingDirectory, + Path testDirectory, + Duration timeout, + Map results) { + this.command = command; + this.workingDirectory = workingDirectory; + this.testDirectory = testDirectory; + this.timeout = timeout; + this.results = results; + } + + static Builder builder(Path testDirectory) { + return new Builder(testDirectory); + } + + void executeSuccessfully() throws IOException, InterruptedException { + execute().assertSuccess(); + } + + String readResult(String resultId, String entryName) throws IOException { + var result = results.get(resultId); + if (result == null) { + throw new IllegalArgumentException("Unknown result " + resultId + ". Available: " + results.keySet()); + } + + try (var zip = new ZipFile(result.toFile())) { + var entry = zip.getEntry(entryName); + assertThat(entry).as("Entry %s in result %s", entryName, resultId).isNotNull(); + try (var stream = zip.getInputStream(entry)) { + return new String(stream.readAllBytes(), StandardCharsets.UTF_8); + } + } + } + + private Result execute() throws IOException, InterruptedException { + var consoleLog = Files.createTempFile(testDirectory, "nfrt-console-", ".log"); + var process = new ProcessBuilder(command) + .directory(workingDirectory.toFile()) + .redirectErrorStream(true) + .redirectOutput(consoleLog.toFile()) + .start(); + + try { + if (!process.waitFor(timeout.toMillis(), TimeUnit.MILLISECONDS)) { + process.destroyForcibly().waitFor(); + fail("NFRT timed out after " + timeout.toSeconds() + + " seconds. Output:\n" + Files.readString(consoleLog)); + } + return new Result(process.exitValue(), Files.readString(consoleLog)); + } finally { + if (process.isAlive()) { + process.destroyForcibly().waitFor(); + } + } + } + + private record Result(int exitCode, String output) { + void assertSuccess() { + assertThat(exitCode).as("NFRT output:%n%s", output).isZero(); + } + } + + static final class Builder { + private final Path testDirectory; + private final Map sources = new LinkedHashMap<>(); + private final List accessTransformers = new ArrayList<>(); + private final List validatedAccessTransformers = new ArrayList<>(); + private Duration timeout = Duration.ofMinutes(1); + + private Builder(Path testDirectory) { + this.testDirectory = testDirectory.toAbsolutePath(); + } + + Builder source(String path, String content) { + sources.put(path, content); + return this; + } + + Builder accessTransformer(String relativePath, String content) { + return accessTransformer(Path.of(relativePath), content); + } + + Builder accessTransformer(Path path, String content) { + accessTransformers.add(new InputFile(path, content)); + return this; + } + + Builder validatedAccessTransformer(String relativePath, String content) { + return validatedAccessTransformer(Path.of(relativePath), content); + } + + Builder validatedAccessTransformer(Path path, String content) { + validatedAccessTransformers.add(new InputFile(path, content)); + return this; + } + + Builder timeout(Duration timeout) { + if (timeout.isZero() || timeout.isNegative()) { + throw new IllegalArgumentException("Timeout must be positive"); + } + this.timeout = timeout; + return this; + } + + NfrtCommand build() throws IOException { + if (sources.isEmpty()) { + throw new IllegalStateException("At least one source must be added"); + } + + Files.createDirectories(testDirectory); + var workingDirectory = testDirectory.resolve("launch"); + Files.createDirectories(workingDirectory); + + var sourcesArchive = testDirectory.resolve("sources.zip"); + writeZip(sourcesArchive, sources); + + var launcherDirectory = createLauncherDirectory(testDirectory, sourcesArchive); + var javaSourceTransformerCoordinate = requiredProperty( + "nfrt.test.java-source-transformer-coordinate" + ); + var neoForm = createNeoForm(testDirectory, javaSourceTransformerCoordinate); + var artifactManifest = createArtifactManifest(testDirectory, javaSourceTransformerCoordinate); + + var regularAtPaths = writeInputFiles(testDirectory, workingDirectory, accessTransformers); + var validatedAtPaths = writeInputFiles(testDirectory, workingDirectory, validatedAccessTransformers); + var gameSources = testDirectory.resolve("result.zip"); + + var command = new ArrayList(); + command.add(ProcessHandle.current().info().command().orElseThrow()); + command.add("-jar"); + command.add(requiredPathProperty("nfrt.test.executable-jar").toString()); + command.add("--home-dir=" + testDirectory.resolve("home")); + command.add("--work-dir=" + testDirectory.resolve("work")); + command.add("--launcher-dir=" + launcherDirectory); + command.add("--artifact-manifest=" + artifactManifest); + command.add("--no-color"); + command.add("--no-emojis"); + command.add("run"); + command.add("--disable-cache"); + command.add("--disable-cache-maintenance"); + command.add("--neoform=" + neoForm); + for (var path : regularAtPaths) { + command.add("--access-transformer=" + path); + } + for (var path : validatedAtPaths) { + command.add("--validated-access-transformer=" + path); + } + command.add("--write-result=" + GAME_SOURCES_RESULT + ":" + gameSources); + + return new NfrtCommand( + List.copyOf(command), + workingDirectory, + testDirectory, + timeout, + Map.of(GAME_SOURCES_RESULT, gameSources) + ); + } + + private static Path createLauncherDirectory(Path testDirectory, Path sourcesArchive) throws IOException { + var launcherDirectory = testDirectory.resolve("launcher"); + var versionDirectory = launcherDirectory.resolve("versions/test"); + Files.createDirectories(versionDirectory); + Files.writeString(versionDirectory.resolve("test.json"), """ + { + "id": "test", + "downloads": { + "client": { + "sha1": "%s", + "size": %d, + "url": "%s" + } + }, + "libraries": [] + } + """.formatted(sha1(sourcesArchive), Files.size(sourcesArchive), sourcesArchive.toUri())); + return launcherDirectory; + } + + private static Path createNeoForm(Path testDirectory, + String javaSourceTransformerCoordinate) throws IOException { + var neoForm = testDirectory.resolve("neoform.zip"); + writeZip(neoForm, Map.of("config.json", """ + { + "spec": 1, + "version": "test", + "official": true, + "java_target": 21, + "encoding": "UTF-8", + "data": {}, + "steps": { + "joined": [ + { "type": "downloadJson" }, + { "type": "downloadClient" }, + { "type": "copySources", "name": "decompile", "input": "{downloadClientOutput}" }, + { "type": "copySources", "name": "patch", "input": "{decompileOutput}" } + ] + }, + "functions": { + "copySources": { + "version": "%s", + "args": ["--in-format", "ARCHIVE", "--out-format", "ARCHIVE", "{input}", "{output}"] + } + }, + "libraries": { + "joined": [] + } + } + """.formatted(javaSourceTransformerCoordinate))); + return neoForm; + } + + private static Path createArtifactManifest(Path testDirectory, + String javaSourceTransformerCoordinate) throws IOException { + var artifactManifest = testDirectory.resolve("artifact-manifest.properties"); + var artifacts = new Properties(); + artifacts.setProperty( + javaSourceTransformerCoordinate, + requiredPathProperty("nfrt.test.java-source-transformer-jar").toString() + ); + try (var output = Files.newOutputStream(artifactManifest)) { + artifacts.store(output, null); + } + return artifactManifest; + } + + private static List writeInputFiles(Path testDirectory, + Path workingDirectory, + List inputs) throws IOException { + var paths = new ArrayList(inputs.size()); + for (var input : inputs) { + var path = input.path().normalize(); + Path output; + if (path.isAbsolute()) { + if (!path.startsWith(testDirectory)) { + throw new IllegalArgumentException( + "Absolute input path must stay within the test directory: " + path + ); + } + output = path; + } else if (path.startsWith("..")) { + throw new IllegalArgumentException( + "Relative input path must stay within the working directory: " + path + ); + } else { + output = workingDirectory.resolve(path); + } + Files.createDirectories(output.getParent()); + Files.writeString(output, input.content()); + paths.add(path); + } + return paths; + } + + private static String sha1(Path input) throws IOException { + try { + var digest = MessageDigest.getInstance("SHA-1"); + return HexFormat.of().formatHex(digest.digest(Files.readAllBytes(input))); + } catch (NoSuchAlgorithmException e) { + throw new IllegalStateException("SHA-1 is not available", e); + } + } + + private static void writeZip(Path output, Map entries) throws IOException { + try (var zip = new ZipOutputStream(Files.newOutputStream(output))) { + for (var entry : new TreeMap<>(entries).entrySet()) { + zip.putNextEntry(new ZipEntry(entry.getKey())); + zip.write(entry.getValue().getBytes(StandardCharsets.UTF_8)); + zip.closeEntry(); + } + } + } + + private static String requiredProperty(String name) { + return Objects.requireNonNull(System.getProperty(name), "Missing system property " + name); + } + + private static Path requiredPathProperty(String name) { + return Path.of(requiredProperty(name)); + } + + private record InputFile(Path path, String content) { + } + } +} diff --git a/src/integrationTest/java/net/neoforged/neoform/runtime/integration/RunNeoFormCommandTest.java b/src/integrationTest/java/net/neoforged/neoform/runtime/integration/RunNeoFormCommandTest.java new file mode 100644 index 0000000..da8e21b --- /dev/null +++ b/src/integrationTest/java/net/neoforged/neoform/runtime/integration/RunNeoFormCommandTest.java @@ -0,0 +1,55 @@ +package net.neoforged.neoform.runtime.integration; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.nio.file.Path; + +import static org.assertj.core.api.Assertions.assertThat; + +class RunNeoFormCommandTest { + private static final String EXAMPLE_SOURCE = """ + class Example { + private int regularField; + private int validatedField; + } + """; + private static final String REGULAR_AT = "public Example regularField\n"; + private static final String VALIDATED_AT = "public Example validatedField\n"; + + @Test + void appliesRelativeAccessTransformerPaths(@TempDir Path tempDir) throws Exception { + var command = NfrtCommand.builder(tempDir) + .source("Example.java", EXAMPLE_SOURCE) + .accessTransformer("config/regular-access-transformer.cfg", REGULAR_AT) + .validatedAccessTransformer("config/validated-access-transformer.cfg", VALIDATED_AT) + .build(); + + command.executeSuccessfully(); + + assertThat(command.readResult("gameSources", "Example.java")) + .contains("public int regularField;") + .contains("public int validatedField;"); + } + + @Test + void appliesAbsoluteAccessTransformerPaths(@TempDir Path tempDir) throws Exception { + var command = NfrtCommand.builder(tempDir) + .source("Example.java", EXAMPLE_SOURCE) + .accessTransformer( + tempDir.resolve("config/regular-access-transformer.cfg").toAbsolutePath(), + REGULAR_AT + ) + .validatedAccessTransformer( + tempDir.resolve("config/validated-access-transformer.cfg").toAbsolutePath(), + VALIDATED_AT + ) + .build(); + + command.executeSuccessfully(); + + assertThat(command.readResult("gameSources", "Example.java")) + .contains("public int regularField;") + .contains("public int validatedField;"); + } +} diff --git a/src/integrationTest/java/net/neoforged/neoform/runtime/integration/package-info.java b/src/integrationTest/java/net/neoforged/neoform/runtime/integration/package-info.java new file mode 100644 index 0000000..effc2e3 --- /dev/null +++ b/src/integrationTest/java/net/neoforged/neoform/runtime/integration/package-info.java @@ -0,0 +1 @@ +package net.neoforged.neoform.runtime.integration; diff --git a/src/main/java/net/neoforged/neoform/runtime/actions/ApplySourceTransformAction.java b/src/main/java/net/neoforged/neoform/runtime/actions/ApplySourceTransformAction.java index 5f81bbf..839b68b 100644 --- a/src/main/java/net/neoforged/neoform/runtime/actions/ApplySourceTransformAction.java +++ b/src/main/java/net/neoforged/neoform/runtime/actions/ApplySourceTransformAction.java @@ -109,12 +109,12 @@ public void run(ProcessingEnvironment environment) throws IOException, Interrupt for (var path : validatedAccessTransformers) { args.add("--access-transformer"); - args.add(environment.getPathArgument(path)); + args.add(environment.getPathArgument(path.toAbsolutePath())); } for (var path : additionalAccessTransformers) { args.add("--access-transformer"); - args.add(environment.getPathArgument(path)); + args.add(environment.getPathArgument(path.toAbsolutePath())); } }