[WIP] Upgrade gradle to 5

[pablisco]

This is an experiment to see if we can upgrade to Gradle 5.

Locally the tests fail so it would be interesting to see if they work on CI

[zegnus]

should this go to develop instead?

[pablisco]

The CI says no:

    * What went wrong:
    Could not create service of type FileHasher using GradleUserHomeScopeServices.createCachingFileHasher().
    > Timeout waiting to lock file hash cache (/tmp/.gradle-test-kit-jenkins/caches/5.2.1/fileHashes). It is currently in use by another Gradle instance.
      Owner PID: 22085
      Our PID: 32765
      Owner Operation: 
      Our operation: 
      Lock file: /tmp/.gradle-test-kit-jenkins/caches/5.2.1/fileHashes/fileHashes.lock

I've had this problem locally too. Uncertain if we are using GradleRunner in a way that conflicts with gradle itself (maybe because we use withPluginClasspath()) 🤔

[pablisco]

Test are running ok now, but we have a problem which is fixed on #170 :

* What went wrong:
    Could not determine the dependencies of task ':findbugsDebug'.
    > Could not get unknown property 'classesDir' for debug classes of type org.gradle.api.internal.tasks.DefaultSourceSetOutput.

[pablisco]

From the build, we also need a settings.gradle in the test project generated by DeployRulesTestRule

Project directory '/storage/jenkins/workspace/gradle-static-analysis-plugin-prb/plugin/build/test-projects/1551778496887' is not part of the build defined by settings file '/storage/jenkins/workspace/gradle-static-analysis-plugin-prb/settings.gradle'. If this is an unrelated build, it must have its own settings file.

[pablisco]

@mr-archano
The problem now is that, after changing to use to classesDirs FindBugs tests is broken because project.fileTree takes a single file instead of a list of them... So the tests that expect failures are passing (I imagine that it doesn't find any sources)

Looking at this.

[pablisco]

Correction, it's not just FindBugs that is giving false positives

[pablisco]

Current issue:

* What went wrong:
A problem occurred configuring root project '1551803380988'.
> Failed to notify project evaluation listener.
   > org.gradle.api.tasks.TaskInputs.dir(Ljava/lang/Object;)Lorg/gradle/api/tasks/TaskInputs;
   > org.gradle.api.tasks.TaskInputs.dir(Ljava/lang/Object;)Lorg/gradle/api/tasks/TaskInputs;

Looking into it it turns out that we have an outdated version of the Kotlin plugin which I presume didn't work with Gradle 5

[tasomaniac]

🤞

[pablisco]

I think that adding .withTestKitDir(new File(projectDir, "test-kit")) to the tests adds significant run time for the tests (from 15 mins to about an hour) because it copies the test kit each test.

So it may be worth exploring a better alternative 🤔

[tasomaniac]

The CI is failing because of the deprecation in Findbugs. Aka #94. I would suggest to handle that in a separate PR using Gradle 4.10 which would make this one easier. Wdyt?

[mr-archano]

@tasomaniac I suggested the same to @pablisco, but I thought there was some problem with that anyway, given the build for #170 was failing too.

@pablisco could you please give us an update when you manage?

[tasomaniac]

I believe the change in #170 may not be enough.

[tasomaniac]

Sorry, accidentally closed

[pablisco]

Got a fix on the findBugs problem: 4e5a6ed

def fileTrees = sourceSet.output.classesDirs.collect { classesDir ->
    project.fileTree(classesDir) {
        builtBy(sourceSet.output)
        include(includes)
    }
}
if (fileTrees.isEmpty()) {
    throw GradleException("No file tree from ${sourceSet} and filters: ${includes}")
} else if (fileTrees.size() == 1) {
    fileTrees.first()
} else {
    fileTrees.inject(fileTrees.head()) { result, current -> result + current }
}

Why couldn't they be called map and fold like in other languages? 🤦‍♂️ Groovy

[pablisco]

Hmm, it seems to also fix the issues with KtlintIntegrationTest locally, let's check with the CI in the morning 🌞

[tasomaniac]

Hurray 🎉 Ktlint is failing now because of Android tooling. Updating AGP version in test projects may help.

[pablisco]

Ok, it didn't work for KtlintIntegrationTest. But we are down to 5 failures 🎉

[mr-archano]

@pablisco let's have a chat about this method, I don't know if you have checked when this was introduced, but that code was copy/pasted from the FindbugsPlugin source and has changed since then: #20 (comment)

[pablisco]

The code has changed but the reason hasn't, has it?

[mr-archano]

@pablisco don't know to be honest, I wonder if there's a simpler way of achieving what we want (as per description of #20)

[tasomaniac]

Looked into what's wrong with ktlint. It is because of this issue JLLeitschuh/ktlint-gradle#201

My test upgrade caused the failure :) I guess that's for the better. Will update the tests in this branch.

[pablisco]

Regarding the time inflation we are observing, it was suggested on Gradle's Slack a possible solution involving making a snapshot of the testKit and copying it over for each test...

[pablisco]

@tasomaniac It's still failing on 7.0 🤔

An exception occurred applying plugin request [id: 'org.jlleitschuh.gradle.ktlint', version: '7.0.0']

[tasomaniac]

I thought 7.0 bug was only for Android projects. We should then disable it completely.

[mr-archano]

Closing this as partially superseded by the 0.8.1 release (#182).

The OutOfMemoryError issue with test-kit will be tracked by #168.