Skip to content

Fix DependencyUtils failing to infer artifact classifier under some circumstances#257

Merged
Technici4n merged 1 commit into
neoforged:mainfrom
CursedFlames:fix/artifact_classifier_inference
Jun 5, 2025
Merged

Fix DependencyUtils failing to infer artifact classifier under some circumstances#257
Technici4n merged 1 commit into
neoforged:mainfrom
CursedFlames:fix/artifact_classifier_inference

Conversation

@CursedFlames

Copy link
Copy Markdown
Contributor

Under some circumstances DependencyUtils can fail to infer the artifact classifier for processed dependencies, which causes the createMinecraftArtifacts task to fail during source recompilation due to Minecraft's dependencies not resolving correctly.

The specific case in which I encountered this is when using an Artifact Transformer; here I have a minimal reproduction of this case using the MDK and a dummy artifact transform plugin that applies a no-op to jar dependencies.

I tested that minimal reproduction against this fix and it succeeded to build; in addition the contents of nfrt_artifact_manifest.properties were identical to those produced with no artifact transform (after sorting the entries), as expected.

I don't know if this fix will work in every case, but I expect that it should at least work in the vast majority of cases.

@neoforged-pr-publishing

Copy link
Copy Markdown
  • Publish PR to GitHub Packages

@Technici4n Technici4n left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. This is also gets rid of the internal API usage, which is good!

@Technici4n Technici4n merged commit 21b2cc7 into neoforged:main Jun 5, 2025
7 checks passed
@neoforged-releases

Copy link
Copy Markdown

🚀 This PR has been released as ModDevGradle version 2.0.90.

@shartte

shartte commented Jun 5, 2025

Copy link
Copy Markdown
Collaborator

LGTM. This is also gets rid of the internal API usage, which is good!

Oh wow I didn't even notice that this gets rid of internal, that is indeed good

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.

3 participants