Feat/support gradle root deps #116

xizhao · 2018-03-20T19:28:17Z

This PR adds support for Gradle projects that manage their dependencies via a root dependencies task rather than within a subtask.

Refs: #74 #114 #10

Changes

When fossa init does not find dependency subtasks, have it elect the root dependencies task for the name field in fossa.yml configuration (represented as an empty taskname + config :compile)
Support falling back to root dependency tasks when task name is unspecified in config
Strip ANSI from dependency output for more reliable parsing
Add extensive documentation + troubleshooting for our gradle integration
Add some configuration handling improvements

How to configure

For users that want to support root dep tasks, this should be a non-breaking change to existing config as well as provide OOTB support for new fossa init.

Options:

Overwrite existing config with fossa init -O and confirm the right task/configuration in fossa.yml

OR

Change your .fossa.yml file to have an empty task name; optionally specify your taskConfiguration:

analyze:
  modules:
    - name: :compile # this is valid, and will run `gradle dependencies --configuration=compile`
      path: build.gradle
      type: gradle
    - name: ''  # this is also valid, and will run `gradle dependencies --configuration=compile`
      path: build.gradle
      type: gradle

…ssas/fossa-cli into feat/support-gradle-root-deps * 'feat/support-gradle-root-deps' of https://github.com/fossas/fossa-cli: chore: Add TODOs, ignore third_party in autoconfig Various improvements to install.sh (#109)

elldritch · 2018-03-20T20:28:40Z

docs/integrations/gradle.md


-### Analysis
+If you'd like to help us improve our Gradle integration, we have 


elldritch · 2018-03-20T20:29:08Z

docs/integrations/gradle.md


-Analysis will support any tasks you can run `:dependendencies` on with the "compile" configuration.  This is equivalent to `gradle app:dependencies --configuration=compile`.
+ 1. Support multiple independent "root" Gradle builds that may be segregated in a codebase


These should probably also be tracked as issues, tagged with help wanted.

elldritch · 2018-03-20T20:30:05Z

docs/integrations/gradle.md

@@ -11,24 +11,103 @@ Gradle support in FOSSA CLI depends on the following tools existing in your envi

 ### Automatic Configuration

-When running `fossa init`, FOSSA will look for a root Gradle build (`build.gradle`) and interrogate it for build-able tasks.  From there, it will write module configurations into your `.fossa.yml` for every task that supports a `:dependency` sub-task.
+An average Gradle build, there are often hundreds of configuration paths that can influence what dependencies are included.  In order to determine what tasks and configurations you care about, `fossa` will attempt a "best-effort" strategy during `fossa init` to discover the most likely configuration for a production-build scenario.


elldritch · 2018-03-20T20:30:40Z

docs/integrations/gradle.md

+When running `fossa init`, FOSSA executes the following steps:
+
+ 1. Look for a root Gradle build (`build.gradle`)
+ 2. Run and parse the output of `gradle tasks --all -q -a --offline` to seek all available sub-tasks that support a `:dependencies` command


s/seek/find

elldritch · 2018-03-20T20:31:13Z

docs/integrations/gradle.md

@@ -11,24 +11,103 @@ Gradle support in FOSSA CLI depends on the following tools existing in your envi

 ### Automatic Configuration

-When running `fossa init`, FOSSA will look for a root Gradle build (`build.gradle`) and interrogate it for build-able tasks.  From there, it will write module configurations into your `.fossa.yml` for every task that supports a `:dependency` sub-task.
+An average Gradle build, there are often hundreds of configuration paths that can influence what dependencies are included.  In order to determine what tasks and configurations you care about, `fossa` will attempt a "best-effort" strategy during `fossa init` to discover the most likely configuration for a production-build scenario.


This is all kind of just marketing noise. I'd remove it.

elldritch · 2018-03-20T20:33:11Z

docs/integrations/gradle.md

+
+- `task` - the name of the task/subtask used to generate your build (most often this is `app` or empty `` for root)
+- `configuration` - the configuration your task runs during a production build (if undefined, defaults to `compile`)
+- `path` - path to your `*.gradle` build file (usually this is just `build.gradle`)


Relative or absolute? Either? If relative, relative to what?

elldritch · 2018-03-20T20:34:57Z

builders/gradle.go

 		taskConfiguration = moduleConfigurationKey[1]
 	}

-	// TODO: We need to let the user configure the right configurations
 	// NOTE: we are intentionally using exec.Command over runLogged here, due to path issues with defining cmd.Dir


What is the issue described in this comment?

elldritch · 2018-03-20T20:38:04Z

builders/gradle.go

-	if len(moduleConfigurationKey) == 2 {
+	if taskName != "" {
+		// this is not the root gradle project
+		taskName = taskName + ":"


Avoid mutation, it makes your intent much less clear.

I'd locate this closer to where the variable is used, so it's clear that this is formatting for usage as an argument to gradle.

elldritch · 2018-03-20T20:39:04Z

builders/gradle.go

+		// this is not the root gradle project
+		taskName = taskName + ":"
+	}
+	if len(moduleConfigurationKey) == 2 && moduleConfigurationKey[1] != "" {


Move this up to be next to the definition of taskConfiguration, so it's clear that your intent is to use a default value.

elldritch · 2018-03-20T20:55:29Z

builders/gradle.go

 		}

-		// Fall back to "app" as default task, even though technically it would be "" (root)
+		// If task list fails, fall back to "app" as default task, even though technically it would be "" (root)


Is this behaviour correct? Should we expect app:compile to work even when gradle tasks fails, or should this be a fatal error?

I'm 50/50 on having this fallback vs erroring if gradle tasks fails, but:

I believe app:compile is a sane default if a build.gradle is found

Checking for a build.gradle is a valid way of identifying a module, and much simpler/more reliable then parsing output of gradle tasks

I think the question is whether app:compile is likely to work. If it works, then this is great. If it doesn't work, then we've kicked a configuration error down the line into a much more obscure runtime error.

I don't have much experience with the Gradle ecosystem, so I trust your intention on this.

elldritch · 2018-03-20T22:15:35Z

builders/gradle.go

-		taskName = taskName + ":"
+
+	taskName := "dependencies" // defaults to root gradle dependencies task
+


Remove newline (makes default-value intention clearer).

elldritch · 2018-03-20T22:18:26Z

builders/gradle.go

 		}

-		// Fall back to "app" as default task, even though technically it would be "" (root)
+		// If task list fails, fall back to "app" as default task, even though technically it would be "" (root)


I think the question is whether app:compile is likely to work. If it works, then this is great. If it doesn't work, then we've kicked a configuration error down the line into a much more obscure runtime error.

I don't have much experience with the Gradle ecosystem, so I trust your intention on this.

elldritch · 2018-03-20T22:18:59Z

docs/integrations/gradle.md

+ 1. Look for a root Gradle build (`build.gradle`)
+ 2. Run and parse the output of `gradle tasks --all -q -a --offline` to find all available sub-tasks that support a `:dependencies` command
+ 3. If task list succeeds but no sub-tasks are available, fallback to the root `dependencies` task in `build.gradle`
+ 4. Filter out any suspicious-looking tasks (i.e. labeled `test` or `testCompile`)


You should really mention that there's a flag to disable this.

xizhao added 3 commits March 20, 2018 11:39

feat(builders): #114 support root dependencies task

97c7315

fix(builders): allow for empty configuration in gradle

5b73fa4

doc(builders): add better gradle docs

15f5af5

xizhao requested a review from elldritch March 20, 2018 19:28

xizhao added 7 commits March 20, 2018 12:33

fix(builders): set TERM=dumb when running gradle dependencies task

1030bd6

Merge branch 'master' into feat/support-gradle-root-deps

60818b4

fix(builders): fix gradle syntax err

db5b36b

doc(builders): improve gradle builder docs

748f307

doc(builders): add another gradle common task

0583626

feat(cmd): add spinner to init cmd

7ef81e0

elldritch suggested changes Mar 20, 2018

View reviewed changes

fix(builders): fix PR comments

99a9552

elldritch approved these changes Mar 20, 2018

View reviewed changes

xizhao merged commit 85c1788 into master Mar 21, 2018

elldritch deleted the feat/support-gradle-root-deps branch April 10, 2018 02:01

meghfossa pushed a commit that referenced this pull request Nov 12, 2021

ghc 8.10 (#116)

9884037

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat/support gradle root deps #116

Feat/support gradle root deps #116

xizhao commented Mar 20, 2018 •

edited

Loading

elldritch Mar 20, 2018

elldritch Mar 20, 2018

elldritch Mar 20, 2018

elldritch Mar 20, 2018

elldritch Mar 20, 2018

elldritch Mar 20, 2018

elldritch Mar 20, 2018

elldritch Mar 20, 2018

elldritch Mar 20, 2018

elldritch Mar 20, 2018

xizhao Mar 20, 2018

elldritch Mar 20, 2018

elldritch Mar 20, 2018

elldritch Mar 20, 2018

elldritch Mar 20, 2018


		### Analysis
		If you'd like to help us improve our Gradle integration, we have


		Analysis will support any tasks you can run `:dependendencies` on with the "compile" configuration. This is equivalent to `gradle app:dependencies --configuration=compile`.
		1. Support multiple independent "root" Gradle builds that may be segregated in a codebase

		taskName = taskName + ":"

		taskName := "dependencies" // defaults to root gradle dependencies task

Feat/support gradle root deps #116

Feat/support gradle root deps #116

Conversation

xizhao commented Mar 20, 2018 • edited Loading

Changes

How to configure

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xizhao commented Mar 20, 2018 •

edited

Loading