Skip to content

Conversation

@Toffikk
Copy link
Contributor

@Toffikk Toffikk commented Sep 30, 2025

fixes #355
since shadow 9.x, the INCLUDE duplication strategy has to be specified in order to properly merge service files

The solution was taken from this shadow issue

duplicatesStrategy = DuplicatesStrategy.INCLUDE
must be set for
mergeServicesFiles()
to work properly. Once this is set, it worked properly.

This pull request replaces #357

@Toffikk Toffikk changed the title fix(deps): adapt shadow configuration for shadow 9.x and bump shadow to 9.2.2 fix(deps): adapt shadow configuration for shadow 9.x and update it to v9.2.2 Sep 30, 2025
@jpenilla
Copy link
Member

This seems like an incorrect or at least an incomplete "fix"

@Toffikk
Copy link
Contributor Author

Toffikk commented Sep 30, 2025

I would assume it is correct since the shadow dev also said that duplicatesStrategy takes precedence over the mergeServiceFiles() now and from what i recall it was changed to EXCLUDE by default
GradleUp/shadow#1348
but if it is indeed incomplete i can try to dig up a little more

@Toffikk
Copy link
Contributor Author

Toffikk commented Sep 30, 2025

Improve the error message for empty mainClassName. ([#1601](https://github.com/GradleUp/shadow/pull/1601))
Default duplicatesStrategy back to EXCLUDE. ([#1617](https://github.com/GradleUp/shadow/pull/1617))
This strategy is consistent with 8.x series behavior, which is more compatible for most users upgrading.
For most ResourceTransformer users, you need to override the strategy to INCLUDE to make them work.
Strongly suggest declaring the duplicatesStrategy explicitly in your ShadowJar configuration to avoid confusion.
See more details about the strategies at [Handling Duplicates Strategy](https://gradleup.com/shadow/configuration/merging/#handling-duplicates-strategy).

shadow 9.0.1 changelog

@jpenilla
Copy link
Member

duplicate non service files should be an error and fail the build. it doesn't seem like a very well thought out change if you can no longer have this behavior while enabling service file merging

@Toffikk
Copy link
Contributor Author

Toffikk commented Sep 30, 2025

  filesNotMatching("META-INF/services/**") {
    duplicatesStrategy = DuplicatesStrategy.EXCLUDE // Or FAIL.
  }

there is a solution like this, should i update my pr with that?
it makes it so the merging works but other files fail when they are duplicate

used a different solution

@Toffikk
Copy link
Contributor Author

Toffikk commented Sep 30, 2025

    filesMatching("META-INF/services/**") {
        duplicatesStrategy = DuplicatesStrategy.INCLUDE
    }

updated to make it only include the services

@jpenilla
Copy link
Member

jpenilla commented Oct 6, 2025

I still think this could cause problems if there's files the merger doesn't touch in that dir for whatever reason, but I guess it's probably good enough

Still seems kind of like a step backwards to require this extra config for merging to work

Copy link
Member

@jpenilla jpenilla left a comment

Choose a reason for hiding this comment

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

Good enough for now, thanks

@jpenilla jpenilla merged commit f439166 into PaperMC:main Oct 10, 2025
1 check passed
@Toffikk Toffikk deleted the fix/shadow-9.x branch October 11, 2025 12:16
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.

reobf mappings fail to generate when paperweight build with shadow 9.x

2 participants