More Gradle: now with makeshift release management! Ready for v3.0.0 #3871

Cervator · 2020-03-30T06:57:05Z

This ate my entire weekend, although just the spare time between GSOC proposal review, baby maintenance, and so on. Up way too late so will be brief ™️ - had imagined I might merge this straight to develop then PR to master for the v3.0.0 release, but too late and now too big.

Realized I needed to be able to vary how artifacts get published, either as releases or snapshots to Artifactory. This used to be done by having two separate jobs in the legacy Jenkins, one tied to master that made releases, one tied to develop that made snapshots. The actual version numbers were modified by hand as part of doing a release (take off -SNAPSHOT, push to master, bump version and reattach -SNAPSHOT ...)

New Jenkins has a multi-branch pipeline job. Shiny. But it was publishing everything as snapshots, even pull requests getting built, meaning at any point in time a module build could run against some engine artifact that came out of an unmerged PR. Not great.

Okay not too bad. Quick conditional to vary publishing by branch just for now. Oh, and really then I might as well just leave the version numbers without -SNAPSHOT and attach that dynamically if we're in develop ! Awesome bonus. Wait, but then what about modules. They use the same shared publish.gradle to keep things standard. That means they'll publish releases when building from master branches 🤔

Alright, too much work to make develop branches for everything, default them, have devs do both branches as a standard... besides we might be able to do better like with Git tags (ohai @skaldarnar) instead of branches, and hey now checks are in place in code to where we could probably just look at tags instead of branches. But this needs to work right now.

Awkward release management to the rescue! We'll default modules to not have release management enabled, meaning even while building master branches unless "isReleaseManaged" : true is added to module.txt the new stuff won't trigger and we'll still just make snapshots. And the embedded modules with the engine will get that set, but nobody else yet. It isn't super pretty, but ... it works.

For any reviewers: The actual change in build.gradle for modules is minimal, I just had to move some things around. This has been tested with the Nanoware job line in Jenkins and repo line in Artifactory (which also took some extra effort to get right)

Bonus: The code will now gently (or angrily, if red text is angry) warn about modules that still use -SNAPSHOT in their own version in module.txt, so we can eventually clean that out (or just move the version out of code into tags). Thanks @sin3point14 for that!

Outstanding

Goes with Minor update to make artifact publishing conditional ModuleJteConfig#3
Javadoc fails on modules that have none, leading to failed build (huh, did this not trigger earlier?)
Analytics run after publishing - that's a sinful allowance since there are no quality gates in recording the analytics yet so no way to fail the build anyway and this way it gets to the interesting parts sooner
Think about next steps sooner or later, this isn't perfect but gets us closer ...
Modules in theory can now run release builds, if enabled. However these will still use the "build harness" from the develop build of the engine. More work to do and scenarios to satisfy - but we'll get there. Shout out to @PS-Soundwave

GooeyHub · 2020-03-30T07:09:58Z

Hooray Jenkins reported success with all tests good!

skaldarnar

More or less looking fine. Some minor changes requested, and long-term we should try to get away from too much logic in the buil file but rather derive as much information from other things present (e.g., version from git tags, where to release to from version, ...)

skaldarnar · 2020-03-31T20:00:39Z

config/gradle/publish.gradle

@@ -17,16 +17,45 @@ publishing {
                maven {
                    name = 'TerasologyOrg'

-                    def repoViaEnv = System.getenv()["PUBLISH_REPO"]
                    if (rootProject.hasProperty("publishRepo")) {


I'm wondering whether we should start to scope the properties in the gradle builds, similar to what most plugins to. This would become terasology.publishRepo then...

Had not thought about that before. Even in a plain build file, rather than a plugin? I've been curious if some of our logic might go better in a plugin, where it would make all kinds of sense, since then you'd probably use neato little blocks to configure it.

skaldarnar · 2020-03-31T20:12:52Z

config/gradle/publish.gradle

+                        if (gitBranch != null && gitBranch != "" && gitBranch.equals("master")) {
+                            // Okay we're in an environment where a branch name is set to 'master' so we might be doing a release
+                            // This is the funny part. Modules aren't ready globally to accept master branch == release, so check a prop that defaults to bypass
+                            if (project.hasProperty("bypassModuleReleaseManagement")) {
+                                if (bypassModuleReleaseManagement == "true") {
+                                    println "Release management not enabled for " + project.name + ", using snapshot repo despite building 'master' branch"
+                                    deducedPublishRepo += snapshotRepoFragment
+                                } else {
+                                    println "Release management *is* enabled for " + project.name + ", using release repo since building 'master' branch"
+                                    deducedPublishRepo += releaseRepoFragment
+                                }
+                            } else {
+                                println "We're working on a 'master' branch with defaults so using the release publish repo"
+                                deducedPublishRepo += releaseRepoFragment
+                            }
+                        } else {
+                            // No master branch so we for sure are just doing snapshots
+                            println "We're not working with a branch name set that's 'master' so assuming we're publishing snapshots"
+                            deducedPublishRepo += "-snapshot-local"
+                        }


The whole logic in here boils down to just this, doesn't it?

if (isMaster(maybeGitBranch) && !shouldBypassModuleRelease()) { deducedPublishRepo += "-release-local" } else { deducedPublishRepo += "-snapshot-local" }

I think with the two additional functions this code is really readable, and the nesting is reduced.

def isMaster(gitBranch) { return gitBranch != null && gitBranch != "" && gitBranch.equals("master"); } def shouldBypassModuleRelease() { return project.hasProperty("bypassModuleReleaseManagement") && bypassModuleReleaseManagement == "true"; }

This would loose a couple of console lines, but overall the structure in the build file is more comprehensible imho.

Thank you! I had even more logic madness in there while trying to find a solution that would cover all the scenarios, this was actually an already boiled down version, but closing in on 2 am was not letting me see clearly in shrinking it further. This is way cleaner! Don't need the logging now that it tests out fine :-)

skaldarnar · 2020-03-31T20:13:32Z

config/gradle/publish.gradle

+                        } else {
+                            // No master branch so we for sure are just doing snapshots
+                            println "We're not working with a branch name set that's 'master' so assuming we're publishing snapshots"
+                            deducedPublishRepo += "-snapshot-local"


Suggested change

deducedPublishRepo += "-snapshot-local"

deducedPublishRepo += snapshotRepoFragment

skaldarnar · 2020-03-31T20:18:28Z

modules/BiomesAPI/build.gradle

+    // Temporary workaround: default modules to bypass release management: master branch builds will still make snapshot builds for the snapshot repo
+    // If a module actually wants release management simply include `"isReleaseManaged" : true` in module.txt - this is needed for the engine repo embedded modules
+    // One option would be to slowly convert modulespace to default to a `develop` + `master` setup living in harmony with associated automation/github tweaks
+    // Alternatively one more round of refactoring could switch to Git tags, a single `master` branch, and possible other things to help match snaps/PR builds somehow?


Heavyly in favor of git tags, as most modules don't have complicated branching going on anyways. But you're right, should not be done in this PR, and needs some proper thoughtwork to figure out details and edge cases...

skaldarnar · 2020-03-31T20:19:50Z

modules/BiomesAPI/build.gradle

+    // This won't work without additionally doing constant version bumps (perhaps via Git tags) - but too much work to switch around all modules at once
+    // Complicating things more the use of publish.gradle to centralize logic means modules and engine bits are treated the same, yet we need to vary modules
+    // Temporary workaround: default modules to bypass release management: master branch builds will still make snapshot builds for the snapshot repo
+    // If a module actually wants release management simply include `"isReleaseManaged" : true` in module.txt - this is needed for the engine repo embedded modules


I don't like the fact that the generic module.txt now holds information specific to our build system. As it is not officially documented anywhere, I guess its not much of a deal. Nevertheless, I would like to get rid of it asap.

Yep, it is a workaround for sure. Lets me get through this big release without an even bigger overhaul. It can be an undocced feature likely never to be used in module land as we can hopefully finish improving things soon.

skaldarnar · 2020-03-31T20:26:37Z

modules/CoreSampleGameplay/build.gradle

+if (moduleConfig.isReleaseManaged && env.BRANCH_NAME == "master") {
+    // This is mildly awkward since we need to bypass by default, yet if release management is on (true) then we set the bypass to false ..
+    ext.bypassModuleReleaseManagement = false
+    if (version.contains("-SNAPSHOT")) {


Suggested change

if (version.contains("-SNAPSHOT")) {

if (version.endsWith("-SNAPSHOT")) {

skaldarnar · 2020-03-31T20:26:46Z

modules/CoreSampleGameplay/build.gradle

+        out.withStyle(Style.FailureHeader).println("WARNING: Module " + project.name + " is explicitly versioned as a snapshot in module.txt, please remove '-SNAPSHOT'")
+    }
+} else {
+    if (!version.contains("-SNAPSHOT")) {


Suggested change

if (!version.contains("-SNAPSHOT")) {

if (!version.endsWith("-SNAPSHOT")) {

skaldarnar · 2020-03-31T20:26:56Z

templates/build.gradle

+import static org.gradle.internal.logging.text.StyledTextOutput.Style
+
+// The only case in which we make module non-snapshots is if release management is enabled and BRANCH_NAME is "master"
+if (moduleConfig.isReleaseManaged && env.BRANCH_NAME == "master") {


skaldarnar · 2020-03-31T20:27:05Z

templates/build.gradle

+if (moduleConfig.isReleaseManaged && env.BRANCH_NAME == "master") {
+    // This is mildly awkward since we need to bypass by default, yet if release management is on (true) then we set the bypass to false ..
+    ext.bypassModuleReleaseManagement = false
+    if (version.contains("-SNAPSHOT")) {


Suggested change

if (version.contains("-SNAPSHOT")) {

if (version.endsWith("-SNAPSHOT")) {

skaldarnar · 2020-03-31T20:27:14Z

templates/build.gradle

+        out.withStyle(Style.FailureHeader).println("WARNING: Module " + project.name + " is explicitly versioned as a snapshot in module.txt, please remove '-SNAPSHOT'")
+    }
+} else {
+    if (!version.contains("-SNAPSHOT")) {


Suggested change

if (!version.contains("-SNAPSHOT")) {

if (!version.endsWith("-SNAPSHOT")) {

GooeyHub · 2020-04-01T03:58:21Z

Hooray Jenkins reported success with all tests good!

Cervator · 2020-04-01T04:49:52Z

Thanks a lot for the fresh eyes on this! Helped a lot, cleaner now, tests out still, hopefully looks better now. Gotta fix the Javadoc thing soon then will merge and do the release 👍

skaldarnar · 2020-04-01T08:02:03Z

facades/subprojects.gradle

-    def subprojectPath = ':' + subprojectName
-    def subproject = project(subprojectPath)
-    subproject.projectDir = possibleSubprojectDir
+    if (!possibleSubprojectDir.name.startsWith(".")) {


What about a general approach like this (in the future): https://medium.com/@ungesehn/use-gitignore-for-gradle-task-excludes-e5d011e99f71

.settings should be gitignores regardless of where in the file tree...

… branch name Change env var Console logging issue Fixy

…ng publish)

Avoids having to manage -SNAPSHOT in code, instead it is deduced from the environment if a branch name is set (Jenkins) If branch is master (and for modules if release management is opted into) then artifacts will be published to a release repo, otherwise to a snapshot repo Retains support for varying the target publish org in Artifactory so "terasology" vs "nanoware" for instance.

…detection (can happen in some IDEs)

GooeyHub · 2020-04-02T01:52:17Z

Hooray Jenkins reported success with all tests good!

Cervator · 2020-04-02T02:21:42Z

Fixed the Javadoc issue, validated with the Nanoware jobs etc, and merged! Initial engine develop job ran fine

Cervator added the Category: Build/CI Requests, Issues and Changes targeting gradle, groovy, Jenkins, etc. label Mar 30, 2020

Cervator added this to the v3.0.0 milestone Mar 30, 2020

skaldarnar requested changes Mar 31, 2020

View reviewed changes

skaldarnar reviewed Apr 1, 2020

View reviewed changes

skaldarnar approved these changes Apr 1, 2020

View reviewed changes

Cervator added 6 commits April 1, 2020 20:39

Support bypassing artifact publishing and varying the target based on…

18f895a

… branch name Change env var Console logging issue Fixy

Release v3.0.0

e714a48

Explicitly include javadoc (would otherwise not generate if not hitti…

39584a7

…ng publish)

Exclude any facade or meta module directories that start with . from …

d3e3cd0

…detection (can happen in some IDEs)

Cleanup from code review

935895b

Cervator force-pushed the develop branch from a5b1bf5 to 935895b Compare April 2, 2020 01:40

Cervator merged commit 935895b into MovingBlocks:develop Apr 2, 2020

Cervator mentioned this pull request Apr 13, 2020

Improve in-game display of module versions - release/snapshot, git hash #3888

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

More Gradle: now with makeshift release management! Ready for v3.0.0 #3871

More Gradle: now with makeshift release management! Ready for v3.0.0 #3871

Cervator commented Mar 30, 2020 •

edited

Loading

GooeyHub commented Mar 30, 2020

skaldarnar left a comment

skaldarnar Mar 31, 2020

Cervator Apr 1, 2020

skaldarnar Mar 31, 2020

skaldarnar Mar 31, 2020

Cervator Apr 1, 2020

skaldarnar Mar 31, 2020

skaldarnar Mar 31, 2020

skaldarnar Mar 31, 2020

Cervator Apr 1, 2020

skaldarnar Mar 31, 2020

skaldarnar Mar 31, 2020

skaldarnar Mar 31, 2020

skaldarnar Mar 31, 2020

skaldarnar Mar 31, 2020

GooeyHub commented Apr 1, 2020

Cervator commented Apr 1, 2020

skaldarnar Apr 1, 2020

GooeyHub commented Apr 2, 2020

Cervator commented Apr 2, 2020

	deducedPublishRepo += "-snapshot-local"
	deducedPublishRepo += snapshotRepoFragment

	if (version.contains("-SNAPSHOT")) {
	if (version.endsWith("-SNAPSHOT")) {

	if (!version.contains("-SNAPSHOT")) {
	if (!version.endsWith("-SNAPSHOT")) {

More Gradle: now with makeshift release management! Ready for v3.0.0 #3871

More Gradle: now with makeshift release management! Ready for v3.0.0 #3871

Conversation

Cervator commented Mar 30, 2020 • edited Loading

Outstanding

GooeyHub commented Mar 30, 2020

skaldarnar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GooeyHub commented Apr 1, 2020

Cervator commented Apr 1, 2020

Choose a reason for hiding this comment

GooeyHub commented Apr 2, 2020

Cervator commented Apr 2, 2020

Cervator commented Mar 30, 2020 •

edited

Loading