Make evaluateViolations task run for specific buildVariant

[novoda]

Make evaluateViolations task run for specific buildVariant

[tasomaniac]

My suggestion is a workaround/hack. I personally think this would be good for the plugin.

[tasomaniac]

What about the variantFilter? Is that not solving this problem?

[bitsydarel]

No, because variant filter does not allow to dynamically choose a variant to include.
Which means that i have only two choices, include all variant or only one... Which is not ideal for automation...

[bitsydarel]

So if I have to run checks for each build I have to change the included variant everytime I want to run static analysis tool for a specific variant... Which is not ideal.

[tasomaniac]

I think this is really nice to have. I'm really curious why the tasks were designed to have only single evaluateViolations that runs all variants. Maybe @mr-archano can enlighten us.

Here is a "hack/trick" or "gradle magic" that you can use to achieve what you want.

Have this following variant filter in your configurations. This was you will be able to configure it from command line. You can run ./gradlew evaluateViolations -PcheckVariant=fooBarDebug

staticAnalysis {
    lintOptions {
        includeVariants { 
            it.name == project.properties['checkVariant']
        }
    }
}

On top of that, you can also make use of GradleBuild task to create tasks for each flavor to do this for you.

applicationVariants.all { variant ->
    project.tasks.create("evaluateViolations${variant.name.capitalize()}", GradleBuild) {
            startParameter = gradle.startParameter.newBuild() // copy from the real build. Needs to come first

            tasks = [ 'evaluateViolations' ]

            startParameter.projectProperties['checkVariant'] = variant.name
        }
}

Note: GradleBuild is a special task which creates another build inside a task execution. Especially if you enable parallel builds, you should not run this with other tasks together.

[bitsydarel]

@tasomaniac Sorry for the delay,

To get the current buildVariant i have this function:

def getCurrentFlavor() {
    File cxConfigFile = new File(project.projectDir.toString() + "/cx.iml")
    if (!cxConfigFile.exists()) return ""
    else {
        String cxconfig = cxConfigFile.text
        Pattern mather = Pattern.compile("(<option name=\"SELECTED_BUILD_VARIANT\" value.*)")
        Matcher matcher = mather.matcher(cxconfig)
        String flavor
        if(matcher.find()) {
            // we found <option name="SELECTED_BUILD_VARIANT" value="variant" />
            flavor = matcher.group(1)
            // transform it to "variant_name"
            flavor = flavor.substring(flavor.lastIndexOf("value"), flavor.length())
            // remove the " before the name and after the name "
            flavor = flavor.substring(flavor.indexOf("\"") + 1, flavor.lastIndexOf("\""))
        } else {
            flavor = ""
        }

        return flavor
    }
}

The issue is that it's only work when building from android studio but from jenkins nope, that's because the iml file is created by android studio but for now this is not an issue.

LintOptions completly ignore the includeVariants, that was my first try by the way.

So if i understand, GradleBuild will create an new build related to each buildvariant.
this line create a ext property that the includeVariant filter for lintOptions:
startParameter.projectProperties['checkVariant'] = variant.name

But i don't understand the purpose of this line :
tasks = [ 'evaluateViolations' ]
Because if we specify "evaluateViolations" as the only available task for each new build then why not just running it directly when building ?

so the only important configuration here is

lintOptions {
        includeVariants { 
            it.name == project.properties['checkVariant'] // the current selected buildVariant
        }
    }

[tasomaniac]

Well, it is because you are able to parameterize and have different configurations for evaluateViolations with this trick.

You can create flavor and buildType dependant variations of the same task but dynamically configure that differently.

[bitsydarel]

I see, thank you very much for the suggestions.
Any plan on adding this feature into the tool or we should just use the provided suggestions?

[mr-archano]

I think I somehow missed this issue. It looks like an interesting request: we should be able to create specific evaluate${variantName}Violations tasks, but the issue I see is that I don't know how the thresholds will work in that case. Is that desired to use (for instance) the same maxWarnings/maxErrors across variants?

[tasomaniac]

To be honest, Android approach with flavors is powerful but makes everything complicated. I don't think any of the tools we support handles this situation. Your suggestion makes sense to me, although I think we need to make it super clear.

[bitsydarel]

The thresholds are not configuration dependent? So why not just generate different configuration for each flavor?

[mr-archano]

@bitsydarel, to be honest I am pretty confused by your messages, so I will try to clarify few points. Apologies in advance if I am stating obvious things, I just want to be sure we're on the same page.

Gradle builds in Android (and specifically the Android Gradle Plugin) support build variants but there is no such a thing as "selected variant"; that instead is a feature of the IDE that decides to just enable one build variant per module to allow you to work comfortably. This is basically the same as disabling variants using variantFilter yourself. Validating this is as simple as running evaluationViolations clicking on the task listed in the Gradle tool window: you will notice that each static analysis tool will run only on the sourceset(s) included in the active build variant of the IDE.

When you say "building from Android Studio" I am not sure what you mean, as the default run configuration to build the app definitely doesn't run check nor evaluateViolations, so maybe you mean when you build the app from the terminal integrated in Android Studio? (more on this below)

Building from command line (like Jenkins will do, or like you do using the terminal integrated in your IDE) means that you are just using Gradle, no IDE gimmick, hence you will experience the standard behaviour of the build script, where all the build variants are active for an Android module unless explicitly disabled (ie: via variantFilter as mentioned above). To avoid this and to allow people to actually target a single variant (or conversely - to skip the analysis on some build variants) we have provided the includeVariant as documented in advanced-usage.

What @tasomaniac suggested is a clever use of a nested build to keep using evaluateViolations in its current form by creating a child Gradle build execution where you will explicitly enable only one build variant. That will be still using the common thresholds as defined in the staticAnalysis {} extension. This trick can be avoided if we manufacture a meta-task for each build variant that can be used to trigger all the configured tools on the sourcesets/classes relevant to that variant.

The thing I'm not onboard with at all is to provide thresholds and tool configurations per-variant: that will add IMO a level of complexity and a whole new level of edge cases that I am not really sure will be worth the effort. Consider again that as @tasomaniac mentioned none of the static analysis tools supported by this plugin is build variant-aware, not even Android lint, where the configuration is using the lintOptions {} extension.

In conclusion: this plugin already offers includeVariants as a way to address exactly the needs like yours and it is based on a more idiomatic and simpler way of defining build scripts. It should be quite simple to inject the variant to be analysed as build parameter in your Jenkins configuration and then use includeVariants to solve your problem. What do you think?

[bitsydarel]

Thanks for the suggestions, that's actually what I did at first and it's currently configured that way in our Jenkins. But I wanted to also make it dynamically in android studio so my colleagues can run checks before doing a pull request. The issue was that even after applying the incudeVariants it's still compile other variants and since we have many variants it's take time to build ...

[rock3r]

Unfortunately static analysis takes a long time, there's not much to do about it. Running Detekt and KtLint is fast because they don't compile the app, but Android Lint takes forever. You can optimise your setup a bit, but it'll still be very slow. Checkstyle, Findbugs and PMD are all kinds of slow, and there's very little you can do to make them faster unfortunately.

If you have a mixed Java + Kotlin project then it'll take even longer since compilation will be slower, especially if you have apt + kapt in action, since the Kotlin compiler needs to be invoked more than once (before and after javac) if there's any Java code in the same module as Kotlin code.

All that is hardly fixable in a meta-plugin such as this; we can't even do things such as using PMD incremental support as Gradle's PMD plugin doesn't support it yet either. Having almost all the supported tools being non-Android specific makes things even harder; trying to selectively compile things is near impossible to obtain without going crazy either... and it's still not going to solve all the perf issues outlined above, unfortunately.

[maxbach]

Guys, I think, tasks for specific buildVariant is useful. Using includeVariants is not so flexible. What are the arguments against the feature?