Skip to content

Commit

Permalink
Deal with inheritance in mappings properly
Browse files Browse the repository at this point in the history
  • Loading branch information
lukebemish committed Dec 30, 2024
1 parent a2cfdce commit db285e9
Show file tree
Hide file tree
Showing 10 changed files with 562 additions and 46 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package dev.lukebemish.taskgraphrunner.runtime.mappings;

import net.fabricmc.mappingio.FlatMappingVisitor;
import net.fabricmc.mappingio.MappingVisitor;
import net.fabricmc.mappingio.adapter.RegularAsFlatMappingVisitor;
import org.jetbrains.annotations.Nullable;

import java.io.IOException;
import java.util.Arrays;
import java.util.Objects;

public class MappingDropMissingSrcVisitor extends ForwardingFlatVisitor {
public MappingDropMissingSrcVisitor(FlatMappingVisitor delegate) {
super(delegate);
}

public static MappingVisitor create(MappingVisitor delegate) {
return new MappingDropMissingSrcVisitor(new RegularAsFlatMappingVisitor(delegate)).asRegularVisitor();
}

private boolean checkNames(@Nullable String[] dstNames) {
return dstNames != null && dstNames.length > 0 && Arrays.stream(dstNames).allMatch(Objects::nonNull);
}

@Override
public boolean visitClass(String srcName, @Nullable String[] dstNames) throws IOException {
return checkNames(dstNames) && super.visitClass(srcName, dstNames);
}

@Override
public boolean visitMethod(String srcClsName, String srcName, @Nullable String srcDesc, @Nullable String[] dstClsNames, @Nullable String[] dstNames, @Nullable String[] dstDescs) throws IOException {
return checkNames(dstNames) && super.visitMethod(srcClsName, srcName, srcDesc, dstClsNames, dstNames, dstDescs);
}

@Override
public boolean visitField(String srcClsName, String srcName, @Nullable String srcDesc, @Nullable String[] dstClsNames, @Nullable String[] dstNames, @Nullable String[] dstDescs) throws IOException {
return checkNames(dstNames) && super.visitField(srcClsName, srcName, srcDesc, dstClsNames, dstNames, dstDescs);
}

@Override
public boolean visitMethodArg(String srcClsName, String srcMethodName, @Nullable String srcMethodDesc, int argPosition, int lvIndex, @Nullable String srcName, @Nullable String[] dstClsNames, @Nullable String[] dstMethodNames, @Nullable String[] dstMethodDescs, String[] dstNames) throws IOException {
return checkNames(dstNames) && super.visitMethodArg(srcClsName, srcMethodName, srcMethodDesc, argPosition, lvIndex, srcName, dstClsNames, dstMethodNames, dstMethodDescs, dstNames);
}

@Override
public boolean visitMethodVar(String srcClsName, String srcMethodName, @Nullable String srcMethodDesc, int lvtRowIndex, int lvIndex, int startOpIdx, int endOpIdx, @Nullable String srcName, @Nullable String[] dstClsNames, @Nullable String[] dstMethodNames, @Nullable String[] dstMethodDescs, String[] dstNames) throws IOException {
return checkNames(dstNames) && super.visitMethodVar(srcClsName, srcMethodName, srcMethodDesc, lvtRowIndex, lvIndex, startOpIdx, endOpIdx, srcName, dstClsNames, dstMethodNames, dstMethodDescs, dstNames);
}
}

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
public sealed interface MappingsSourceImpl {
MappingTree makeMappings(Context context);

MappingsUtil.MappingProvider makeMappingsFillInheritance(Context context);

List<TaskInput> inputs();

interface MappingConsumer extends AutoCloseable {
Expand Down Expand Up @@ -120,6 +122,19 @@ public MappingTree makeMappings(Context context) {
.toList());
}

@Override
public MappingsUtil.MappingProvider makeMappingsFillInheritance(Context context) {
return inheritance -> MappingsUtil.filledChain(inheritance, files.paths(context).stream()
.map(p -> {
try {
return MappingsUtil.provider(loadMappings(p));
} catch (IOException e) {
throw new UncheckedIOException(e);
}
})
.toList());
}

@Override
public List<TaskInput> inputs() {
return List.of(label, files);
Expand Down Expand Up @@ -172,6 +187,13 @@ public MappingTree makeMappings(Context context) {
.toList());
}

@Override
public MappingsUtil.MappingProvider makeMappingsFillInheritance(Context context) {
return inheritance -> MappingsUtil.filledChain(inheritance, sources.stream()
.map(s -> s.makeMappingsFillInheritance(context))
.toList());
}

@Override
public List<TaskInput> inputs() {
var list = sources.stream()
Expand Down Expand Up @@ -204,6 +226,19 @@ public MappingTree makeMappings(Context context) {
.toList());
}

@Override
public MappingsUtil.MappingProvider makeMappingsFillInheritance(Context context) {
return inheritance -> MappingsUtil.filledMerge(inheritance, files.paths(context).stream()
.map(p -> {
try {
return MappingsUtil.provider(loadMappings(p));
} catch (IOException e) {
throw new UncheckedIOException(e);
}
})
.toList());
}

@Override
public List<TaskInput> inputs() {
return List.of(label, files);
Expand All @@ -226,6 +261,13 @@ public MappingTree makeMappings(Context context) {
.toList());
}

@Override
public MappingsUtil.MappingProvider makeMappingsFillInheritance(Context context) {
return inheritance -> MappingsUtil.filledMerge(inheritance, sources.stream()
.map(s -> s.makeMappingsFillInheritance(context))
.toList());
}

@Override
public List<TaskInput> inputs() {
var list = sources.stream()
Expand All @@ -251,6 +293,15 @@ public MappingTree makeMappings(Context context) {
return MappingsUtil.reverse(mappings);
}

@Override
public MappingsUtil.MappingProvider makeMappingsFillInheritance(Context context) {
var sourceMappings = source.makeMappingsFillInheritance(context);
return inheritance -> {
MappingTree mappings = sourceMappings.make(inheritance);
return MappingsUtil.reverse(mappings);
};
}

@Override
public List<TaskInput> inputs() {
var list = new ArrayList<>(source.inputs());
Expand Down Expand Up @@ -292,6 +343,12 @@ public MappingTree makeMappings(Context context) {
}
}

@Override
public MappingsUtil.MappingProvider makeMappingsFillInheritance(Context context) {
var mappings = makeMappings(context);
return MappingsUtil.provider(mappings);
}

@Override
public List<TaskInput> inputs() {
if (extension == null) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,34 +1,28 @@
package dev.lukebemish.taskgraphrunner.runtime.mappings;

import net.fabricmc.mappingio.FlatMappingVisitor;
import net.fabricmc.mappingio.MappedElementKind;
import net.fabricmc.mappingio.MappingVisitor;
import net.fabricmc.mappingio.adapter.ForwardingMappingVisitor;
import net.fabricmc.mappingio.adapter.MappingDstNsReorder;
import net.fabricmc.mappingio.adapter.MappingNsCompleter;
import net.fabricmc.mappingio.adapter.MappingNsRenamer;
import net.fabricmc.mappingio.adapter.MappingSourceNsSwitch;
import net.fabricmc.mappingio.tree.MappingTree;
import net.fabricmc.mappingio.tree.MappingTreeView;
import net.fabricmc.mappingio.tree.MemoryMappingTree;
import org.jetbrains.annotations.Nullable;

import java.io.IOException;
import java.io.UncheckedIOException;
import java.util.Arrays;
import java.util.ArrayList;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;

public final class MappingsUtil {
private MappingsUtil() {}

public static MappingTree reverse(MappingTree tree) {
if (tree.getMaxNamespaceId() < 0) {
return tree;
}
var newTree = new MemoryMappingTree();
var reverser = new MappingSourceNsSwitch(newTree, tree.getDstNamespaces().getFirst(), true);
var reverser = tree.getDstNamespaces().isEmpty() ? newTree : new MappingSourceNsSwitch(newTree, tree.getDstNamespaces().getFirst(), true);
try {
tree.accept(reverser);
} catch (IOException e) {
Expand All @@ -37,7 +31,7 @@ public static MappingTree reverse(MappingTree tree) {
return newTree;
}

public static MappingTree merge(List<? extends MappingTreeView> trees) {
public static MappingTree merge(List<? extends MappingTree> trees) {
var newTree = new MemoryMappingTree();
for (var tree : trees) {
MappingVisitor visitor = newTree;
Expand All @@ -59,9 +53,45 @@ public static MappingTree merge(List<? extends MappingTreeView> trees) {
return newTree;
}

public static MappingTree chain(List<? extends MappingTreeView> trees) {
public interface MappingProvider {
MappingTree make(MappingInheritance inheritance) throws IOException;
}

public static MappingTree filledMerge(MappingInheritance inheritance, List<MappingProvider> trees) {
try {
var appliedTrees = new ArrayList<MappingTree>(trees.size());
for (var tree : trees) {
appliedTrees.add(tree.make(inheritance));
}
return merge(appliedTrees);
} catch (IOException e) {
throw new UncheckedIOException(e);
}
}

public static MappingProvider provider(MappingTree tree) {
return inheritance -> inheritance.fill(tree);
}

public static MappingTree filledChain(MappingInheritance inheritance, List<MappingProvider> trees) {
try {
var tree = trees.getFirst().make(inheritance);
inheritance = inheritance.remap(tree, tree.getMaxNamespaceId() - 1);
for (var other : trees.subList(1, trees.size())) {
var mappings = other.make(inheritance);
tree = chain(List.of(tree, mappings));
inheritance = inheritance.remap(mappings, mappings.getMaxNamespaceId() - 1);
}
return tree;
} catch (IOException e) {
throw new UncheckedIOException(e);
}
}

public static MappingTree chain(List<? extends MappingTree> trees) {
// This is a *non-transitive* chain, since some mappings (looking at you, intermediary) just... leave stuff out.
// Thus, its results may not be the most intuitive if you expect mapping "composition" or the like
// This can be avoided by "completing" mappings by their inheritance
var newTree = new MemoryMappingTree();
newTree.setSrcNamespace("namespace0");
int i = 0;
Expand Down Expand Up @@ -92,39 +122,12 @@ public static MappingTree chain(List<? extends MappingTreeView> trees) {

var trimmedTree = new MemoryMappingTree();
trimmedTree.setSrcNamespace("left");
var flatVisitor = FlatMappingVisitor.fromRegularVisitor(trimmedTree);
flatVisitor = new ForwardingFlatVisitor(flatVisitor) {
private boolean checkNames(@Nullable String[] dstNames) {
return dstNames != null && dstNames.length > 0 && Arrays.stream(dstNames).allMatch(Objects::nonNull);
}

@Override
public boolean visitClass(String srcName, @Nullable String[] dstNames) throws IOException {
return checkNames(dstNames) && super.visitClass(srcName, dstNames);
}

@Override
public boolean visitMethod(String srcClsName, String srcName, @Nullable String srcDesc, @Nullable String[] dstClsNames, @Nullable String[] dstNames, @Nullable String[] dstDescs) throws IOException {
return checkNames(dstNames) && super.visitMethod(srcClsName, srcName, srcDesc, dstClsNames, dstNames, dstDescs);
}

@Override
public boolean visitField(String srcClsName, String srcName, @Nullable String srcDesc, @Nullable String[] dstClsNames, @Nullable String[] dstNames, @Nullable String[] dstDescs) throws IOException {
return checkNames(dstNames) && super.visitField(srcClsName, srcName, srcDesc, dstClsNames, dstNames, dstDescs);
}

@Override
public boolean visitMethodArg(String srcClsName, String srcMethodName, @Nullable String srcMethodDesc, int argPosition, int lvIndex, @Nullable String srcName, @Nullable String[] dstClsNames, @Nullable String[] dstMethodNames, @Nullable String[] dstMethodDescs, String[] dstNames) throws IOException {
return checkNames(dstNames) && super.visitMethodArg(srcClsName, srcMethodName, srcMethodDesc, argPosition, lvIndex, srcName, dstClsNames, dstMethodNames, dstMethodDescs, dstNames);
}

@Override
public boolean visitMethodVar(String srcClsName, String srcMethodName, @Nullable String srcMethodDesc, int lvtRowIndex, int lvIndex, int startOpIdx, int endOpIdx, @Nullable String srcName, @Nullable String[] dstClsNames, @Nullable String[] dstMethodNames, @Nullable String[] dstMethodDescs, String[] dstNames) throws IOException {
return checkNames(dstNames) && super.visitMethodVar(srcClsName, srcMethodName, srcMethodDesc, lvtRowIndex, lvIndex, startOpIdx, endOpIdx, srcName, dstClsNames, dstMethodNames, dstMethodDescs, dstNames);
}
};
MappingVisitor trimmedVisitor = new MappingDstNsReorder(flatVisitor.asRegularVisitor(), List.of("right"));
MappingVisitor trimmedVisitor = MappingDropMissingSrcVisitor.create(trimmedTree);
trimmedVisitor = new MappingDstNsReorder(trimmedVisitor, List.of("right"));
trimmedVisitor = new MappingNsRenamer(trimmedVisitor, Map.of("namespace0", "left", "namespace"+i, "right"));
for (i = trees.size(); i > 0; i--) {
trimmedVisitor = new MappingNsCompleter(trimmedVisitor, Map.of("namespace" + i, "namespace" + (i - 1)));
}
try {
newTree.accept(trimmedVisitor);
} catch (IOException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@
import dev.lukebemish.taskgraphrunner.runtime.Task;
import dev.lukebemish.taskgraphrunner.runtime.TaskInput;
import dev.lukebemish.taskgraphrunner.runtime.execution.ToolDaemonExecutor;
import dev.lukebemish.taskgraphrunner.runtime.mappings.MappingInheritance;
import dev.lukebemish.taskgraphrunner.runtime.mappings.MappingsSourceImpl;
import dev.lukebemish.taskgraphrunner.runtime.mappings.MappingsUtil;
import dev.lukebemish.taskgraphrunner.runtime.mappings.ParchmentMappingWriter;
import dev.lukebemish.taskgraphrunner.runtime.util.Tools;
import net.fabricmc.mappingio.tree.MappingTree;
import org.jspecify.annotations.Nullable;

import java.io.File;
Expand All @@ -34,6 +36,7 @@ public class JstTask extends Task {
private final TaskInput.@Nullable FileListInput accessTransformers;
private final TaskInput.@Nullable FileListInput interfaceInjection;
private final @Nullable MappingsSourceImpl parchmentMappingsSource;
private final TaskInput.@Nullable HasFileInput binaryInput;
private final List<TaskInput> inputs;
private final List<ArgumentProcessor.Arg> args;
private final Map<String, String> outputExtensions;
Expand Down Expand Up @@ -90,6 +93,11 @@ public JstTask(TaskModel.Jst model, WorkItem workItem, Context context) {

this.inputs.addAll(args.stream().flatMap(ArgumentProcessor.Arg::inputs).toList());

this.binaryInput = model.binaryInput == null ? null : TaskInput.file("binaryInput", model.binaryInput, workItem, context, PathSensitivity.NONE);
if (this.binaryInput != null) {
inputs.add(this.binaryInput);
}

this.classpathScopedJvm = model.classpathScopedJvm;
}

Expand Down Expand Up @@ -121,7 +129,18 @@ private void collectArguments(ArrayList<String> command, Context context, Path w
// Might eventually look at making this configurable
command.add("--parchment-conflict-prefix=p");
var parchmentFile = workingDirectory.resolve("parchment.json");
var mappings = MappingsUtil.fixInnerClasses(parchmentMappingsSource.makeMappings(context));
MappingTree mappings;
if (binaryInput == null) {
mappings = MappingsUtil.fixInnerClasses(parchmentMappingsSource.makeMappings(context));
} else {
try {
var path = binaryInput.path(context);
var inheritance = MappingInheritance.read(path);
mappings = MappingsUtil.fixInnerClasses(parchmentMappingsSource.makeMappingsFillInheritance(context).make(inheritance));
} catch (IOException e) {
throw new UncheckedIOException(e);
}
}
try (var writer = Files.newBufferedWriter(parchmentFile, StandardCharsets.UTF_8);
var mappingWriter = new ParchmentMappingWriter(writer)) {
mappingWriter.accept(mappings);
Expand Down
Loading

0 comments on commit db285e9

Please sign in to comment.