Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[1.20.4] Sourcesets hack breaks Groovy mod builds #9844

Open
PaintNinja opened this issue Jan 8, 2024 · 4 comments
Open

[1.20.4] Sourcesets hack breaks Groovy mod builds #9844

PaintNinja opened this issue Jan 8, 2024 · 4 comments
Assignees
Labels
1.20 Regression This request demonstrates something missing in a newer version that was present in a prior version.

Comments

@PaintNinja
Copy link
Contributor

PaintNinja commented Jan 8, 2024

Minecraft Version: 1.20.4

Forge Version: 49.0.14

Logs:

20:22:13: Executing 'build'...


> Configure project :
Java: 17.0.8, JVM: 17.0.8+9-LTS-jvmci-23.0-b14 (Oracle Corporation), Arch: amd64

> Task :compileJava
> Task :compileGroovy
> Task :processResources FAILED
3 actionable tasks: 3 executed

FAILURE: Build failed with an exception.

* What went wrong:
A problem was found with the configuration of task ':processResources' (type 'ProcessResources').
  - Gradle detected a problem with the following location: 'C:\Users\oscar\Downloads\sourcesets-issue-repro-forge-1.20.4-49.0.14-mdk\build\sourcesSets\main'.
    
    Reason: Task ':compileGroovy' uses this output of task ':processResources' without declaring an explicit or implicit dependency. This can lead to incorrect results being produced, depending on what order the tasks are executed.
    
    Possible solutions:
      1. Declare task ':processResources' as an input of ':compileGroovy'.
      2. Declare an explicit dependency on ':processResources' from ':compileGroovy' using Task#dependsOn.
      3. Declare an explicit dependency on ':processResources' from ':compileGroovy' using Task#mustRunAfter.
    
    For more information, please refer to https://docs.gradle.org/8.4/userguide/validation_problems.html#implicit_dependency in the Gradle documentation.

* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Run with --scan to get full insights.
> Get more help at https://help.gradle.org.

BUILD FAILED in 6s
20:22:20: Execution finished 'build'.

Steps to Reproduce:

  1. Download and import this zip: sourcesets-issue-repro-forge-1.20.4-49.0.14-mdk.zip
  2. Run gradlew build

The changes from stock MDK are implementation localGroovy(), the id 'groovy' plugin and a Test.groovy source file to make the plugin do something.

Description of issue:
The current technique of combining sourcesets in build.gradle is incompatible with certain types of builds and JVM languages, such as multiplatform mod dev and language providers like Groovy. The former have solved the issue by partially reimplementing union launching only in the dev environment, which I think is a good approach from the perspective of minimising the chance of breaking gradle plugins, while the latter doesn't have any easy solution without more workarounds.

I consider this issue a blocker for 1.20.4 RB.

@PaintNinja PaintNinja added the Triage This request requires the active attention of the Triage Team. Requires labelling or reviews. label Jan 8, 2024
@autoforge autoforge bot added the 1.20 label Jan 8, 2024
@PaintNinja PaintNinja added Regression This request demonstrates something missing in a newer version that was present in a prior version. and removed Triage This request requires the active attention of the Triage Team. Requires labelling or reviews. labels Jan 8, 2024
@LexManos
Copy link
Member

LexManos commented Jan 8, 2024

To make this clear and public documentation.
The removal of the whole MOD_CLASSES and suggestion for merging the sourcesets comes from trying to follow JPMS specs. As the JPMS needs to have the entire module in a single path.
I'm not opposed to re-introducing the MOD_CLASSES system, but it should be done as a bespoke simple gradle plugin that people can apply in cases where they can't do anything else. Such as using groovy apparently.
The mdk changes were also done mainly for IntelliJ which insists on having the separate source and resources directories. Eclipse defaults to just spitting everything into the same folder.

The fundamental issue is that gradle doesn't like the output of two tasks being overlapped.
This is only an issue for IntellIj which uses gradle to run all of its backend stuff if it detects a gradle project.
Perhaps a solution would be to have the run tasks use the built jar file in the classpath instead of sourceset outputs. Is that possible? This is something i'll need those who use IntelliJ to look into.

@PaintNinja
Copy link
Contributor Author

Gradle and run configs are my weakness I'm afraid, so can't advise too strongly on an approach. I gave arch loom's solution as a possible idea as it looked like one that would avoid the headaches of Gradle altogether.

The issue can be reproduced from the CLI outside of IntelliJ, so I don't believe the fix should be opt-in for non-eclipse environments (such as CI running ./gradlew build).

@LexManos
Copy link
Member

LexManos commented Jan 9, 2024

Well, unless someone else volunteers this looks like its gunna stall then. Because for normal mod development the current system works fine. And personally all the other language loaders should be on the loader to support their edge cases, not us. Minecraft is written in java, the only language Forge officially supports is Java. And it, in all of my tests is fully functional for java.

As for CIs, the paths don't matter for a CI as they are just producing the result. They could just disable the merging on CI.
The only reason the paths need to be merged is for actually running the code. Again because the Java itself needs things in a single directory.

Thats why im curious to see any other JPMS gradle project in existence. See how they deal with this basic java limitation. My bet {based on trying to find examples} is that almost no project exists that properly uses the JPMS with gradle.

@PaintNinja
Copy link
Contributor Author

PaintNinja commented Jan 12, 2024

Any Gradle plugin that relies on the output of one sourceset to compile or process another is currently broken - Groovy was an example that affects me.

Needing the sourcesets hack in build.gradle seems to have been a common problem for people porting to 1.20.4 as the error when missing it doesn't make the culprit obvious and isn't a solution for things like multiloader or other special build systems. Turns out a lot didn't check the MDK.

The comment on the sourcesets bit says that it will eventually be migrated to FG. We can't really do that if it breaks some people's builds - especially if different logic is required for CI vs local dev. I've tried a couple of different approaches in Gradle and run configs but had no luck, they're my weakness. I don't really care how you end up solving it - restoring MOD_CLASSES only in userdev with UnionRelauncher or some other approach, but it needs to be done if you want it to be part of FG and to solve what's currently the most common source of confusion when porting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.20 Regression This request demonstrates something missing in a newer version that was present in a prior version.
Projects
None yet
Development

No branches or pull requests

2 participants