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

Migrate Makefile targets to Gradle tasks #105

Merged
merged 8 commits into from
Feb 10, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ cache:
- ${HOME}/.m2

script:
- make test
- ./gradlew preMerge

after_success:
- bash <(curl -s https://codecov.io/bash)
20 changes: 0 additions & 20 deletions Makefile

This file was deleted.

58 changes: 58 additions & 0 deletions build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,61 @@ subprojects {
jcenter()
}
}

val installVenv = tasks.register("installVenv", Exec::class.java) {
description = "Install a new virtualenv in ./venv"

outputs.dir("./venv")

commandLine("virtualenv", "venv")
}

val installPreCommit = tasks.register("installPreCommit", Exec::class.java) {
description = "Installs pre-commit in ./venv"

dependsOn(installVenv)

outputs.file("./venv/bin/pre-commit")

commandLine("./venv/bin/pip", "install", "pre-commit")
}

val installHooks = tasks.register("installHooks", Exec::class.java) {
description = "Run pre-commit hooks without installing them"
martinbonnin marked this conversation as resolved.
Show resolved Hide resolved

dependsOn(installPreCommit)

outputs.file(".git/hooks/pre-commit")
inputs.file(".pre-commit-config.yaml")

commandLine("./venv/bin/pre-commit", "install", "--install-hooks")
}

val runHooks = tasks.register("runHooks", Exec::class.java) {
description = "Run pre-commit hooks without installing them"

dependsOn(installPreCommit)

commandLine("./venv/bin/pre-commit", "run", "--all-files")
}

val preMerge = tasks.register("preMerge") {
description = "Runs pre-commit hooks, build the plugin and sample and execute tests."

dependsOn(runHooks)
dependsOn(gradle.includedBuild("gradle-plugin").task(":plugin:build"))
dependsOn(gradle.includedBuild("gradle-plugin").task(":plugin:check"))
dependsOn(subprojects.filter { it.name != "samples" }.map { it.tasks.named("assembleDebug") })
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could maybe remove the "assembleDebug" there. The "check" test below already executes testDebugUnitTest and testReleaseUnitTest so it will compile the sources for both debug and release mode.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agree, let's remove it 👍

dependsOn(subprojects.filter { it.name != "samples" }.map { it.tasks.named("check") })
Copy link
Collaborator

Choose a reason for hiding this comment

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

This won't work at the moment, as you have to specify also the mustRunAfter rules. In the current situation you're just saying that preMerge depends on runHooks, :plugin:build, :plugin:check, assembleDebug, check but you're not specifying the order.

This is also the reason why Travis is failing (he's running a one of a sample's check before the generateSwagger).

Copy link
Contributor Author

@martinbonnin martinbonnin Feb 8, 2020

Choose a reason for hiding this comment

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

This is also the reason why Travis is failing (he's running a one of a sample's check before the generateSwagger).

That was the point of this block int the root build.gradle.kts:

        tasks.all {
            if (name == "assembleDebug") {
                dependsOn(tasks.named("generateSwagger"))
            }
        }

Except I forgot that the generated files are needed before the compile task 🤦‍♂. So at the moment I think there's a race between compileKotlin and generateSwagger, both being dependencies of assembleDebug. I hopefully fixed that here: b0e45e7#diff-392475fdf2bc320d17762ed97109a121R66

}

subprojects {
afterEvaluate { // Remove when we have lazy configuration
tasks.all {
if (name == "assembleDebug") {
// assembleDebug needs the generated files
dependsOn(tasks.named("generateSwagger"))
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we can have this as part of the preMerge?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess we could but isn't it nice to have gradle regenerate the swagger files on-demand ?

I.e. you could run assembleDebug, build, compileMainReleaseeKotlin, etc... and Gradle would make sure the generated files are up-to-date without having to run generateSwagger first.

}
}
}
3 changes: 3 additions & 0 deletions gradle.properties
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# The check task requires a lot of MetaSpace. Not sure if it is a leak or something else
# But it is needed to make the tasks pass
org.gradle.jvmargs=-Xmx4g -XX:MaxMetaspaceSize=2g