Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mixin configurations' refmap property is not set when refmap name is set in an afterEvaluation #1249

Open
isXander opened this issue Jan 14, 2025 · 1 comment

Comments

@isXander
Copy link

Minimal reproduction

plugins {
    id("fabric-loom") version "1.10.0-alpha.9"
}

group = "dev.isxander"
version = "1.0.0"

repositories {
    mavenCentral()
}

loom {
    mixin {
        afterEvaluate {
            defaultRefmapName = "test.refmap.json"
        }
    }
}

dependencies {
    minecraft("com.mojang:minecraft:1.21.4")
    mappings(loom.officialMojangMappings())
    modImplementation("net.fabricmc:fabric-loader:0.16.10")
}

The afterEvaluate is the issue here.

Problem

Loom builds with a refmap equal to the conventional value for the property defaultRefmapName, and does not add refmap property to mixin configuration files when defaultRefmapName is set within an afterEvaluate.

  • When building, the refmap is produced by Mixin AP with the file name set by the convention of defaultRefmapName, equal to ${project.name}-refmap.json.
  • Later in the build (in remapJar), Loom attempts to add the refmap property to mixin configuration files. It resolves defaultRefmapName correctly as per the minimal reproduction above as test.refmap.json.
  • Before adding, Loom checks that the jar contains such refmap, this fails, since the name of the refmap differs.

Breaking it down

  • CompileConfiguration#run#afterEvaluationWithService calls CompileConfiguration#setupMixinAp
  • which calls MixinExtensionImpl#init
  • which calls MixinExtensionImpl#initDefault
  • which applies mixin AP to a sourceset, with the refmap name resolving defaultRefmapName

In remapJar, Loom adds the refmap property to mixin configs defined inside of fabric.mod.json.
Here is a snippet, I have added comments

	public void applyToJar(Path path) throws IOException {
        // Get the fabric.mod.json from the `jar` task built jar
		final FabricModJson fabricModJson = FabricModJsonFactory.createFromZipNullable(path);

		if (fabricModJson == null) {
			return;
		}

        // Get mixin configurations defined in the FMJ
		final List<String> allMixinConfigs = fabricModJson.getMixinConfigurations();
        // Gets mixin files that match a pattern from resources
		final List<String> mixinConfigs = getOptions().getMixinConfigs().get().stream()
				.filter(allMixinConfigs::contains) // intersect with the ones in FMJ
				.toList();

        // This refmapName is resolved (property `defaultRefmapName`) in the
        // constructor of `RemapJar` where `getOptions()` property is set.
		final String refmapName = getOptions().getRefmapName().get();

        // This check fails, since the AP was told to generate a refmap with a 
        // different name (the conventional name). The refmap is never added.
		if (ZipUtils.contains(path, refmapName)) {
			int transformed = ZipUtils.transformJson(JsonObject.class, path, mixinConfigs.stream().collect(Collectors.toMap(s -> s, s -> json -> {
				if (!json.has("refmap")) {
					json.addProperty("refmap", refmapName);
				}

				return json;
			})));
		}
	}

To summarise

  • Loom resolves defaultRefmapName within an afterEvaluate, using it to tell the Mixin AP the name to generate the refmap
  • Loom resolves defaultRefmapName again at build-time within remapJar, to add it to mixin configuration files. It fails the check to see if the refmap exists in the jar, since it's been written as a different name.

This is a race condition between Loom's afterEvaluate and the buildscript's.

Potential fixes

  1. Call finalizeOnRead() on defaultRefmapName, making this behaviour impossible
  2. Move configuration of Mixin AP to later in the build lifecycle.
  3. Something else I can't think of
@isXander
Copy link
Author

@modmuss50 - as mentioned in discord

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant