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

Gradle upgrading #3806

Closed
wants to merge 20 commits into from
Closed

Conversation

DarkWeird
Copy link
Contributor

@DarkWeird DarkWeird commented Dec 15, 2019

Contains

Upgrade to Gradle 6.0
Upgrade FindBugs (Abandoned) -> SpotBugs
Simplification module dependencies src-bin determination (gradle way)
Replace artifactory to gradle's maven-publish plugin (support all maven repos)
Replacement compile to implementation (faster compile, smaller modules classpath)

How to test

  1. Basic:
    • Run gradlew build
    • Must work (allTests passed, exists tests from MTE and Modules)
  2. Client:
    • Run gradlew game
    • Game must started and work normal (all needed modules loaded, assets available)
  3. Server:
    • Run gradlew server
    • Server must started and work normal (all needed modules loaded)
  4. TeraEd:
    • Run gradlew editor
    • Editor must started and work normal
  5. Distribution:
    • Run gradlew distPC
    • Grab distribution and launch it
    • Game must started and work normal (all needed modules loaded, assets available)
  6. Publish:
    • Run gradlew publishToMavenLocal
    • Check maven local repository for valid artifacts (.jar, sources.jar, javadoc.jar)
  7. Publish to Artifactory:
    • set environment variables ORG_GRADLE_PROJECT_mavenUser and ORG_GRADLE_PROJECT_mavenPass
    • Run gradlew publishAllPublicationsToMovingBlocksRepository (taskname has been generated, i called repository as MovingBlocks)
    • Check artifactory repository for valid artifacts (.jar, sources.jar, javadoc.jar)

Outstanding before merging (Move to Future PRs)

@GooeyHub
Copy link
Member

Uh oh, something went wrong with the build. Need to check on that

@GooeyHub
Copy link
Member

Uh oh, something went wrong with the build. Need to check on that

@GooeyHub
Copy link
Member

Uh oh, something went wrong with the build. Need to check on that

@GooeyHub
Copy link
Member

Uh oh, something went wrong with the build. Need to check on that

Fix tests after gradle upgrade
@GooeyHub
Copy link
Member

Hooray Jenkins reported success with all tests good!

@GooeyHub
Copy link
Member

Hooray Jenkins reported success with all tests good!

@GooeyHub
Copy link
Member

Hooray Jenkins reported success with all tests good!

@pollend
Copy link
Member

pollend commented Dec 22, 2019

These changes are pretty nice, I was able to verify that this works on my local setup.

@DarkWeird
Copy link
Contributor Author

DarkWeird commented Dec 22, 2019

@pollend thanks.
Is you have any suggestions?

@pollend
Copy link
Member

pollend commented Dec 22, 2019

can we verify if this works on the new CBC instance @Cervator

@Cervator
Copy link
Member

The build itself in the new CBC Jenkins finishes with success, but doesn't seem to produce a viable game zip tho, way too tiny:

image

Another bigger piece is then applying that new style of build to the module ecosystem, which is a larger task, and that's going to be really hard for me to find time for until mid January or so :-(

@DarkWeird
Copy link
Contributor Author

The build itself in the new CBC Jenkins finishes with success, but doesn't seem to produce a viable game zip tho, way too tiny:

image

Another bigger piece is then applying that new style of build to the module ecosystem, which is a larger task, and that's going to be really hard for me to find time for until mid January or so :-(

  1. Yeah, I found problem with size - missing dep libs.
  2. New style of build system. What is it?
    @Cervator

Fix gather libraries for distribution
Fix. move -Xmx options to jvmArgs
@GooeyHub
Copy link
Member

Hooray Jenkins reported success with all tests good!

@Cervator
Copy link
Member

Trying to test this a bit in my mega-workspace, but hitting some issues :-)

Before detailing that though: nice work! I made it through reading all the changes and am impressed, especially with all the cleanup. I don't have the brainspace or time to understand how half of it works right now, but I'm still happy with it 😁

  • I have the server facade embedded in my workspace, which complained about some deprecated usages of the leftshift convenience function - maybe that's where I remember it from. Can fetch with groovyw facade get server I think - I don't have the CrashReporter embedded right now but it may need to be checked as well
  • I have 171 modules in my workspace 😅 Only the few embedded ones like Core get the updated build.gradle. Even after I tried to manually replace the one in AdditionalFruits it still errored out
    • It used to be we could use gradlew refresh to force copy fresh module build files to every local non-embedded module but .... when there are syntax/dependencies errors in the old build files we're trying to replace Gradle never gets that far
    • It may be wise for us to introduce groovyw module refresh that would take over from that old Gradle task (so it can just overwrite from the template without having to first load Gradle's project structure). That'd be a great extra to add to this PR 👍
    • On a related note I noticed the old task in Gradle to update modules get tweaked, but since groovyw module update-all exists we can just delete that one instead!
  • When we do get all the module files updated in a workspace they should work as well as Core - but this is where "applying that new style of build to the module ecosystem" comes in: in Jenkins the modules build independently. That's why there's a bunch of logic in there to test whether the module lives in an engine workspace or is itself a top level project - the Gradle needs to work in both cases :-) Same for the extra facades and libs we can add. So there are entire separate apps to consider beyond just standalone modules, fun!

@Cervator Cervator added Size: L Very big effort likely requiring a lot of research and work in many areas across the codebase Category: Build/CI Requests, Issues and Changes targeting gradle, groovy, Jenkins, etc. Category: Security Requests, Issues and Changes targeting security labels Dec 29, 2019
@DarkWeird
Copy link
Contributor Author

@Cervator
I replace dependency detect from "if" in module to workspace detect and rewrite (gradle way, i think) .
I test this in idea resolves(last idea work with gradle nice, i don't use any idea tasks)
Theoretically, you can use and separate module root and workspace already, if copy build.gradle from templates.

…n remapper (FacadeServer cannot building in workspace)
@DarkWeird
Copy link
Contributor Author

@Cervator
I'm fixed FacadeServer :)

MovingBlocks/FacadeServer#30

@GooeyHub
Copy link
Member

GooeyHub commented Jan 4, 2020

Hooray Jenkins reported success with all tests good!

Move workspace features from module to workspace
Unify plugin style
@GooeyHub
Copy link
Member

GooeyHub commented Jan 9, 2020

Hooray Jenkins reported success with all tests good!

@GooeyHub
Copy link
Member

Hooray Jenkins reported success with all tests good!

@Cervator
Copy link
Member

Cervator commented Feb 2, 2020

Been testing and reviewing some more 👍

Working in https://github.com/Nanoware/Terasology where I can use a separate org, repo in Artifactory, different Jenkins jobs, etc. So I've got some config tweak in a local gradle.properties to redirect things

  • Fixed minor issue where args were ignored in the game task
  • Made some minor naming tweaks (like "MovingBlocks" to "TerasologyOrg" and maven.gradle to publish.gradle)
  • Noticed rarely that Gradle would stall during gradlew idea or related tasks: suspecting org.gradle.parallel=true which I had set to false in the past over similar quirks. Seeing how that goes while testing further
  • Tested against Artifactory with the generic Maven publish - so far so good!
  • Bumped to 3.0.0-SNAPSHOT since this is pretty major (that was the goal for the next release anyway because of the amount of stuff we've broken)
  • Found and documented Dists do not fetch transitive modules Nanoware/Terasology#88 which precedes this PR but I hadn't noticed yet
  • gradlew editor breaks when I run it, but that doesn't surprise me - TeraEd isn't really a high priority
  • While publishing works the lack of logging weirds me out a bit. But I could confirm in Artifactory that v3 artifacts went to the Nanoware repo. Couldn't find where it went locally with publishToMavenLocal though, but then Windows sucks at file searching (really? Even with "3.0.0" you insist on finding anything with a 3 or a 0 in it, Win10? Really? Blah)
  • Newer Gradle by default also published sha files to the target repository, bit of a surprise that bit a lot of people, it seems. Causes some ugly errors in the log and for others have left to indirect issues later so I turned it off with systemProp.org.gradle.internal.publish.checksums.insecure=true in gradle.properties

More to come ... :)

@GooeyHub
Copy link
Member

GooeyHub commented Feb 2, 2020

Hooray Jenkins reported success with all tests good!

@DarkWeird
Copy link
Contributor Author

@Cervator

Noticed rarely that Gradle would stall during gradlew idea or related tasks: suspecting org.gradle.parallel=true which I had set to false in the past over similar quirks. Seeing how that goes while testing further
I don't use idea task.... it's non needed, imho.
.ipr, .iws - legacy file formats for idea.

last IDEA can handle gradle properly.
since 2017, seems.
excludes can sync with .gitignore
Jdk version setup from gradle
git activates automatically

Seems idea task not working properly :) (i haven't new run configs)

gradlew editor breaks when I run it, but that doesn't surprise me - TeraEd isn't really a high priority
it's work for me.....

Bumped to 3.0.0-SNAPSHOT since this is pretty major (that was the goal for the next release anyway because of the amount of stuff we've broken)
Oh, NOOOOOO. again snapshot :( (i talked about problem with snapshot deps on modules, can be module and there)

IDK, what will do futher....

@GooeyHub
Copy link
Member

GooeyHub commented Feb 2, 2020

Hooray Jenkins reported success with all tests good!

@Cervator
Copy link
Member

Cervator commented Feb 2, 2020

So gradlew idea really was just one example of a thing that seems able to cause trouble in parallel, it could be other stuff too. There's probably a way to isolate what can't be multithreaded better and handle that, then let stuff like code analytics still run in parallel. I agree that the idea task needs to be tossed, that's just a slightly different battle :-)

For TeraEd I do get the window, but exceptions in the log, is your log clear?

Don't mind the snapshot stuff, we'll sort it out - another battle and I'm making some progress :-)

@DarkWeird
Copy link
Contributor Author

In teraed - Opened javafx(seems) window. Logs clear, window haven't content.
Then i close editor.

@Cervator
Copy link
Member

Cervator commented Mar 3, 2020

Closing as merged by #3835!

@Cervator Cervator closed this Mar 3, 2020
@Cervator Cervator added this to the v3.0.0 milestone Mar 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Build/CI Requests, Issues and Changes targeting gradle, groovy, Jenkins, etc. Category: Security Requests, Issues and Changes targeting security Size: L Very big effort likely requiring a lot of research and work in many areas across the codebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants