Add proper support for JPMS in normal mods.toml mods (#10125)

Make FMLModContainer read and apply Add-Opens and Add-Exports manifest entries from mod files.
Forge is now the `net.minecraftforge.forge` module instead of `forge`

https://forums.minecraftforge.net/topic/153333-proper-java-module-support-in-forge-mods/

fmlearlydisplay/src/main/java/net/minecraftforge/fml/earlydisplay/DisplayWindow.java

@@ -22,9 +22,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.awt.Desktop;
-import java.io.BufferedReader;
 import java.io.IOException;
-import java.io.InputStreamReader;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 import java.net.URI;
@@ -302,13 +300,6 @@ public class DisplayWindow implements ImmediateWindowProvider {
     }
 
     private void crashElegantly(String errorDetails) {
-        String qrText;
-        try (var is = new BufferedReader(new InputStreamReader(getClass().getResourceAsStream("/glfailure.txt")))) {
-            qrText = is.lines().collect(Collectors.joining("\n"));
-        } catch (IOException ioe) {
-            qrText = "";
-        }
-
         StringBuilder msgBuilder = new StringBuilder(2000);
         msgBuilder.append("Failed to initialize graphics window with current settings.\n");
         msgBuilder.append("\n\n");
@@ -407,13 +398,13 @@ public class DisplayWindow implements ImmediateWindowProvider {
             if (showHelpLog && versidx == 0) {
                 LOGGER.info("""
                 If this message is the only thing at the bottom of your log before a crash, you probably have a driver issue.
-                
+
                 Possible solutions:
                 A) Make sure Minecraft is set to prefer high performance graphics in the OS and/or driver control panel
                 B) Check for driver updates on the graphics brand's website
                 C) Try reinstalling your graphics drivers
                 D) If still not working after trying all of the above, ask for further help on the Forge forums or Discord
-                
+
                 You can safely ignore this message if the game starts up successfully.""");
             }
             glfwWindowHint(GLFW_CONTEXT_VERSION_MAJOR, GL_VERSIONS[versidx][0]); // we try our versions one at a time
@@ -612,10 +603,25 @@ public class DisplayWindow implements ImmediateWindowProvider {
 
     @Override
     public void updateModuleReads(final ModuleLayer layer) {
-        var fm = layer.findModule("forge").orElseThrow();
-        getClass().getModule().addReads(fm);
-        var clz = FMLLoader.getGameLayer().findModule("forge").map(l->Class.forName(l, "net.minecraftforge.client.loading.ForgeLoadingOverlay")).orElseThrow();
-        loadingOverlay = Arrays.stream(clz.getDeclaredMethods()).filter(m-> Modifier.isStatic(m.getModifiers()) && m.getName().equals("newInstance")).findFirst().orElseThrow();
+        final var FORGE_MODULE = "net.minecraftforge.forge";
+        var forge_module = layer.findModule(FORGE_MODULE).orElse(null);
+        if (forge_module == null)
+            throw new IllegalStateException("Could not find " + FORGE_MODULE + " in " + layer);
+
+        getClass().getModule().addReads(forge_module);
+
+
+        var clz = Class.forName(forge_module, "net.minecraftforge.client.loading.ForgeLoadingOverlay");
+
+        for (var mtd : clz.getDeclaredMethods()) {
+            if (Modifier.isStatic(mtd.getModifiers()) && "newInstance".equals(mtd.getName())) {
+                this.loadingOverlay = mtd;
+                break;
+            }
+        }
+
+        if (loadingOverlay == null)
+            throw new IllegalStateException("Could not find static newInstace method in " + clz.getName());
     }
 
     public int getFramebufferTextureId() {

fmlloader/src/main/java/net/minecraftforge/fml/loading/ImmediateWindowHandler.java

@@ -140,7 +140,7 @@ public class ImmediateWindowHandler {
 
         @Override
         public void updateModuleReads(final ModuleLayer layer) {
-            var fm = layer.findModule("forge");
+            var fm = layer.findModule("net.minecraftforge.forge");
             if (fm.isPresent()) {
                 getClass().getModule().addReads(fm.get());
                 var clz = fm.map(l -> Class.forName(l, "net.minecraftforge.client.loading.NoVizFallback")).orElseThrow();

fmlloader/src/main/java/net/minecraftforge/fml/loading/UniqueModListBuilder.java

@@ -135,7 +135,7 @@ public class UniqueModListBuilder
             return modFile.getSecureJar().name();
         }
 
-        return modFile.getModFileInfo().moduleName();
+        return modFile.getModFileInfo().getMods().get(0).getModId();
     }
 
     public record UniqueModListData(List<ModFile> modFiles, Map<String, List<ModFile>> modFilesByFirstId) {}

fmlloader/src/main/java/net/minecraftforge/fml/loading/moddiscovery/AbstractModProvider.java

@@ -6,6 +6,7 @@
 package net.minecraftforge.fml.loading.moddiscovery;
 
 import com.mojang.logging.LogUtils;
+
 import cpw.mods.jarhandling.JarMetadata;
 import cpw.mods.jarhandling.SecureJar;
 import net.minecraftforge.fml.loading.LogMarkers;
@@ -22,19 +23,25 @@ import org.jetbrains.annotations.Nullable;
 import org.slf4j.Logger;
 
 import java.io.IOException;
+import java.lang.module.InvalidModuleDescriptorException;
+import java.lang.module.ModuleDescriptor;
+import java.lang.module.ModuleDescriptor.Version;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.function.Consumer;
 import java.util.jar.JarFile;
+import java.util.stream.Collectors;
 
 @ApiStatus.Internal
 public abstract class AbstractModProvider implements IModProvider {
-    private static final   Logger LOGGER    = LogUtils.getLogger();
-    protected static final String MODS_TOML = "META-INF/mods.toml";
+    private static final   Logger LOGGER      = LogUtils.getLogger();
+    protected static final String MODS_TOML   = "META-INF/mods.toml";
+    protected static final String MODULE_INFO = "module-info.class";
 
     protected IModLocator.ModFileOrException createMod(Path path) {
         return createMod(path, false);
@@ -48,10 +55,12 @@ public abstract class AbstractModProvider implements IModProvider {
     @Nullable
     protected IModLocator.ModFileOrException createMod(Path path, boolean ignoreUnknown, String defaultType) {
         var mjm = new ModJarMetadata();
-        var sj = SecureJar.from(
-            jar -> jar.moduleDataProvider().findFile(MODS_TOML).isPresent() ? mjm : JarMetadata.from(jar, path),
-            path
-        );
+        SecureJar sj = null;
+        try {
+            sj = SecureJar.from(jar -> loadMetaFromJar(jar, mjm), path);
+        } catch (Throwable t) {
+            return new IModLocator.ModFileOrException(null, new ModFileLoadingException("Failed to create secure jar for \"" + path + "\" - " + t.getMessage()));
+        }
 
         IModFile mod;
         var type = sj.moduleDataProvider().getManifest().getMainAttributes().getValue(ModFile.TYPE);
@@ -61,6 +70,8 @@ public abstract class AbstractModProvider implements IModProvider {
         if (sj.moduleDataProvider().findFile(MODS_TOML).isPresent()) {
             LOGGER.debug(LogMarkers.SCAN, "Found {} mod of type {}: {}", MODS_TOML, type, path);
             mod = new ModFile(sj, this, ModFileParser::modsTomlParser);
+            // ModJarMetadata is only used when loading mods.toml mods as ModJarMetadata
+            mjm.setModFile(mod);
             if (mod.getModFileInfo().getFileProperties().containsKey(ModFileInfo.NOT_A_FORGE_MOD_PROP)) {
                 LOGGER.error(LogMarkers.SCAN, "Unable to load file \"{}\" because its mods.toml is requesting an invalid javafml loaderVersion (use \"*\" if you want to allow all versions) and is missing a forge modId dependency declaration (see the sample mods.toml in the MDK).", path);
                 return new IModLocator.ModFileOrException(null, new ModFileLoadingException("File \"%s\" is not a Forge mod and cannot be loaded. Look for a Forge version of this mod or consider alternative mods.".formatted(mod.getFileName())));
@@ -73,10 +84,54 @@ public abstract class AbstractModProvider implements IModProvider {
         } else
             return new IModLocator.ModFileOrException(null, new ModFileLoadingException("Invalid mod file found " + path));
 
-        mjm.setModFile(mod);
         return new IModLocator.ModFileOrException(mod, null);
     }
 
+    private static JarMetadata loadMetaFromJar(SecureJar jar, ModJarMetadata mjm) {
+        var info = jar.moduleDataProvider().open(MODULE_INFO).orElse(null);
+        if (info != null) {
+            try {
+                var desc = ModuleDescriptor.read(info, jar::getPackages);
+                var all = new HashSet<>(jar.getPackages());
+                all.removeAll(desc.packages());
+                if (!all.isEmpty()) {
+                    var missing = all.stream().sorted().collect(Collectors.joining(", "));
+                    LOGGER.error("Invalid module-info, missing packages " + missing);
+                    throw new ModFileLoadingException("Invalid module-info, missing packages " + missing);
+                } else {
+                    return new JarMetadata() {
+                        @Override
+                        public String name() {
+                            return desc.name();
+                        }
+
+                        @Override
+                        public String version() {
+                            return desc.version().map(Version::toString).or(desc::rawVersion).orElse(null);
+                        }
+
+                        @Override
+                        public ModuleDescriptor descriptor() {
+                            return desc;
+                        }
+                    };
+                }
+            } catch (InvalidModuleDescriptorException | IOException e) {
+                LOGGER.error("Failed to parse " + jar.getPrimaryPath() + " module-info", e);
+                throw new ModFileLoadingException("Invalid module-info: " + e.getMessage());
+            } finally {
+                try {
+                    info.close();
+                } catch (IOException e) {}
+            }
+        }
+
+        if (jar.moduleDataProvider().findFile(MODS_TOML).isEmpty())
+            return JarMetadata.from(jar, jar.getPrimaryPath());
+
+        return mjm;
+    }
+
     protected IModFileInfo manifestParser(final IModFile mod) {
         var mf = mod.getSecureJar().moduleDataProvider().getManifest().getMainAttributes();
         var license = mf.getValue("LICENSE");

fmlloader/src/main/java/net/minecraftforge/fml/loading/moddiscovery/MinecraftLocator.java

@@ -22,13 +22,11 @@ public final class MinecraftLocator extends AbstractModProvider implements IModL
     @Override
     public List<IModLocator.ModFileOrException> scanMods() {
         var minecraft = FMLLoader.getLaunchHandler().getMinecraftPaths();
+        var paths = minecraft.toArray(Path[]::new);
 
         // Minecraft itself.
         var meta = new ModJarMetadata();
-        var mcjar = SecureJar.from(
-            jar -> meta,
-            minecraft.toArray(Path[]::new)
-        );
+        var mcjar = SecureJar.from(jar -> meta, paths);
         var mc = ModFileFactory.FACTORY.build(mcjar, this, MinecraftLocator::buildMinecraftTOML);
         meta.setModFile(mc);
 

fmlloader/src/main/java/net/minecraftforge/fml/loading/moddiscovery/ModFileInfo.java

@@ -259,7 +259,7 @@ public class ModFileInfo implements IModFileInfo, IConfigurable {
 
     @Override
     public String moduleName() {
-        return getMods().get(0).getModId();
+        return this.modFile.getSecureJar().name();
     }
 
     @Override

fmlloader/src/main/java/net/minecraftforge/fml/loading/moddiscovery/ModJarMetadata.java

@@ -9,49 +9,64 @@ import cpw.mods.jarhandling.JarMetadata;
 import net.minecraftforge.forgespi.locating.IModFile;
 import java.lang.module.ModuleDescriptor;
 import java.util.Objects;
-
 import org.jetbrains.annotations.ApiStatus;
 
 @ApiStatus.Internal
 public final class ModJarMetadata implements JarMetadata {
+    private static final String AUTOMATIC_MODULE_NAME = "Automatic-Module-Name";
     private IModFile modFile;
+    private String name;
+    private String version;
     private ModuleDescriptor descriptor;
 
-    ModJarMetadata() {
-    }
+    ModJarMetadata() { }
 
     public void setModFile(IModFile file) {
         this.modFile = file;
+        var mods = this.modFile.getModFileInfo().getMods();
+
+        if (!mods.isEmpty()) {
+            var main = mods.get(0);
+            this.name = main.getModId();
+            this.version = main.getVersion().toString();
+        }
+
+        var jar = file.getSecureJar();
+        var aname = jar.moduleDataProvider().getManifest().getMainAttributes().getValue(AUTOMATIC_MODULE_NAME);
+        if (aname != null)
+            this.name = aname;
     }
 
     @Override
     public String name() {
-        return modFile.getModFileInfo().moduleName();
+        return this.name;
     }
 
     @Override
     public String version() {
-        return modFile.getModFileInfo().versionString();
+        return this.version;
     }
 
     @Override
     public ModuleDescriptor descriptor() {
-        if (descriptor != null) return descriptor;
+        if (descriptor != null)
+            return descriptor;
+
         var bld = ModuleDescriptor.newAutomaticModule(name())
-                .version(version())
-                .packages(modFile.getSecureJar().getPackages());
-        modFile.getSecureJar().getProviders().stream()
-                .filter(p -> !p.providers().isEmpty())
-                .forEach(p -> bld.provides(p.serviceName(), p.providers()));
-        modFile.getModFileInfo().usesServices().forEach(bld::uses);
+            .version(version())
+            .packages(modFile.getSecureJar().getPackages());
+
+        for (var provider : modFile.getSecureJar().getProviders()) {
+            if (provider.providers().isEmpty())
+                continue;
+
+            bld.provides(provider.serviceName(), provider.providers());
+        }
+
         descriptor = bld.build();
         return descriptor;
     }
 
-    public IModFile modFile() {
-        return modFile;
-    }
-
     @Override
     public boolean equals(Object obj) {
         if (obj == this) return true;

fmlloader/src/main/java/net/minecraftforge/fml/loading/targets/CommonLaunchHandler.java

@@ -73,7 +73,7 @@ public abstract class CommonLaunchHandler implements ILaunchHandlerService {
     protected static final LaunchType CLIENT          = new LaunchType("client", "minecraft", "net.minecraft.client.main.Main", Dist.CLIENT, false);
     protected static final LaunchType DATA            = new LaunchType("data",   "minecraft", "net.minecraft.data.Main", Dist.CLIENT, true);
     protected static final LaunchType SERVER          = new LaunchType("server", "minecraft", "net.minecraft.server.Main", Dist.DEDICATED_SERVER, false);
-    protected static final LaunchType SERVER_GAMETEST = new LaunchType("server_gametest", "forge", "net.minecraftforge.gametest.GameTestMain", Dist.DEDICATED_SERVER, false);
+    protected static final LaunchType SERVER_GAMETEST = new LaunchType("server_gametest", "net.minecraftforge.forge", "net.minecraftforge.gametest.GameTestMain", Dist.DEDICATED_SERVER, false);
 
     protected void runTarget(String module, String target, final String[] arguments, final ModuleLayer layer) throws Throwable {
         var mod = layer.findModule(module).orElse(null);

fmlloader/src/main/java/net/minecraftforge/fml/loading/targets/ForgeDevLocator.java

@@ -91,23 +91,24 @@ public final class ForgeDevLocator extends AbstractModProvider implements IModLo
         // First lets find all class files, that have the @Mod annotation and map packages to modids.
         var packages = findTestModPackages(path);
         for (var entry : packages.entrySet()) {
-            var pkg = entry.getKey();
+            var pkg = entry.getKey() + '/';
             var modids = entry.getValue();
 
             // Find resource directories for every mod in this group
-            var resourcePaths = new LinkedHashSet<Path>();
+            var paths = new LinkedHashSet<Path>();
             for (var modid : modids) {
                 var rsc = path.resolve(modid);
                 if (Files.exists(rsc))
-                    resourcePaths.add(rsc);
+                    paths.add(rsc);
             }
 
             var root = memory.getPath(modids.iterator().next()); // use the first modid as our root
-            buildModsToml(resourcePaths, modids, root);
-            buildPackMeta(resourcePaths, root);
+            buildModsToml(paths, modids, root);
+            buildPackMeta(paths, root);
+            moveModuleInfo(paths, root);
 
             if (Files.exists(root))
-                resourcePaths.add(root);
+                paths.add(root);
 
             // We want just the class files from the root of the input paths. So make a new union with a filter.
             var classes = UnionHelper.newFileSystem(
@@ -121,14 +122,10 @@ public final class ForgeDevLocator extends AbstractModProvider implements IModLo
                 }, new Path[] { path }
             );
 
-            // And we want the resources, and none of the classes, just in case things get kinda fucky and somehow a package overlaps a sub package {shading?}
-            var resources = UnionHelper.newFileSystem((name, base) -> !name.endsWith(".class"), resourcePaths.toArray(Path[]::new));
+            paths.addFirst(classes.getRootDirectories().iterator().next());
 
             // Union of unions, Yay!
-            var union = UnionHelper.newFileSystem(null, new Path[] {
-                classes.getRootDirectories().iterator().next(),
-                resources.getRootDirectories().iterator().next()
-            });
+            var union = UnionHelper.newFileSystem(null, paths.stream().toArray(Path[]::new));
             mod.add(union.getRootDirectories().iterator().next());
         }
 
@@ -274,6 +271,25 @@ public final class ForgeDevLocator extends AbstractModProvider implements IModLo
         }
     }
 
+    private static void moveModuleInfo(Set<Path> paths, Path root) {
+        var existing = paths.stream()
+            .map(p -> p.resolve("module-info.dat"))
+            .filter(Files::exists)
+            .findFirst()
+            .orElse(null);
+
+        if (existing == null)
+            return;
+
+        var target = root.resolve("module-info.class");
+        try {
+            Files.createDirectories(target.getParent());
+            Files.copy(existing, target);
+        } catch (IOException e) {
+            throw new IllegalStateException("Failed to copy module-info.dat to memory: " + target, e);
+        }
+    }
+
     @SuppressWarnings("unchecked")
     private static <E extends Throwable, R> R sneak(Throwable e) throws E {
         throw (E)e;

javafmllanguage/build.gradle

@@ -13,6 +13,8 @@ dependencies {
     compileOnly(libs.nulls)
     implementation(project(':fmlloader'))
     implementation(project(':fmlcore'))
+    implementation(libs.unsafe)
+    implementation(libs.securemodules)
 }
 
 java {
@@ -57,7 +59,7 @@ publishing {
             license PomUtils.Licenses.LGPLv2_1
         }
     }
-    
+
     repositories {
         maven gradleutils.publishingForgeMaven
     }

javafmllanguage/src/main/java/net/minecraftforge/fml/javafmlmod/FMLModContainer.java

@@ -16,15 +16,21 @@ import net.minecraftforge.fml.ModLoadingStage;
 import net.minecraftforge.fml.event.IModBusEvent;
 import net.minecraftforge.forgespi.language.IModInfo;
 import net.minecraftforge.forgespi.language.ModFileScanData;
+import net.minecraftforge.unsafe.UnsafeHacks;
+
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.Marker;
 import org.apache.logging.log4j.MarkerManager;
 
+import cpw.mods.jarhandling.SecureJar;
+
 import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
 import java.util.Objects;
 import java.util.Optional;
+import java.util.jar.Attributes;
 
 public class FMLModContainer extends ModContainer {
     private static final Logger LOGGER = LogManager.getLogger();
@@ -47,14 +53,82 @@ public class FMLModContainer extends ModContainer {
             var moduleName = info.getOwningFile().moduleName();
             var module = gameLayer.findModule(moduleName)
                 .orElseThrow(() -> new IllegalStateException("Failed to find " + moduleName + " in " + gameLayer));
+
+            openModules(gameLayer, module, info.getOwningFile().getFile().getSecureJar());
+
             modClass = Class.forName(module, className);
-            LOGGER.trace(LOADING,"Loaded modclass {} with {}", modClass.getName(), modClass.getClassLoader());
+            LOGGER.debug(LOADING,"Loaded modclass {}/{} with {}", modClass.getModule().getName(), modClass.getName(), modClass.getClassLoader());
         } catch (Throwable e) {
             LOGGER.error(LOADING, "Failed to load class {}", className, e);
             throw new ModLoadingException(info, ModLoadingStage.CONSTRUCT, "fml.modloading.failedtoloadmodclass", e);
         }
     }
 
+    /**
+     * <pre>
+     * Reads the Add-Exports and Add-Opens attributes (see note) from a mod file and
+     * attempts to apply them. This differers from the JEP in two significant ways:
+     *   1) Instead of opening things to ALL-UNNAMED it instead only opens to the
+     *      mod's module itself. Because I don't want other mods accidently relying
+     *      on transitive behavior
+     *   2) It is read from all mods not just the executable jars.
+     *
+     * From <a href = "https://openjdk.org/jeps/261">JEP 261: Module System</a>
+     *
+     * Two new JDK-specific JAR-file manifest attributes are defined to correspond
+     * to the --add-exports and --add-opens command-line options:
+     *     Add-Exports: &lt;module&gt;/&lt;package&gt;( &lt;module&gt;/&lt;package&gt;)*
+     *     Add-Opens: &lt;module&gt;/&lt;package&gt;( &lt;module&gt;/&lt;package&gt;)*
+     *
+     * The value of each attribute is a space-separated list of slash-separated
+     * module-name/package-name pairs. A &gt;module&lt;/&gt;package&lt; pair in
+     * the value of an Add-Exports attribute has the same meaning as the
+     * command-line option --add-exports &gt;module&lt;/&gt;package&lt;=ALL-UNNAMED.
+     *
+     * A &gt;module&lt;/&gt;package&lt; pair in the value of an Add-Opens attribute has the
+     * same meaning as the command-line option --add-opens &gt;module&lt;/&gt;package&lt;=ALL-UNNAMED.
+     *
+     * Each attribute can occur at most once, in the main section of a MANIFEST.MF file.
+     * A particular pair can be listed more than once. If a specified module was not
+     * resolved, or if a specified package does not exist, then the corresponding pair
+     * is ignored.
+     */
+    private static void openModules(ModuleLayer layer, Module self, SecureJar jar) throws NoSuchMethodException, SecurityException, IllegalAccessException, InvocationTargetException {
+        var manifest = jar.moduleDataProvider().getManifest().getMainAttributes();
+        addOpenOrExports(layer, self, true, manifest);
+        addOpenOrExports(layer, self, false, manifest);
+    }
+
+    private static void addOpenOrExports(ModuleLayer layer, Module self, boolean open, Attributes attrs) throws NoSuchMethodException, SecurityException, IllegalAccessException, InvocationTargetException {
+        var key = open ? "Add-Opens" : "Add-Exports";
+        var entry = attrs.getValue(key);
+        if (entry == null)
+            return;
+
+        for (var pair : entry.split(" ")) {
+            var pts = pair.trim().split("/");
+            if (pts.length == 2) {
+                var target = layer.findModule(pts[0]).orElse(null);
+                if (target == null || !target.getDescriptor().packages().contains(pts[1]))
+                    continue;
+                addOpenOrExport(target, pts[1], self, open);
+            } else {
+                LOGGER.warn(LOADING, "Invalid {} entry in {}: {}", key, self.getName(), pair);
+            }
+        }
+    }
+
+    private static Method implAddExportsOrOpens;
+    private static void addOpenOrExport(Module target, String pkg, Module reader, boolean open) throws NoSuchMethodException, SecurityException, IllegalAccessException, InvocationTargetException {
+        if (implAddExportsOrOpens == null) {
+            implAddExportsOrOpens = Module.class.getDeclaredMethod("implAddExportsOrOpens", String.class, Module.class, boolean.class, boolean.class);
+            UnsafeHacks.setAccessible(implAddExportsOrOpens);
+        }
+
+        LOGGER.info(LOADING, "{} {}/{} to {}", open ? "Opening" : "Exporting", target.getName(), pkg, reader.getName());
+        implAddExportsOrOpens.invoke(target, pkg, reader, open, /*syncVM*/true);
+    }
+
     private static void onEventFailed(IEventBus iEventBus, Event event, IEventListener[] iEventListeners, int i, Throwable throwable) {
         LOGGER.error(new EventBusErrorMessage(event, i, iEventListeners, throwable));
     }

src/generated_test/resources/criterion_test/data/criterion_test/advancements/break_glass_with_fish.json

@@ -1,37 +0,0 @@
-{
-  "parent": "minecraft:story/root",
-  "criteria": {
-    "break_glass_with_fish": {
-      "conditions": {
-        "breakingBlock": {
-          "tag": "forge:glass"
-        },
-        "holdingItem": {
-          "tag": "criterion_test:fish"
-        }
-      },
-      "trigger": "criterion_test:criterion"
-    }
-  },
-  "display": {
-    "announce_to_chat": true,
-    "description": {
-      "text": "Fish wins!"
-    },
-    "frame": "task",
-    "hidden": false,
-    "icon": {
-      "item": "minecraft:cod"
-    },
-    "show_toast": true,
-    "title": {
-      "text": "Fish vs Glass"
-    }
-  },
-  "requirements": [
-    [
-      "break_glass_with_fish"
-    ]
-  ],
-  "sends_telemetry_event": false
-}

src/generated_test/resources/criterion_test/data/criterion_test/tags/items/fish.json

@@ -1,10 +0,0 @@
-{
-  "values": [
-    "minecraft:cod",
-    "minecraft:salmon",
-    "minecraft:tropical_fish",
-    "minecraft:pufferfish",
-    "minecraft:cooked_cod",
-    "minecraft:cooked_salmon"
-  ]
-}

src/main/java/net/minecraftforge/gametest/ForgeGameTestHooks.java

@@ -182,10 +182,10 @@ public class ForgeGameTestHooks {
         var batch = func.batchName();
         classes.add(batch);
 
-        int idx = batch.indexOf('.');
+        int idx = batch.lastIndexOf('.');
         while (idx != -1) {
             batch = batch.substring(0, idx);
-            idx = batch.indexOf('.');
+            idx = batch.lastIndexOf('.');
             classes.add(batch);
         }
     }

src/test/generated/closed_module_test/META-INF/MANIFEST.MF

@@ -0,0 +1,4 @@
+Manifest-Version: 1.0
+Add-Opens: net.minecraftforge.debug.modules.closed/net.minecraftforge.de
+ bug.modules.closed.internala
+

src/test/generated/criterion_test/data/criterion_test/advancement/break_glass_with_fish.json

@@ -5,7 +5,7 @@
       "conditions": {
         "allowOffhand": true,
         "breakingBlock": {
-          "blocks": "#forge:glass"
+          "blocks": "#c:glass_blocks"
         },
         "holdingItem": {
           "items": "#criterion_test:fish"

src/test/java/net/minecraftforge/debug/gameplay/crafting/TagsTest.java

@@ -1,3 +1,8 @@
+/*
+ * Copyright (c) Forge Development LLC and contributors
+ * SPDX-License-Identifier: LGPL-2.1-only
+ */
+
 package net.minecraftforge.debug.gameplay.crafting;
 
 import net.minecraft.gametest.framework.GameTest;

src/test/java/net/minecraftforge/debug/modules/automatic/AutomaticModuleMod.java

@@ -0,0 +1,32 @@
+/*
+ * Copyright (c) Forge Development LLC and contributors
+ * SPDX-License-Identifier: LGPL-2.1-only
+ */
+
+package net.minecraftforge.debug.modules.automatic;
+
+import net.minecraft.gametest.framework.GameTest;
+import net.minecraft.gametest.framework.GameTestHelper;
+import net.minecraftforge.fml.common.Mod;
+import net.minecraftforge.fml.javafmlmod.FMLJavaModLoadingContext;
+import net.minecraftforge.gametest.GameTestHolder;
+import net.minecraftforge.test.BaseTestMod;
+
+@Mod(AutomaticModuleMod.MODID)
+@GameTestHolder("forge.module.automatic")
+public class AutomaticModuleMod extends BaseTestMod {
+    public static final String MODID = "automatic_module";
+
+    public AutomaticModuleMod(FMLJavaModLoadingContext context) {
+        super(context);
+    }
+
+    @GameTest(template = "forge:empty3x3x3")
+    public static void correct_name(GameTestHelper helper) throws ReflectiveOperationException {
+        var mod = AutomaticModuleMod.class.getModule();
+        if ("net.minecraftforge.debug.modules.automatic".equals(mod.getName()))
+            helper.succeed();
+        else
+            helper.fail("Invalid module name: " + mod.getName());
+    }
+}

src/test/java/net/minecraftforge/debug/modules/closed/ClosedMod.java

@@ -0,0 +1,53 @@
+/*
+ * Copyright (c) Forge Development LLC and contributors
+ * SPDX-License-Identifier: LGPL-2.1-only
+ */
+
+package net.minecraftforge.debug.modules.closed;
+
+import java.lang.module.ModuleDescriptor;
+import java.util.Set;
+import net.minecraftforge.data.event.GatherDataEvent;
+import net.minecraftforge.debug.modules.closed.api.PublicUtils;
+import net.minecraftforge.debug.modules.closed.internala.InternalA;
+import net.minecraftforge.debug.modules.closed.internalb.InternalB;
+import net.minecraftforge.eventbus.api.SubscribeEvent;
+import net.minecraftforge.fml.common.Mod;
+import net.minecraftforge.fml.javafmlmod.FMLJavaModLoadingContext;
+import net.minecraftforge.test.BaseTestMod;
+import net.minecraftforge.test.ModuleProvider;
+
+@Mod(ClosedMod.MODID)
+public class ClosedMod extends BaseTestMod {
+    public static final String MODID = "closed_module";
+
+    public ClosedMod(FMLJavaModLoadingContext context) {
+        super(context);
+    }
+
+    @SubscribeEvent
+    public void runData(GatherDataEvent event) {
+        var out = event.getGenerator().getPackOutput();
+        event.getGenerator().addProvider(true, new ModuleProvider(out, module()));
+    }
+
+    private ModuleDescriptor module() {
+        var self = ClosedMod.class.getPackageName();
+        var api = PublicUtils.class.getPackageName();
+        var internalA = InternalA.class.getPackageName();
+        var internalB = InternalB.class.getPackageName();
+        var forge = Set.of(
+            "net.minecraftforge.javafmlmod",
+            "net.minecraftforge.eventbus",
+            "net.minecraftforge.fmlcore",
+            "net.minecraftforge.forge"
+        );
+        var bldr = ModuleDescriptor.newModule(self)
+            .packages(Set.of(self, api, internalA, internalB))
+            .opens(api)
+            .exports(api)
+            .opens(self, forge);
+        forge.forEach(bldr::requires);
+        return bldr.build();
+    }
+}

src/test/java/net/minecraftforge/debug/modules/closed/api/PublicUtils.java

@@ -0,0 +1,12 @@
+/*
+ * Copyright (c) Forge Development LLC and contributors
+ * SPDX-License-Identifier: LGPL-2.1-only
+ */
+
+package net.minecraftforge.debug.modules.closed.api;
+
+public class PublicUtils {
+    public static void publicMethod() {
+        System.out.println("Public method called");
+    }
+}

src/test/java/net/minecraftforge/debug/modules/closed/internala/InternalA.java

@@ -0,0 +1,12 @@
+/*
+ * Copyright (c) Forge Development LLC and contributors
+ * SPDX-License-Identifier: LGPL-2.1-only
+ */
+
+package net.minecraftforge.debug.modules.closed.internala;
+
+public class InternalA {
+    public static void internalMethod() {
+        System.out.println("Internal A method called");
+    }
+}

src/test/java/net/minecraftforge/debug/modules/closed/internalb/InternalB.java

@@ -0,0 +1,12 @@
+/*
+ * Copyright (c) Forge Development LLC and contributors
+ * SPDX-License-Identifier: LGPL-2.1-only
+ */
+
+package net.minecraftforge.debug.modules.closed.internalb;
+
+public class InternalB {
+    public static void internalMethod() {
+        System.out.println("Internal B method called");
+    }
+}

src/test/java/net/minecraftforge/debug/modules/closedtest/ClosedTestsMod.java

@@ -0,0 +1,99 @@
+/*
+ * Copyright (c) Forge Development LLC and contributors
+ * SPDX-License-Identifier: LGPL-2.1-only
+ */
+
+package net.minecraftforge.debug.modules.closedtest;
+
+import java.lang.module.ModuleDescriptor;
+import java.util.jar.Manifest;
+
+import net.minecraftforge.data.event.GatherDataEvent;
+import net.minecraftforge.debug.modules.closed.api.PublicUtils;
+import net.minecraftforge.eventbus.api.SubscribeEvent;
+import net.minecraftforge.fml.common.Mod;
+import net.minecraftforge.fml.javafmlmod.FMLJavaModLoadingContext;
+import net.minecraftforge.gametest.GameTestHolder;
+import net.minecraftforge.test.BaseTestMod;
+import net.minecraftforge.test.ManifestProvider;
+import net.minecraftforge.test.ModuleProvider;
+import net.minecraft.gametest.framework.GameTest;
+import net.minecraft.gametest.framework.GameTestHelper;
+
+@Mod(ClosedTestsMod.MODID)
+@GameTestHolder("forge.module.closed")
+public class ClosedTestsMod extends BaseTestMod {
+    public static final String MODID = "closed_module_test";
+
+    public ClosedTestsMod(FMLJavaModLoadingContext context) {
+        super(context);
+    }
+
+    @SubscribeEvent
+    public void runData(GatherDataEvent event) {
+        var out = event.getGenerator().getPackOutput();
+        event.getGenerator().addProvider(true, new ModuleProvider(out, module()));
+        event.getGenerator().addProvider(true, new ManifestProvider(out, MODID, manifest()));
+    }
+
+    private ModuleDescriptor module() {
+        return ModuleDescriptor.newOpenModule(getClass().getPackageName())
+            .requires("net.minecraftforge.eventbus")
+            .requires("net.minecraftforge.fmlcore")
+            .requires("net.minecraftforge.forge")
+            .requires("net.minecraftforge.javafmlmod")
+            .requires("net.minecraftforge.debug.modules.closed")
+            .build();
+    }
+
+    private Manifest manifest() {
+        var ret = new Manifest();
+        // Add-Opens is respected by FMLModContainer, it should give us access to closed packages
+        ret.getMainAttributes().putValue("Add-Opens", "net.minecraftforge.debug.modules.closed/net.minecraftforge.debug.modules.closed.internala" );
+        return ret;
+    }
+
+    /*
+     * This attempted to access a class and method that IS exported by ClosedMod
+     * Should succeed with no issues
+     */
+    @GameTest(template = "forge:empty3x3x3")
+    public static void can_reflect_exported(GameTestHelper helper) throws ReflectiveOperationException {
+        var method = PublicUtils.class.getDeclaredMethod("publicMethod");
+        method.invoke(null);
+        helper.succeed();
+    }
+
+    /*
+     * This is opened by us having an Add-Opens entry in ClosedTest's manifest
+     * It is NOT exported/opened by ClosedMod
+     * Should succeed with no exceptions
+     */
+    @GameTest(template = "forge:empty3x3x3")
+    public static void can_reflect_opened_internal(GameTestHelper helper) throws ReflectiveOperationException {
+        try {
+            var cls = Class.forName("net.minecraftforge.debug.modules.closed.internala.InternalA");
+            var method = cls.getDeclaredMethod("internalMethod");
+            method.invoke(null);
+            helper.succeed();
+        } catch (IllegalAccessException e) {
+            helper.fail("Failed to invoke internal method: " + e.getMessage());
+        }
+    }
+
+    /*
+     * This is NOT opened by us, and is NOT exported by ClosedTest
+     * This should error with IllegalAccessException because java is enforcing access control.
+     */
+    @GameTest(template = "forge:empty3x3x3")
+    public static void cant_reflect_internal(GameTestHelper helper) throws ReflectiveOperationException {
+        try {
+            var cls = Class.forName("net.minecraftforge.debug.modules.closed.internalb.InternalB");
+            var method = cls.getDeclaredMethod("internalMethod");
+            method.invoke(null);
+            helper.fail("Invoked internal method without error");
+        } catch (IllegalAccessException e) {
+            helper.succeed();
+        }
+    }
+}

src/test/java/net/minecraftforge/test/ManifestProvider.java

@@ -0,0 +1,55 @@
+/*
+ * Copyright (c) Forge Development LLC and contributors
+ * SPDX-License-Identifier: LGPL-2.1-only
+ */
+
+package net.minecraftforge.test;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.util.concurrent.CompletableFuture;
+import java.util.jar.JarFile;
+import java.util.jar.Manifest;
+import java.util.jar.Attributes;
+import com.google.common.hash.Hashing;
+import net.minecraft.Util;
+import net.minecraft.data.CachedOutput;
+import net.minecraft.data.DataProvider;
+import net.minecraft.data.PackOutput;
+
+public class ManifestProvider implements DataProvider {
+    private final PackOutput output;
+    private final String name;
+    private final Manifest manifest;
+
+    public ManifestProvider(PackOutput output, String name, Manifest manifest) {
+        this.output = output;
+        this.name = name;
+        this.manifest = manifest;
+    }
+
+    @Override
+    public String getName() {
+        return "ManifestProvider[" + this.name + ']';
+    }
+
+    @Override
+    public CompletableFuture<?> run(CachedOutput cache) {
+        return CompletableFuture.runAsync(() -> {
+            var target = this.output.getOutputFolder().resolve(JarFile.MANIFEST_NAME);
+            try {
+                if (this.manifest.getMainAttributes().getValue(Attributes.Name.MANIFEST_VERSION) == null)
+                    this.manifest.getMainAttributes().put(Attributes.Name.MANIFEST_VERSION, "1.0");
+
+                var bos = new ByteArrayOutputStream();
+                this.manifest.write(bos);
+                var data = bos.toByteArray();
+                @SuppressWarnings("deprecation")
+                var hash = Hashing.sha1().hashBytes(data);
+                cache.writeIfNeeded(target, data, hash);
+            } catch (IOException ioexception) {
+                LOGGER.error("Failed to save file to {}", target, ioexception);
+            }
+        }, Util.backgroundExecutor());
+    }
+}

src/test/java/net/minecraftforge/test/ModuleProvider.java

@@ -0,0 +1,129 @@
+/*
+ * Copyright (c) Forge Development LLC and contributors
+ * SPDX-License-Identifier: LGPL-2.1-only
+ */
+
+package net.minecraftforge.test;
+
+import java.io.IOException;
+import java.lang.module.ModuleDescriptor;
+import java.lang.module.ModuleDescriptor.Exports;
+import java.lang.module.ModuleDescriptor.Opens;
+import java.lang.module.ModuleDescriptor.Provides;
+import java.lang.module.ModuleDescriptor.Requires;
+import java.lang.module.ModuleDescriptor.Version;
+import java.lang.reflect.AccessFlag;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+import java.util.Optional;
+import java.util.Set;
+import java.util.concurrent.CompletableFuture;
+import java.util.function.Function;
+
+import org.objectweb.asm.ClassWriter;
+import org.objectweb.asm.Opcodes;
+
+import com.google.common.hash.Hashing;
+import net.minecraft.Util;
+import net.minecraft.data.CachedOutput;
+import net.minecraft.data.DataProvider;
+import net.minecraft.data.PackOutput;
+
+public class ModuleProvider implements DataProvider {
+    private final PackOutput output;
+    private final ModuleDescriptor desc;
+
+    public ModuleProvider(PackOutput output, ModuleDescriptor desc) {
+        this.output = output;
+        this.desc = desc;
+    }
+
+    @Override
+    public String getName() {
+        return "ModuleProvider[" + this.desc.name() + ']';
+    }
+
+    @Override
+    public CompletableFuture<?> run(CachedOutput cache) {
+        return CompletableFuture.runAsync(() -> {
+            var target = this.output.getOutputFolder().resolve("module-info.dat");
+            try {
+                var data = writeModuleInfo();
+                @SuppressWarnings("deprecation")
+                var hash = Hashing.sha1().hashBytes(data);
+                cache.writeIfNeeded(target, data, hash);
+            } catch (IOException ioexception) {
+                LOGGER.error("Failed to save file to {}", target, ioexception);
+            }
+        }, Util.backgroundExecutor());
+    }
+
+    private byte[] writeModuleInfo() {
+        var writer = new ClassWriter(0);
+
+        writer.visit(Opcodes.V9, Opcodes.ACC_MODULE, "module-info", null, null, null);
+
+        var module = writer.visitModule(desc.name(), flags(desc.accessFlags()), version(desc.version(), desc.rawVersion()));
+
+        desc.mainClass().ifPresent(module::visitMainClass);
+
+        for (var pkg : sorted(desc.packages(), Function.identity()))
+            module.visitPackage(binary(pkg));
+
+        for (var req : sorted(desc.requires(), Requires::name))
+            module.visitRequire(req.name(), flags(req.accessFlags()), version(req.compiledVersion(), req.rawCompiledVersion()));
+
+        for (var exp : sorted(desc.exports(), Exports::source))
+            module.visitExport(binary(exp.source()), flags(exp.accessFlags()), array(exp.targets()));
+
+        for (var open : sorted(desc.opens(), Opens::source))
+            module.visitOpen(binary(open.source()), flags(open.accessFlags()), array(open.targets()));
+
+        for (var uses : sorted(desc.uses(), Function.identity()))
+            module.visitUse(binary(uses));
+
+        for (var provide : sorted(desc.provides(), Provides::service)) {
+            var providers = new ArrayList<String>();
+            for (var provider : provide.providers())
+                providers.add(binary(provider));
+
+            module.visitProvide(provide.service(), array(providers));
+        }
+
+        module.visitEnd();
+
+        return writer.toByteArray();
+    }
+
+    private static int flags(Set<AccessFlag> flags) {
+        int access = 0;
+        for (var flag : flags)
+            access |= flag.mask();
+        return access;
+    }
+
+    private static String[] array(Collection<String> lst) {
+        return lst.stream().toArray(String[]::new);
+    }
+
+    private static String binary(String cls) {
+        return cls.replace('.', '/');
+    }
+
+    private static String version(Optional<Version> ver, Optional<String> str) {
+        var version = ver.map(Version::toString).orElse(null);
+        if (version == null)
+            return str.orElse(null);
+        return version;
+    }
+
+    private static <T> List<T> sorted(Collection<T> data, Function<T, String> toString) {
+        var ret = new ArrayList<T>();
+        ret.addAll(data);
+        Collections.sort(ret, (a, b) -> toString.apply(a).compareTo(toString.apply(b)));
+        return ret;
+    }
+
+}

src/test/resources/automatic_module/META-INF/MANIFEST.MF

@@ -0,0 +1 @@
+Automatic-Module-Name: net.minecraftforge.debug.modules.automatic