From b12271694d9b86ebcb60a9e39c74898638436ef4 Mon Sep 17 00:00:00 2001 From: Leo Zhang Date: Mon, 2 Aug 2021 02:00:18 -0700 Subject: [PATCH] Respond to code review comments --- docs/strategies/gradle.md | 36 +++++++++++----------- scripts/jsondeps.gradle | 63 +++++++++++++-------------------------- src/Strategy/Gradle.hs | 6 ++-- 3 files changed, 41 insertions(+), 64 deletions(-) diff --git a/docs/strategies/gradle.md b/docs/strategies/gradle.md index dcc001d4f..03811110c 100644 --- a/docs/strategies/gradle.md +++ b/docs/strategies/gradle.md @@ -2,11 +2,11 @@ [Gradle](https://gradle.org/) is a polyglot tool mostly used by JVM projects. It's popular with Java and Android projects. -Gradle users generally specify their builds using a `build.gradle` file, written in Groovy. These builds are specified as programs that must be dynamically evaluated. +Gradle users generally specify their builds using a `build.gradle` file (written in Groovy) or a `build.gradle.kts` file (written in Kotlin). These builds are specified as programs that must be dynamically evaluated. -| | | -| --------- | ---------------------------------------- | -| :warning: | This strategy requires dynamic analysis. | +| | | +| --------- | ------------------------------------------------------------------------- | +| :warning: | This strategy requires dynamic analysis, which requires a CI integration. | ## Table of contents @@ -34,7 +34,7 @@ Most sizable Gradle builds organize their dependencies with two concepts: subpro #### Subprojects -_Subprojects_ are used when you have multiple "projects" in a single Gradle build (e.g. having multiple projects in a single repository managed by a single `build.gradle`). Gradle calls these "multi-projects". Gradle multi-projects have one root project, and one or more subprojects. +_Subprojects_ are used when you have multiple "projects" in a single Gradle build (e.g. having multiple projects in a single repository managed by with single `settings.gradle` and one or more `build.gradle` files). Gradle calls these "multi-projects". Gradle multi-projects have one root project, and one or more subprojects. A single subproject roughly corresponds to a single set of outputs. Hence, we treat subprojects as separate analysis targets. @@ -80,17 +80,17 @@ In this documentation below, for brevity, we'll always refer to the selected too ## Discovery -This strategy discovers analysis targets by looking for files in the folder being analyzed whose names start with `build.gradle`. This matches both `build.gradle` as well as `build.gradle.kts` and build files in other Gradle-supported languages (`build.gradle.*`). +This strategy discovers analysis targets by looking for files in the folder being analyzed whose names start with `build.gradle`. This matches both `build.gradle` as well as `build.gradle.kts` and Gradle build scripts in other Gradle-supported languages (`build.gradle.*`). It then executes `gradle projects` in the directory where the build file is found to get a list of subprojects for this Gradle build. These subprojects are used to create the analysis targets for this Gradle build. -If there are no subprojects, a analysis target is created that analyzes the root project. Otherwise, a set of analysis targets is created: one for each Gradle subproject. +If there are no subprojects, an analysis target is created that analyzes the root project. Otherwise, a set of analysis targets is created: one for each Gradle subproject. ## Tactics ### Tactic selection -This strategy selects tactics by trying them in preference order and using the results of the first tactic that succeeds. +This strategy selects tactics by trying them in preference order and uses the results of the first tactic to succeed. The order of tactics for this strategy is: @@ -99,11 +99,11 @@ The order of tactics for this strategy is: ### Gradle build plugin -| | | -| ------------------ | ---------------------------------------------------------------------------------------- | -| :heavy_check_mark: | This tactic reports dependencies for all subprojects. | -| :heavy_check_mark: | This tactic provides a graph for subproject dependencies. | -| :warning: | This tactic requires dynamic analysis. | +| | | +| ------------------ | --------------------------------------------------------- | +| :heavy_check_mark: | This tactic reports dependencies for all subprojects. | +| :heavy_check_mark: | This tactic provides a graph for subproject dependencies. | +| :warning: | This tactic requires dynamic analysis. | This tactic runs a Gradle [init script](https://docs.gradle.org/current/userguide/init_scripts.html) to output the dependencies in each Gradle subproject. Mechanically, this tactic: @@ -115,8 +115,6 @@ This init script is implemented [here](https://github.com/fossas/spectrometer/bl The script works by iterating through configurations, resolving their dependencies, and then serializing those dependencies into JSON. -Currently, this script only reports dependencies in the `default` configuration of each subproject. - ### Parsing `gradle :dependencies` | | | @@ -134,7 +132,7 @@ To determine whether the CLI is properly detecting your Gradle project, run `fos -For each of your Gradle subprojects, you should see a `gradle@PATH_TO_GRADLE_SUBPROJECT` target in the list of analysis targets. +For each of your Gradle subprojects, you should see a `gradle@PATH_TO_ROOT_PROJECT:SUBPROJECT` target in the list of analysis targets. If you _don't_ see this, one of two things is likely happening: @@ -158,13 +156,13 @@ The Gradle build plugin is a Gradle [init script](https://docs.gradle.org/curren If this tactic doesn't appear to be working (e.g. is giving you incorrect dependencies or is missing dependencies), you can run the init script directly using: ``` -gradle -I$PATH_TO_SCRIPT $SUBPROJECT1:jsonDeps $SUBPROJECT2:jsonDeps ... +gradle -I$PATH_TO_SCRIPT jsonDeps ``` -For example, for a Gradle build with subprojects `foo` and `bar` and the script extracted to `/tmp/jsondeps.gradle`, you should run (from within the build file's working directory): +For example, with the script extracted to `/tmp/jsondeps.gradle`, you should run (from within the Gradle build script's working directory): ``` -gradle -I/tmp/jsondeps.gradle foo:jsonDeps bar:jsonDeps +gradle -I/tmp/jsondeps.gradle jsonDeps ``` Providing this output with a bug report will help us debug issues with the analysis. diff --git a/scripts/jsondeps.gradle b/scripts/jsondeps.gradle index 27d4aec92..585066bc3 100644 --- a/scripts/jsondeps.gradle +++ b/scripts/jsondeps.gradle @@ -56,6 +56,14 @@ allprojects { // iterator can be empty (for dependencies with no artifacts // e.g. `jackson-bom`). // + // See also: + // - https://docs.gradle.org/current/userguide/declaring_dependencies.html#sub:module_dependencies + // - https://stackoverflow.com/questions/67328406/what-is-junit-bom-and-junit-platform-for-and-should-i-include-them-in-gradle-de + if (resolvedDep.moduleArtifacts.size() == 0) { + println "DEBUG: Dependency has no module artifacts" + return null + } + // artifact: https://docs.gradle.org/current/javadoc/org/gradle/api/artifacts/ResolvedArtifact.html def artifact = resolvedDep.moduleArtifacts.iterator().next() @@ -83,24 +91,9 @@ allprojects { if (!resolvedDep.children.isEmpty()) { // childResolvedDep: https://docs.gradle.org/current/javadoc/org/gradle/api/artifacts/ResolvedDependency.html resolvedDep.children.each { childResolvedDep -> - // If we don't do this check, then calling - // `resolvedDep.moduleArtifacts.iterator().next()` - // above explodes because the `moduleArtifacts` - // iterator is empty. - // - // This seems to happen for at least `jackson-bom` - // and and `junit-bom`. I'm not sure how this is - // possible. - // - // See also: - // - https://docs.gradle.org/current/userguide/declaring_dependencies.html#sub:module_dependencies - // - https://stackoverflow.com/questions/67328406/what-is-junit-bom-and-junit-platform-for-and-should-i-include-them-in-gradle-de - if (childResolvedDep.moduleArtifacts.size() > 0) { - def result = depToJSON childResolvedDep + def result = depToJSON childResolvedDep + if (result != null) { childResults << result - } else { - println "DEBUG: Found dep with no module artifacts" - println childResolvedDep } } } @@ -121,8 +114,6 @@ allprojects { // config: https://docs.gradle.org/current/javadoc/org/gradle/api/artifacts/Configuration.html def configToKeyValue = { config -> def jsonDeps = [] - println "DEBUG: Trying to resolve configuration" - println config // config.resolvedConfiguration: https://docs.gradle.org/current/javadoc/org/gradle/api/artifacts/ResolvedConfiguration.html config.resolvedConfiguration.firstLevelModuleDependencies.each { dep -> println "DEBUG: Found direct dependency in configuration" @@ -143,36 +134,24 @@ allprojects { println "DEBUG: Trying to resolve configuration" println config try { - def result = configToKeyValue config - println "DEBUG: Configuration resolved" - println result - jsonConfigs << result - } catch (IllegalStateException e) { - // This particular exception is fairly common, so we - // handle it specially. There's nothing to do in this - // case. Some configurations aren't meant to be - // resolved, because they're just meant to be containers - // of dependency constraints. + // Some configurations aren't meant to be resolved, + // because they're just meant to be containers of + // dependency constraints. // // See also: // - https://discuss.gradle.org/t/what-is-a-configuration-which-cant-be-directly-resolved/30721 // - https://docs.gradle.org/current/userguide/declaring_dependencies.html#sec:resolvable-consumable-configs - if (e.getMessage().contains("canBeResolved=false")) { - println "DEBUG: Could not resolve configuration" + if (config.canBeResolved) { + println "DEBUG: Configuration is resolvable" + def result = configToKeyValue config + println "DEBUG: Configuration resolved" + println result + jsonConfigs << result } else { - throw e + println "DEBUG: Configuration is not resolvable" } } catch (Exception ignored) { - // If more common exceptions occur, add a catch case to - // handle them. Groovy supports multiple catch clauses - // (the documentation calls this "multi-catch"), and - // will try to pattern match exceptions to handlers in - // order. - // - // See also: - // - https://www.tutorialspoint.com/groovy/groovy_exception_handling.htm - // - https://groovy-lang.org/semantics.html#_exception_handling - println "DEBUG: Some other exception occurred" + println "DEBUG: An exception occurred" println ignored ignored.printStackTrace() } diff --git a/src/Strategy/Gradle.hs b/src/Strategy/Gradle.hs index 939f7c471..b50e5755d 100644 --- a/src/Strategy/Gradle.hs +++ b/src/Strategy/Gradle.hs @@ -101,9 +101,9 @@ discover :: ) => Path Abs Dir -> m [DiscoveredProject run] -discover dir = - context "Gradle" $ - (fmap . fmap) mkProject $ context "Finding projects" $ findProjects dir +discover dir = context "Gradle" $ do + found <- context "Finding projects" $ findProjects dir + pure $ mkProject <$> found -- Run a Gradle command in a specific working directory, while correctly trying -- Gradle wrappers.