Skip to content

Commit eb157e4

Browse files
SpaceWalkerRSSpaceWalkerRS
authored
fix Unpick constants for layered mappings with Unpick metadata V1 (#1512)
* implement fallback unpick constants for layered mappings * fix unpick metadata serialization * simplify maven notation split regex * always upgrade unpick metadata v1 to v2 for layered mappings * update tests
1 parent 37ba852 commit eb157e4

File tree

12 files changed

+194
-20
lines changed

12 files changed

+194
-20
lines changed

‎src/main/java/net/fabricmc/loom/configuration/providers/mappings/LayeredMappingsFactory.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,9 @@ private void writeUnpickData(LayeredMappingsProcessor processor, List<MappingLay
164164
return;
165165
}
166166

167+
byte[] data = UnpickMetadata.toJson(unpickData.metadata()).getBytes(StandardCharsets.UTF_8);
168+
167169
ZipUtils.add(mappingsFile, UnpickMetadata.UNPICK_DEFINITIONS_PATH, unpickData.definitions());
168-
ZipUtils.add(mappingsFile, UnpickMetadata.UNPICK_METADATA_PATH, unpickData.rawMetadata());
170+
ZipUtils.add(mappingsFile, UnpickMetadata.UNPICK_METADATA_PATH, data);
169171
}
170172
}

‎src/main/java/net/fabricmc/loom/configuration/providers/mappings/LayeredMappingsProcessor.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import net.fabricmc.loom.configuration.providers.mappings.extras.annotations.AnnotationsLayer;
4242
import net.fabricmc.loom.configuration.providers.mappings.extras.signatures.SignatureFixesLayer;
4343
import net.fabricmc.loom.configuration.providers.mappings.extras.unpick.UnpickLayer;
44+
import net.fabricmc.loom.configuration.providers.mappings.unpick.UnpickMetadata;
4445
import net.fabricmc.mappingio.adapter.MappingSourceNsSwitch;
4546
import net.fabricmc.mappingio.tree.MemoryMappingTree;
4647

@@ -151,6 +152,19 @@ public Map<String, String> getSignatureFixes(List<MappingLayer> layers) {
151152
UnpickLayer.UnpickData data = unpickLayer.getUnpickData();
152153
if (data == null) continue;
153154

155+
if (!data.metadata().hasConstantsLocation()) {
156+
// if the constants location is not provided explicitly, Loom
157+
// cannot trick its way into finding them, since the mappings
158+
// dependency points to the layered mappings instead of the
159+
// individual layer that provided the unpick data
160+
String fallbackConstants = unpickLayer.getFallbackConstants();
161+
UnpickMetadata metadata = fallbackConstants == null
162+
? data.metadata().withoutConstants()
163+
: data.metadata().withConstants(fallbackConstants);
164+
165+
data = new UnpickLayer.UnpickData(metadata, data.definitions());
166+
}
167+
154168
unpickDataList.add(data);
155169
}
156170
}

‎src/main/java/net/fabricmc/loom/configuration/providers/mappings/extras/unpick/UnpickLayer.java‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,13 @@ public interface UnpickLayer {
3838
@Nullable
3939
UnpickData getUnpickData() throws IOException;
4040

41-
record UnpickData(UnpickMetadata metadata, byte[] rawMetadata, byte[] definitions) {
41+
@Nullable
42+
String getFallbackConstants();
43+
44+
record UnpickData(UnpickMetadata metadata, byte[] definitions) {
4245
public static UnpickData read(Path metadataPath, Path definitionPath) throws IOException {
4346
final byte[] definitions = Files.readAllBytes(definitionPath);
44-
final byte[] metadata = Files.readAllBytes(metadataPath);
45-
return new UnpickData(UnpickMetadata.parse(metadataPath), metadata, definitions);
47+
return new UnpickData(UnpickMetadata.parse(metadataPath), definitions);
4648
}
4749
}
4850
}

‎src/main/java/net/fabricmc/loom/configuration/providers/mappings/file/FileMappingsLayer.java‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,8 @@ public record FileMappingsLayer(
5555
boolean enigma, // Enigma cannot be automatically detected since it's stored in a directory.
5656
boolean unpick,
5757
boolean annotations,
58-
String mergeNamespace
58+
String mergeNamespace,
59+
String fallbackUnpickConstants
5960
) implements MappingLayer, UnpickLayer, AnnotationsLayer {
6061
@Override
6162
public void visit(MappingVisitor mappingVisitor) throws IOException {
@@ -120,6 +121,15 @@ public List<Class<? extends MappingLayer>> dependsOn() {
120121
}
121122
}
122123

124+
@Override
125+
public @Nullable String getFallbackConstants() {
126+
if (!unpick) {
127+
return null;
128+
}
129+
130+
return fallbackUnpickConstants;
131+
}
132+
123133
@Override
124134
public @Nullable AnnotationsData getAnnotationsData() throws IOException {
125135
if (!annotations) {

‎src/main/java/net/fabricmc/loom/configuration/providers/mappings/file/FileMappingsSpec.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,14 @@ public record FileMappingsSpec(
3535
FileSpec fileSpec, String mappingPath,
3636
Optional<String> fallbackSourceNamespace, String fallbackTargetNamespace,
3737
boolean enigma, boolean unpick, boolean annotations,
38-
Optional<String> mergeNamespace
38+
Optional<String> mergeNamespace,
39+
Optional<String> fallbackUnpickConstants
3940
) implements MappingsSpec<FileMappingsLayer> {
4041
@Override
4142
public FileMappingsLayer createLayer(MappingContext context) {
4243
String finalFallbackSourceNamespace = fallbackSourceNamespace.orElse(context.productionNamespace());
4344
String finalMergeNamespace = mergeNamespace.orElse(context.isUsingIntermediateMappings() ? MappingsNamespace.INTERMEDIARY.toString() : MappingsNamespace.OFFICIAL.toString());
44-
return new FileMappingsLayer(fileSpec.get(context), mappingPath, finalFallbackSourceNamespace, fallbackTargetNamespace, enigma, unpick, annotations, finalMergeNamespace);
45+
String finalFallbackUnpickConstants = unpick ? fallbackUnpickConstants.orElse(null) : null;
46+
return new FileMappingsLayer(fileSpec.get(context), mappingPath, finalFallbackSourceNamespace, fallbackTargetNamespace, enigma, unpick, annotations, finalMergeNamespace, finalFallbackUnpickConstants);
4547
}
4648
}

‎src/main/java/net/fabricmc/loom/configuration/providers/mappings/file/FileMappingsSpecBuilderImpl.java‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import net.fabricmc.loom.api.mappings.layered.MappingsNamespace;
3131
import net.fabricmc.loom.api.mappings.layered.spec.FileMappingsSpecBuilder;
3232
import net.fabricmc.loom.api.mappings.layered.spec.FileSpec;
33+
import net.fabricmc.loom.configuration.providers.mappings.utils.MavenFileSpec;
3334

3435
public class FileMappingsSpecBuilderImpl implements FileMappingsSpecBuilder {
3536
/**
@@ -45,6 +46,7 @@ public class FileMappingsSpecBuilderImpl implements FileMappingsSpecBuilder {
4546
private boolean unpick = false;
4647
private boolean annotations = false;
4748
private Optional<String> mergeNamespace = Optional.empty();
49+
private Optional<String> fallbackUnpickConstants = Optional.empty();
4850

4951
private FileMappingsSpecBuilderImpl(FileSpec fileSpec) {
5052
this.fileSpec = fileSpec;
@@ -82,6 +84,18 @@ public FileMappingsSpecBuilder containsAnnotations() {
8284
@Override
8385
public FileMappingsSpecBuilderImpl containsUnpick() {
8486
unpick = true;
87+
88+
if (fileSpec instanceof MavenFileSpec mavenFileSpec) {
89+
String dependencyNotation = mavenFileSpec.dependencyNotation();
90+
String[] notationParts = dependencyNotation.split(":");
91+
92+
if (notationParts.length == 4) {
93+
dependencyNotation = dependencyNotation.substring(0, dependencyNotation.lastIndexOf(':'));
94+
}
95+
96+
fallbackUnpickConstants = Optional.of(dependencyNotation + ":constants");
97+
}
98+
8599
return this;
86100
}
87101

@@ -104,6 +118,6 @@ public FileMappingsSpecBuilderImpl mergeNamespace(String namespace) {
104118
}
105119

106120
public FileMappingsSpec build() {
107-
return new FileMappingsSpec(fileSpec, mappingPath, fallbackSourceNamespace, fallbackTargetNamespace, enigma, unpick, annotations, mergeNamespace);
121+
return new FileMappingsSpec(fileSpec, mappingPath, fallbackSourceNamespace, fallbackTargetNamespace, enigma, unpick, annotations, mergeNamespace, fallbackUnpickConstants);
108122
}
109123
}

‎src/main/java/net/fabricmc/loom/configuration/providers/mappings/unpick/UnpickMetadata.java‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,28 @@
3333
import org.jspecify.annotations.Nullable;
3434

3535
import net.fabricmc.loom.LoomGradlePlugin;
36+
import net.fabricmc.loom.api.mappings.layered.MappingsNamespace;
3637

3738
public sealed interface UnpickMetadata permits UnpickMetadata.V1, UnpickMetadata.V2 {
3839
String UNPICK_METADATA_PATH = "extras/unpick.json";
3940
String UNPICK_DEFINITIONS_PATH = "extras/definitions.unpick";
4041

42+
/**
43+
* @return whether constants are required by the Unpick definitions
44+
*/
4145
boolean hasConstants();
4246

47+
/**
48+
* @return whether the maven location of the constants is specified by this metadata
49+
*/
50+
boolean hasConstantsLocation();
51+
52+
UnpickMetadata withConstants(String constants);
53+
54+
default UnpickMetadata withoutConstants() {
55+
return withConstants(null);
56+
}
57+
4358
/**
4459
* @param unpickGroup Deprecated, always uses the version of unpick loom depends on.
4560
* @param unpickVersion Deprecated, always uses the version of unpick loom depends on.
@@ -49,6 +64,18 @@ record V1(@Deprecated String unpickGroup, @Deprecated String unpickVersion) impl
4964
public boolean hasConstants() {
5065
return true;
5166
}
67+
68+
@Override
69+
public boolean hasConstantsLocation() {
70+
return false;
71+
}
72+
73+
@Override
74+
public UnpickMetadata withConstants(String constants) {
75+
// all v1 data is deprecated and ignored by Loom
76+
// and only v2 format allows setting the constants location
77+
return new UnpickMetadata.V2(MappingsNamespace.NAMED.toString(), constants);
78+
}
5279
}
5380

5481
/**
@@ -62,6 +89,28 @@ record V2(String namespace, @Nullable String constants) implements UnpickMetadat
6289
public boolean hasConstants() {
6390
return constants != null;
6491
}
92+
93+
@Override
94+
public boolean hasConstantsLocation() {
95+
return true;
96+
}
97+
98+
@Override
99+
public UnpickMetadata withConstants(String constants) {
100+
return new UnpickMetadata.V2(namespace, constants);
101+
}
102+
}
103+
104+
static String toJson(UnpickMetadata metadata) {
105+
JsonObject json = LoomGradlePlugin.GSON.toJsonTree(metadata).getAsJsonObject();
106+
107+
int version = switch (metadata) {
108+
case UnpickMetadata.V1 v1 -> 1;
109+
case UnpickMetadata.V2 v2 -> 2;
110+
};
111+
json.addProperty("version", version);
112+
113+
return json.toString();
65114
}
66115

67116
static UnpickMetadata parse(Path path) throws IOException {

‎src/test/groovy/net/fabricmc/loom/test/unit/layeredmappings/LayeredMappingSpecBuilderTest.groovy‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ class LayeredMappingSpecBuilderTest extends Specification {
115115
}
116116
def layers = spec.layers()
117117
then:
118-
spec.version == "layered+hash.38489917"
118+
spec.version == "layered+hash.1193216257"
119119
layers.size() == 2
120120
layers[0].class == IntermediaryMappingsSpec
121121
layers[1].class == FileMappingsSpec

‎src/test/groovy/net/fabricmc/loom/test/unit/layeredmappings/LayeredMappingsSpecification.groovy‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ abstract class LayeredMappingsSpecification extends Specification implements Lay
9090

9191
MemoryMappingTree getSingleMapping(MappingsSpec<? extends MappingLayer> spec) {
9292
MemoryMappingTree mappingTree = new MemoryMappingTree()
93-
spec.createLayer(new TestMappingContext([spec])).visit(mappingTree)
93+
spec.createLayer(createMappingContext(spec)).visit(mappingTree)
9494
return mappingTree
9595
}
9696

@@ -108,7 +108,12 @@ abstract class LayeredMappingsSpecification extends Specification implements Lay
108108

109109
UnpickLayer.UnpickData getUnpickData(MappingsSpec<? extends MappingLayer>... specs) {
110110
LayeredMappingsProcessor processor = createLayeredMappingsProcessor(specs)
111-
return processor.getUnpickData(processor.resolveLayers(new TestMappingContext(specs.toList())))
111+
MappingContext context = createMappingContext(specs)
112+
return processor.getUnpickData(processor.resolveLayers(context))
113+
}
114+
115+
MappingContext createMappingContext(MappingsSpec<? extends MappingLayer>... specs) {
116+
return new TestMappingContext(specs.toList())
112117
}
113118

114119
private static LayeredMappingsProcessor createLayeredMappingsProcessor(MappingsSpec<? extends MappingLayer>... specs) {

‎src/test/groovy/net/fabricmc/loom/test/unit/layeredmappings/LayeredMappingsTestConstants.groovy‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,16 @@ package net.fabricmc.loom.test.unit.layeredmappings
2727
import net.fabricmc.loom.configuration.providers.minecraft.MinecraftVersionMeta
2828

2929
interface LayeredMappingsTestConstants {
30+
public static final String INTERMEDIARY_1_21_11_URL = "https://maven.fabricmc.net/net/fabricmc/intermediary/1.21.11/intermediary-1.21.11-v2.jar"
3031
public static final String INTERMEDIARY_1_17_URL = "https://maven.fabricmc.net/net/fabricmc/intermediary/1.17/intermediary-1.17-v2.jar"
3132
public static final String INTERMEDIARY_1_16_5_URL = "https://maven.fabricmc.net/net/fabricmc/intermediary/1.16.5/intermediary-1.16.5-v2.jar"
3233

34+
public static final Map<String, MinecraftVersionMeta.Download> DOWNLOADS_1_21_11 = [
35+
client_mappings: new MinecraftVersionMeta.Download(null, "031a68bebf55d824f66d6573d8c752f0e1bf232a", 11779287, "https://piston-data.mojang.com/v1/objects/031a68bebf55d824f66d6573d8c752f0e1bf232a/client.txt"),
36+
server_mappings: new MinecraftVersionMeta.Download(null, "64bb6d763bed0a9f1d632ec347938594144943ed", 56327581, "https://launcher.mojang.com/v1/objects/64bb6d763bed0a9f1d632ec347938594144943ed/server.txt")
37+
]
38+
public static final MinecraftVersionMeta VERSION_META_1_21_11 = new MinecraftVersionMeta(null, null, null, 0, DOWNLOADS_1_21_11, null, null, null, null, 0, "2025-12-09T12:23:30+00:00", null, null, null)
39+
3340
public static final Map<String, MinecraftVersionMeta.Download> DOWNLOADS_1_17 = [
3441
client_mappings: new MinecraftVersionMeta.Download(null, "227d16f520848747a59bef6f490ae19dc290a804", 6431705, "https://launcher.mojang.com/v1/objects/227d16f520848747a59bef6f490ae19dc290a804/client.txt"),
3542
server_mappings: new MinecraftVersionMeta.Download(null, "84d80036e14bc5c7894a4fad9dd9f367d3000334", 4948536, "https://launcher.mojang.com/v1/objects/84d80036e14bc5c7894a4fad9dd9f367d3000334/server.txt")
@@ -44,5 +51,8 @@ interface LayeredMappingsTestConstants {
4451

4552
public static final String PARCHMENT_NOTATION = "org.parchmentmc.data:parchment-1.16.5:20210608-SNAPSHOT@zip"
4653
public static final String PARCHMENT_URL = "https://maven.parchmentmc.net/org/parchmentmc/data/parchment-1.16.5/20210608-SNAPSHOT/parchment-1.16.5-20210608-SNAPSHOT.zip"
54+
public static final String YARN_1_21_11_URL = "https://maven.fabricmc.net/net/fabricmc/yarn/1.21.11%2Bbuild.4/yarn-1.21.11%2Bbuild.4-v2.jar"
4755
public static final String YARN_1_17_URL = "https://maven.fabricmc.net/net/fabricmc/yarn/1.17%2Bbuild.13/yarn-1.17%2Bbuild.13-v2.jar"
56+
public static final String YARN_1_21_11_NOTATION = "net.fabricmc:yarn:1.21.11+build.4"
57+
public static final String YARN_1_17_NOTATION = "net.fabricmc:yarn:1.17+build.13"
4858
}

0 commit comments

Comments
 (0)