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

Add support for consolidated report in multi-project setup #1295

Closed
vasanthdharmaraj opened this issue Oct 30, 2018 · 42 comments
Closed

Add support for consolidated report in multi-project setup #1295

vasanthdharmaraj opened this issue Oct 30, 2018 · 42 comments
Assignees
Labels

Comments

@vasanthdharmaraj
Copy link

Based on the recent change highlighted in the change log, I have setup a multi-project by adding detekt to the subprojects like:

subprojects {
    detekt {
        // properties
    }
 }

The reports are generated successfully in the sub-projects. What should I do to get a consolidated report of all sub-projects? The earlier suggestions seem to be for the older profile based config and does not seem to work with this change.

@Mauin
Copy link
Collaborator

Mauin commented Oct 30, 2018

It would however be possible to add support for consolidating the reports if that is a requested feature. Adding "Waiting for Feedback" to see if more requests come in for this.

@Mauin Mauin changed the title 1.0.0.RC9.2 - consolidated report in multi-project setup Add support for consolidated report in multi-project setup Oct 30, 2018
@vasanthdharmaraj
Copy link
Author

Great. Thanks for the quick response.

@schalkms
Copy link
Member

schalkms commented Nov 2, 2018

This makes sense. This would be a nice improvement, I think.

@abelhegedus
Copy link

Before migrating to RC9, we took advantage of the aggregated report that could be easily published as part of a Jenkins build, similarly to JUnit results or JaCoCo coverage.
Additionally, I think for a multi-project setup, it makes more sense to fail the build by taking into account the aggregated score instead of separate modules.

@athkalia
Copy link

athkalia commented Nov 7, 2018

Having one report would indeed be great.

@marschwar
Copy link
Contributor

I have started to do some work on this topic and hope to have something to show for next week.

@fredgrott
Copy link

a question I have is detekt using gradle's reporting stuff if so than the gradle build dashboard should consolidate things into one dash board I mention it as we can look at the code for that plugin to bootstrap on how to consolidate reports...just thinking out loud as I have the same problem as I am doing clean arch as separate modules deps to my app module

@galet
Copy link

galet commented Mar 11, 2019

I think this is a legitimate and important enhancement. We have a project with 300+ modules, so having a single merged report would be great.

@arturbosch
Copy link
Member

Yes I agree. Please see the latest comments at #1329.
@galet 300 kotlin modules? How many minutes does detekt need to fully inspect your code base? Asking just of curiousity :)

@galet
Copy link

galet commented Mar 27, 2019

Thanks. The project contains mostly Java modules but only a few Kotlin or mixed Java/Kotlin modules since we just started with Kotlin adoption. Nevertheless, it's still hard to find and read detekt report one by one with so many modules.

@abelhegedus
Copy link

@arturbosch in case you are interested, in our project we have 27 modules completely written in Kotlin, totaling at 37k lines of code, with the biggest modules containing around 5k lines. The time it takes detekt to analyze everything is 77.4 seconds (based on the detekt finished in X ms. log messages) with individual modules coming in between 1700 and 5300 ms and reporting 1.2k issues in total.

@dimsuz
Copy link
Contributor

dimsuz commented Apr 3, 2019

I must say that after migrating to running detekt in each subproject, time to complete the analysis has greatly increased. We have about 30-40 modules and when detekt was running on all srcSets at once it took like 1.5 seconds max. Now it needs to go through more gradle stuff and... let me see... latest build took 1m 17s to complete ./gradlew detekt check on CI.

That's not much, but when I think about how fast it was earlier, I cringe a little.

Maybe that's due to reports it prints on each module and having consolidated report would help here (I hope).

@arturbosch
Copy link
Member

arturbosch commented Apr 7, 2019

Actually the report printing is like 15ms, the overhead is to start a process and setup the kotlin compiler for each module. So for each module 500-1000ms overhead.

@abelhegedus thanks for the info, 77 seconds is really much for 37k :/
Without this overhead it should like 9 seconds (for detekt it is 8.3 seconds for 30k loc)

@jaloszek
Copy link

jaloszek commented Jun 7, 2019

+1, definitely worth implementing.

@jbialkowski13
Copy link

Any update on this? This would simplify CI setup for multi modules application.

@kord2003
Copy link

kord2003 commented Jun 7, 2019

I would really like to have a consolidated report of all sub-projects

@jcornaz
Copy link

jcornaz commented Jun 19, 2019

@Mauin, @arturbosch is this issue still waiting for feedback?

@arturbosch
Copy link
Member

@jcornaz @whiter13 @kord2003 please try out RC15 which ships with #1329.
A merged report gets created when a higher level module has sources and sub modules.

@dimsuz
Copy link
Contributor

dimsuz commented Jun 19, 2019

Does this mean that I can remove this from my root-project's build.gradle:

subprojects {
   apply plugin: detekt
   detekt { 
      // config
   }
}

In earlier versions I had to apply detekt to each submodule like that. Can I now return to a single detekt configuration for top-level module only?

@jcornaz
Copy link

jcornaz commented Jun 19, 2019

please try out RC15 which ships with #1329.

Cool thanks.

Just sad that we cannot use RC15 in our team. We are blocked by #1686 :'-(

A merged report gets created when a higher level module has sources and sub modules.

But anyway our root module does not have sources...

@arturbosch
Copy link
Member

Does this mean that I can remove this from my root-project's build.gradle:

subprojects {
   apply plugin: detekt
   detekt { 
      // config
   }
}

In earlier versions I had to apply detekt to each submodule like that. Can I now return to a single detekt configuration for top-level module only?

Sure, please see https://github.com/arturbosch/detekt/blob/7de59b954c32d01b69e7f8a85ff8c01bbc174506/build.gradle.kts#L280-L295

@dimsuz
Copy link
Contributor

dimsuz commented Jun 19, 2019

Great, thank you very much, I will try this!

@joshfriend
Copy link

I had trouble getting detekt RC16 to run on all of my modules at once. I moved the detekt setup closure outside the subprojects {} section and it only reports issues with my main app module.

Here's how i had it configured:

//<editor-fold desc="detekt">
def detektVersion = "1.0.0-RC16"
apply plugin: 'io.gitlab.arturbosch.detekt'
detekt {
  toolVersion = detektVersion
  config = files("$rootProject.projectDir/detekt-config.yml")
  baseline = file("$rootProject.projectDir/detekt-baseline.xml")
  input = files("**/*.kt") // removing/adding this seems to have no effect

  reports {
    html {
      enabled = true
      destination = file("$project.buildDir/reports/detekt.html")
    }
    xml {
      enabled = true
      destination = file("$project.buildDir/reports/detekt.xml")
    }
  }
}

// enable formatting rules
dependencies {
  detektPlugins "io.gitlab.arturbosch.detekt:detekt-formatting:$detektVersion"
}
//</editor-fold>

@arturbosch I was confused by this section of your example above:

include("**/*.kt") 
include("**/*.kts") 
exclude("**/resources/**") 
exclude("**/build/**")

the include and exclude functions apply to SourceSet and I can't figure out how to make an equivalent construct in a traditional groovy syntax build.gradle.

@arturbosch
Copy link
Member

arturbosch commented Jul 27, 2019

@joshfriend we have recently updated the website doc for groovy based gradle build files, please see https://arturbosch.github.io/detekt/groovydsl.html

The include and exclude part was also confusing for me after we changed the Detekt task to extend the SourceTask. Gradle supports excluding and including files, so we wanted to be gradle idiomic here.
The include and exclude parts must be configured on the task level not on the extension level.

You could query the detekt task via tasks and configure them or declare a custom detekt task like mentioned in the docs:

task detektFailFast(type: io.gitlab.arturbosch.detekt.Detekt) {
   description = "Runs a failfast detekt build."

   input = files("src/main/java")
   config = files("$rootDir/config.yml")
   debug = true
   reports {
       xml {
           destination = file("build/reports/failfast.xml")
       }
       html.destination = file("build/reports/failfast.html")
   }
    include '**/*.kt'
    include '**/*.kts'
    exclude 'resources/'
    exclude 'build/'
}

I hope this helps!

@dimsuz
Copy link
Contributor

dimsuz commented Jul 29, 2019

The thing I discovered that moving detekt + tasks.withType(Detekt) out of subprojects clause, doesn't work for Android projects, which all usually have no sources in the rootProject.

Detekt simply reports: detekt: NO SOURCE and skips. I worked around this by doing:

tasks.withType(Detekt) {
  sources = files(rootProject.projectDir)
  ...
}

This way it grabs all files recursively and feeds them to detekt. But this has just broken because our project became so big that all the filenames do not fit in limit of allowed command line argument count :)

So now I'm back to doing

subprojects { 
 detekt {}
 tasks.withType(Detekt) {}
}

And analyze time is back to several minutes instead of 2 seconds :(

@joshfriend
Copy link

no sources in the rootProject

ohhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhh. 🤦‍♂I think that's related to #1768?

I thought I was just an idiot who couldn't figure out how to configure it properly.

Here's what finally works for me with RC16, this is from the root build.gradle:

//<editor-fold desc="detekt">
apply plugin: 'io.gitlab.arturbosch.detekt'

tasks.withType(io.gitlab.arturbosch.detekt.Detekt) {
    config = files("$rootProject.projectDir/detekt-config.yml")
    baseline = file("$rootProject.projectDir/detekt-baseline.xml")
    input = files(rootProject.projectDir)
    include '**/*.kt'
    include '**/*.kts'
    exclude '**/resources/'
    exclude '**/build/'
    reports {
        html {
            enabled = true
            destination = file("$rootProject.buildDir/reports/detekt.html")
        }
        xml {
            enabled = true
            destination = file("$rootProject.buildDir/reports/detekt.xml")
        }
    }
}
dependencies {
    detektPlugins "io.gitlab.arturbosch.detekt:detekt-formatting:1.0.0-RC16"
}
//</editor-fold>

Note that if you try to put the xml report anywhere but $rootProject.buildDir/reports/detekt.xml it will not follow your configuration and will put it there anyways :D

@dimsuz
Copy link
Contributor

dimsuz commented Jul 29, 2019

So you've done what we had:

input = files(rootProject.projectDir)

Be aware that all these files are passed to detekt through command line, and a) this fails on Windows after number of files exceed certain (low) amount b) this will fail even on Linux whenever number of files exceed a (larger) amount.

Note that if you try to put the xml report anywhere but $rootProject.buildDir/reports/detekt.xml it will not follow your configuration and will put it there anyways :D

That's because these settings are ignored when specified in tasks.withType() clause. We had xml.enabled = false there and it still generated them. Moving this section to detekt extension clause fixes this.

@marschwar
Copy link
Contributor

This way it grabs all files recursively and feeds them to detekt. But this has just broken because our project became so big that all the filenames do not fit in limit of allowed command line argument count :)

@dimsuz Which version of detekt are you using? Is the build on Windows still broken with RC16? I thought #1686 would fix the issue with too many files being passed via cli.

@dimsuz
Copy link
Contributor

dimsuz commented Jul 29, 2019

We use RC16. Although we used sources = files(rootProject.projectDir), not input = files(rootProject.projectDir). Maybe this is the cause?

But somehow I thought that input= accepts a list of directories to search for input files, does it work if I pass it llist with hundreds of .kt files? Didn't check (yet).

@AlexCzar
Copy link
Contributor

Any updates? What's the current status for 1.4.0? Latest information seems to be about 1.0.0-RC16

@arturbosch
Copy link
Member

arturbosch commented Jan 14, 2020

The current state for this is basically a conversation from slack:

Mario Sanoguera de Lorenzo:house_with_garden: Aug 17th, 2019 at 2:43 AM
what am I missing? how can I have a centralized baseline for the project?

6 replies

Artur Bosch  5 months ago
Unfortunately we haven't figured out how to have a centralized baseline file across different gradle modules.
Something that works for Gradle but is also as generic to be included into detekt-api would be great.

Artur Bosch  5 months ago
An idea which just came up:
we could have a modularized baseline which have different sections for each gradle module like detekt-cli and just override detekt-cli when run ...

Artur Bosch  5 months ago
The generic part is tricky... we could add a --baseline [prefix:]path option which the gradle plugin could use to let detekt know which baseline part to override...

A workaround is documented here: https://arturbosch.github.io/detekt/baseline.html#gradle.

We know this is a huge blocker for our users. We put it on the roadmap https://github.com/arturbosch/detekt/projects/4.
I see only #2035 being more important for now.

@AlexCzar
Copy link
Contributor

Thanks for the reply! I was asking about consolidated report - to simplify reporting on Jenkins - not about the baseline (which I at the moment don't even use), but #2035 is indeed more important.
Thanks again, detekt is [mostly] awesome :)

@arturbosch
Copy link
Member

Oh, I just read "consolidated" and instantly thought about the baseline issue.
Yes, both consolidations are important :).
Thx!

@volodia-chornenkyy
Copy link

@AlexCzar @arturbosch I've written a Gradle task which gathers all reports into one folder. Still, it is not a consolidated report but simplifies life on CI.
Gist file: https://gist.github.com/volodia-chornenkyy/f1e1c9e00ccd36aa6e23e040869b5b10

Also, as an idea, maybe there is a way to combine *.xml reports.

@drewhannay
Copy link

At my company, we have a solution internally that's based on merging the xml reports, and it works pretty well. High level summary is there's a separate Gradle plugin that is applied by the root project and adds a detektMerge task that knows how to combine reports, and properly hooks up dependencies on any other detekt tasks that are going to run for each submodule.

Not sure I'm able to share or contribute the actual code, but I'd highly recommend a solution that looks something like what I described.

@alexzaitsev
Copy link

I have to use several files (each for a module) for now. That's really inconvenient and it would be awesome to see a consolidated HTML report.

@cortinico
Copy link
Member

@alexzaitsev We're on it: #3394. A follow up will come up as soon as we have bandwidth

@G00fY2
Copy link
Contributor

G00fY2 commented Mar 11, 2021

So now with #3491 available with 1.16.0 we are able to merge the reports. But it is still required/best approach for Android projects to apply and configure the detekt plugin for each module in the subprojects block, right?

@chao2zhang
Copy link
Member

That's correct.

Meanwhile, I believe we need to apply detekt plugin per module, but it does not necessarily need to be subprojects in root projects.

@chao2zhang
Copy link
Member

This issue is resolved by #3394 and #3522. If there are new bug reports or feature requests, we can always discuss in new issue or discussion.

@artsmvch
Copy link

artsmvch commented Jun 2, 2023

Hello,
Are there any plans to make it possible to generate a single HTML report for all modules?

@schalkms
Copy link
Member

schalkms commented Jun 4, 2023

Hello,
Are there any plans to make it possible to generate a single HTML report for all modules?

@alexeiartsimovich
No, there aren't.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests