Skip to content

Add the NeoForgedRepositoryFilter to address the issues associated with NeoForge Maven using the Maven Central repository#329

Closed
Gu-ZT wants to merge 7 commits into
neoforged:mainfrom
Gu-ZT:feature/fix/327
Closed

Add the NeoForgedRepositoryFilter to address the issues associated with NeoForge Maven using the Maven Central repository#329
Gu-ZT wants to merge 7 commits into
neoforged:mainfrom
Gu-ZT:feature/fix/327

Conversation

@Gu-ZT
Copy link
Copy Markdown

@Gu-ZT Gu-ZT commented Mar 19, 2026

Copilot AI review requested due to automatic review settings March 19, 2026 19:47
@neoforged-pr-publishing
Copy link
Copy Markdown

  • Publish PR to GitHub Packages

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a repository content filter for the NeoForged Releases Maven repo to prevent it from behaving like a partial Maven Central mirror and blocking fallback resolution of missing artifacts (e.g., *-interfaceinjection.json), addressing issue #327.

Changes:

  • Apply a content allowlist (NeoForgedRepositoryFilter) to the https://maven.neoforged.net/releases/ repository.
  • Introduce a generated allowlist class enumerating the allowed modules for the NeoForged Releases repository.

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated no comments.

File Description
src/main/java/net/neoforged/moddevgradle/internal/RepositoriesPlugin.java Applies the new NeoForged repository content filter to the NeoForged Releases repo.
src/generated/java/net/neoforged/moddevgradle/internal/generated/NeoForgedRepositoryFilter.java New allowlist used to constrain which coordinates may resolve from the NeoForged Releases repo.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 2 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…th NeoForge Maven using the Maven Central repository
@Gu-ZT Gu-ZT force-pushed the feature/fix/327 branch from 40b2243 to 8796eb9 Compare May 19, 2026 16:34
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (2)

src/main/java/net/neoforged/moddevgradle/internal/ModDevPlugin.java:173

  • fetchModuleDependencies performs a blocking network call during configuration via moduleUrl.openStream() without any connection/read timeouts and without respecting Gradle offline mode. This can hang configuration indefinitely on network/DNS issues and will make unit tests (e.g., ModDevPluginTest using ProjectBuilder) flaky/slow when network access is restricted. Use a URLConnection/HttpURLConnection with explicit connect/read timeouts, and skip these HTTP fetches when gradle.startParameter.offline is true (falling back to the stable module list).
    private static void fetchModuleDependencies(String path, Pattern depPattern) {
        try {
            var moduleUrl = URI.create(
                    "https://maven.neoforged.net/releases/" + path).toURL();

            try (var stream = moduleUrl.openStream()) {
                var content = new String(stream.readAllBytes(), StandardCharsets.UTF_8);
                var matcher = depPattern.matcher(content);
                while (matcher.find()) {
                    var group = matcher.group(1);
                    var module = matcher.group(2);
                    if ("*".equals(group) || "*".equals(module)) {
                        continue;
                    }
                    NeoForgedRepositoryFilter.addGameLibrary(group, module);
                }
            }
        } catch (Exception e) {

src/main/java/net/neoforged/moddevgradle/internal/ModDevPlugin.java:153

  • When repositories are applied at the settings level, ModDevPlugin.apply() intentionally does not apply RepositoriesPlugin to the project. In that case populateNeoForgeRepositoryFilter still runs and attempts RepositoriesPlugin.applyContentFilter(project), which will throw because the __internal_neoForgeRepository extension is not present (and will log a warning each time). Gate this logic on the repository actually being present (e.g., findByName on the extension, or checking whether the project has RepositoriesPlugin applied) and skip both the HTTP discovery and applyContentFilter when using settings-level repositories.
        // Apply the content filter now — before any dependency resolution uses the
        // NeoForge repository.
        try {
            RepositoriesPlugin.applyContentFilter(project);
        } catch (Exception e) {
            LOG.warn("Failed to apply NeoForge repository content filter: {}", e.getMessage());
        }

Comment thread src/main/java/net/neoforged/moddevgradle/internal/ModDevPlugin.java Outdated
Comment thread src/main/java/net/neoforged/moddevgradle/internal/NeoForgedRepositoryFilter.java Outdated
Comment thread src/main/java/net/neoforged/moddevgradle/internal/ModDevPlugin.java
…ears stale state and applies content filter consistently
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (3)

src/main/java/net/neoforged/moddevgradle/internal/ModDevPlugin.java:165

  • fetchModuleDependencies performs an unconditional HTTP request (URL#openStream) during ModDevPlugin.enable() (configuration phase). This can break the Gradle configuration cache (the project advertises config-cache support) and can hang builds because no connect/read timeouts are set. Consider moving this to a configuration-cache compatible mechanism (e.g., a ValueSource/BuildService with declared inputs + caching) and using an HTTP client with explicit timeouts / offline handling, or avoid network I/O during configuration entirely.
    private static void fetchModuleDependencies(String path, Pattern depPattern) {
        try {
            var moduleUrl = URI.create(
                    "https://maven.neoforged.net/releases/" + path).toURL();

            try (var stream = moduleUrl.openStream()) {
                var content = new String(stream.readAllBytes(), StandardCharsets.UTF_8);

src/main/java/net/neoforged/moddevgradle/internal/ModDevPlugin.java:180

  • The exception is swallowed with only e.getMessage(), which drops the stack trace and makes diagnosing failed metadata downloads difficult. Logging the exception object (or at least the root cause) and narrowing the catch to the expected exception types (e.g., IO-related) would make failures actionable without hiding unexpected runtime errors.
        } catch (Exception e) {
            LOG.warn(
                    "Failed to discover dependencies from {}: {}",
                    path, e.getMessage());
        }

src/test/java/net/neoforged/moddevgradle/internal/ModDevPluginTest.java:184

  • extension.setVersion(...) now triggers populateNeoForgeRepositoryFilter, which performs network I/O to maven.neoforged.net. That makes this unit test potentially slow/flaky in CI environments without network access (even though failures are swallowed). Consider refactoring the metadata fetch behind an injectable service and substituting a stub in tests, or using local test fixtures instead of real HTTP requests.
            extension.setVersion("21.10.48-beta");

            var neoRepo = RepositoriesPlugin.getNeoForgeRepository(project);
            assertThat(neoRepo).isNotNull();

Comment thread src/main/java/net/neoforged/moddevgradle/internal/ModDevPlugin.java
Comment thread src/test/java/net/neoforged/moddevgradle/internal/ModDevPluginTest.java Outdated
Comment thread src/main/java/net/neoforged/moddevgradle/internal/NeoForgedRepositoryFilter.java Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (1)

src/test/java/net/neoforged/moddevgradle/internal/ModDevPluginTest.java:236

  • This test is named as if it verifies that the NeoForged repository content filter is applied, but it only asserts the repository exists. Consider either renaming the test to match the assertion, or adding an assertion that the content filter actually blocks an unrelated module and allows a known included module.
        @Test
        void testContentFilterAppliedInVanillaOnlyMode() {
            extension.setNeoFormVersion("1.21.4-20240101.235959");

            var neoRepo = RepositoriesPlugin.getNeoForgeRepository(project);
            assertThat(neoRepo).isNotNull();
            assertThat(project.getRepositories().stream()
                    .filter(r -> "NeoForged Releases".equals(r.getName()))
                    .findFirst()).isPresent();
        }

Comment thread src/main/java/net/neoforged/moddevgradle/internal/ModDevPlugin.java Outdated
Comment thread src/main/java/net/neoforged/moddevgradle/internal/ModDevPlugin.java
Comment thread src/test/java/net/neoforged/moddevgradle/internal/ModDevPluginTest.java Outdated
@Matyrobbrt
Copy link
Copy Markdown
Member

Matyrobbrt commented May 19, 2026

Thank you for your contribution! Unfortunately, this is really not an approach we wish to consider, as it requires maintaining a list of artifacts hardcoded within the plugin and/or http-requests at configuration time. Eventually our maven will stop proxying central (as there are other issues), but in the meantime, you'll have to declare an exclusive repository for your artifact (see https://docs.gradle.org/current/userguide/filtering_repository_content.html#sec:declaring-content-repositories)

@Matyrobbrt Matyrobbrt closed this May 19, 2026
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

Successfully merging this pull request may close these issues.

Neoforged Maven Central Mirror Missing interfaceinjection.json Artifacts

3 participants