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.

Sign up for GitHub

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

Jump to bottom

convert engine build to kotlin #5196

Merged
merged 3 commits into from
May 14, 2024
Merged

convert engine build to kotlin #5196

merged 3 commits into from
May 14, 2024

Conversation

soloturn
Copy link
Contributor

migrate engine build to kotlin.

@soloturn soloturn changed the title Qa/kotlin convert engine build to kotlin Dec 23, 2023
@github-actions github-actions bot added the Category: Build/CI Requests, Issues and Changes targeting gradle, groovy, Jenkins, etc. label Dec 24, 2023
@soloturn soloturn force-pushed the qa/kotlin branch 9 times, most recently from ac8c575 to 7f94796 Compare December 28, 2023 09:58
@soloturn soloturn marked this pull request as ready for review December 28, 2023 09:59
@soloturn soloturn force-pushed the qa/kotlin branch 2 times, most recently from f2dbe74 to d6bd80a Compare January 1, 2024 00:11
@soloturn soloturn requested a review from BenjaminAmos January 3, 2024 09:48
BenjaminAmos
BenjaminAmos reviewed Jan 3, 2024
View reviewed changes
build-logic/src/main/kotlin/terasology-module.gradle.kts Outdated
@@ -145,7 +148,7 @@ tasks.named("processResources") {
}

tasks.named("compileJava") {
dependsOn("processResources")
dependsOn("processResources", ":engine:compileJmhJava")
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't benchmarks be compiled separately? Could you explain why modules need to depend on the engine's private benchmarks?

Copy link
Contributor Author

@soloturn soloturn Jan 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

the error message without was:

> Task :engine:jar
> Task :modules:CoreAssets:compileJava NO-SOURCE
> Task :modules:BiomesAPI:compileJava FAILED

FAILURE: Build failed with an exception.

* What went wrong:
A problem was found with the configuration of task ':modules:BiomesAPI:compileJava' (type 'JavaCompile').
  - Gradle detected a problem with the following location: '/srv/http/repos/terasology/engine/build/classes'.
    
    Reason: Task ':modules:BiomesAPI:compileJava' uses this output of task ':engine:compileJmhJava' 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 ':engine:compileJmhJava' as an input of ':modules:BiomesAPI:compileJava'.
      2. Declare an explicit dependency on ':engine:compileJmhJava' from ':modules:BiomesAPI:compileJava' using Task#dependsOn.
      3. Declare an explicit dependency on ':engine:compileJmhJava' from ':modules:BiomesAPI:compileJava' using Task#mustRunAfter.
    
    For more information, please refer to https://docs.gradle.org/8.5/userguide/validation_problems.html#implicit_dependency in the Gradle documentation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for the feedback on discord. to make the engine jar not contain the jmh files i added

    exclude("src/jmh")

which then allowed to compile it without all these undesired dependency spread out.

@soloturn soloturn requested a review from BenjaminAmos January 4, 2024 07:42
@BenjaminAmos BenjaminAmos mentioned this pull request Jan 8, 2024
Merged
@soloturn soloturn force-pushed the qa/kotlin branch 2 times, most recently from b63a9ec to 3fce95a Compare January 21, 2024 17:22
@soloturn soloturn force-pushed the qa/kotlin branch 4 times, most recently from 8b3240c to 9bc6bec Compare January 27, 2024 18:37
BenjaminAmos
BenjaminAmos previously approved these changes Jan 27, 2024
View reviewed changes
@BenjaminAmos
Copy link
Contributor

Unfortunately, this fails to run the tests succesfully at the moment:

* What went wrong:
A problem was found with the configuration of task ':engine-tests:compileJava' (type 'JavaCompile').
  - Gradle detected a problem with the following location: '/home/jenkins/agent/workspace/Terasology_engine_PR-5196/engine/build/classes'.
    
    Reason: Task ':engine-tests:compileJava' uses this output of task ':engine:compileJmhJava' 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 ':engine:compileJmhJava' as an input of ':engine-tests:compileJava'.
      2. Declare an explicit dependency on ':engine:compileJmhJava' from ':engine-tests:compileJava' using Task#dependsOn.
      3. Declare an explicit dependency on ':engine:compileJmhJava' from ':engine-tests:compileJava' using Task#mustRunAfter.
    
    For more information, please refer to https://docs.gradle.org/8.5/userguide/validation_problems.html#implicit_dependency in the Gradle documentation.

@BenjaminAmos
Copy link
Contributor

This still does not compile, unfortunately:

FAILURE: Build failed with an exception.

* Where:
Build file '/home/jenkins/agent/workspace/Terasology_engine_PR-5196/engine/build.gradle.kts' line: 240

* What went wrong:
Script compilation errors:

  Line 240:     source = sourceSets.jmh.get().java
                                    ^ Unresolved reference: jmh

  Line 241:     classpath = sourceSets.jmh.get().compileClasspath
                                       ^ Unresolved reference: jmh

  Line 242:     destinationDirectory.set(sourceSets.jmh.get().java.destinationDirectory)
                                                    ^ Unresolved reference: jmh

  Line 248:     classpath = sourceSets.jmh.get().compileClasspath + sourceSets.jmh.get().runtimeClasspath
                                       ^ Unresolved reference: jmh

  Line 248:     classpath = sourceSets.jmh.get().compileClasspath + sourceSets.jmh.get().runtimeClasspath
                                                                               ^ Unresolved reference: jmh

  Line 252:     jmhAnnotationProcessor("org.openjdk.jmh:jmh-generator-annprocess:1.27")
                ^ Unresolved reference: jmhAnnotationProcessor

  Line 253:     jmhImplementation("org.openjdk.jmh:jmh-core:1.27")
                ^ Unresolved reference: jmhImplementation

  Line 254:     jmhImplementation("org.openjdk.jmh:jmh-generator-annprocess:1.27")
                ^ Unresolved reference: jmhImplementation

@jdrueckert
Copy link
Member

@soloturn are you planning to take this up again?

@jdrueckert jdrueckert added the Status: Needs Author Input Requires more information by the author on the reported issue or provided changes label Apr 21, 2024
soloturn added 2 commits May 13, 2024 04:47
@soloturn
rename to build.gradle.kts
1966bb5 
* timestamp cannot be ignored easily with ignoreproperties, so leave
it as is, and remove the todo.

* move down writing the jar file, to assure version is specified in
 typehandlerlibrary e.g. which has it like project(:engine).version

* add jmh gradle plugin
@soloturn
intermediate jmh
b6ed93b
@soloturn
Copy link
Contributor Author

soloturn commented May 13, 2024 •
edited
Loading

running out of ideas how to fix this i asked in the gradle forums:
https://discuss.gradle.org/t/exclude-jmh-files-in-resulting-jar-jmh-plugin-bug-or-our-bug/48450

tried different notation now, removing the duplicate task. if you could test it again please with gradle jmh.

@soloturn soloturn removed the Status: Needs Author Input Requires more information by the author on the reported issue or provided changes label May 13, 2024
@soloturn soloturn merged commit 13c07b8 into develop May 14, 2024
10 checks passed
@soloturn soloturn deleted the qa/kotlin branch May 14, 2024 03:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BenjaminAmos BenjaminAmos BenjaminAmos approved these changes

Assignees
No one assigned
Labels
Category: Build/CI Requests, Issues and Changes targeting gradle, groovy, Jenkins, etc.
Projects
Terasology Inbox
Status: No status
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@soloturn @BenjaminAmos @jdrueckert