Skip to content
This repository has been archived by the owner on Apr 1, 2022. It is now read-only.

Analysis-target filtering #140

Merged
merged 91 commits into from
Oct 8, 2020
Merged

Analysis-target filtering #140

merged 91 commits into from
Oct 8, 2020

Conversation

cnr
Copy link
Contributor

@cnr cnr commented Sep 28, 2020

This is a big one.

Summary

See the design doc for additional context.

  • This PR adds a project-level analysis-target for all project types
  • This PR adds support for subproject analysis-targets for gradle only. Further subproject support will come in the future.

User-facing additions

fossa analyze --filter ANALYSIS-TARGET

This allows for project-wide filtering or analysis-target-specific filtering. When more than one filter is specified, an analysis-target that passes any of the filters will be analyzed

fossa list-targets

This command lists valid analysis-targets that can be used with --filter.

Example output:

Found project: maven@examples/example-gauth/
Found project: maven@examples/example-tls/
Found project: maven@examples/
Found project: gradle@./
Found target: gradle@./::grpc-all
Found target: gradle@./::grpc-alts
Found target: gradle@./::grpc-api
Found target: gradle@./::grpc-auth
Found target: gradle@./::grpc-benchmarks
Found target: gradle@./::grpc-bom
Found target: gradle@./::grpc-compiler
Found target: gradle@./::grpc-context
Found target: gradle@./::grpc-core
Found target: gradle@./::grpc-gae-interop-testing-jdk8
Found target: gradle@./::grpc-grpclb
Found target: gradle@./::grpc-interop-testing
Found target: gradle@./::grpc-netty
Found target: gradle@./::grpc-netty-shaded
Found target: gradle@./::grpc-okhttp
Found target: gradle@./::grpc-protobuf
Found target: gradle@./::grpc-protobuf-lite
Found target: gradle@./::grpc-services
Found target: gradle@./::grpc-stub
Found target: gradle@./::grpc-testing
Found target: gradle@./::grpc-testing-proto

Implementation notes

strategy-focused pipeline

Historically, we've had a very "strategy"-focused analysis pipeline, where we have a bunch of possibly-related strategies, that don't know about each other, that all go try to find and analyze projects independently.

For example, a yarn-lock strategy that only does static analysis on yarn.lock would be run independently of a yarn-command-running strategy. We'd pick the "best" strategy's dependency graph when deduplicating projects.

This has a couple of downsides:

  • There are cases when we'd want to merge dependency graphs between strategies, e.g., a setup.py and a requirements.txt strategy would want their dependency graphs unioned.
  • This makes it harder to reason about which dependency graph will be used

As part of these changes, we move toward a more project-focused analysis (e.g., "yarn"). This is critical for allowing users to select projects as analysis-targets

subprojects

Subprojects are not a well-supported notion in our current datatypes, leading to the hacky workaround of a SubprojectType dependency type. We need to reify subprojects as a first-class construct, to both eliminate that tech debt, but also to allow users to select subprojects as analysis-targets.

These changes reify "BuildTargets" as part of the project, which are typically subprojects -- but in the future, things like bazel build targets will fit there too.

Bonus improvements

  • combine setup.py and requirements.txt dependency graphs
  • use maven-pom project discovery for maven plugin strategy. this wildly improves discovery for maven-plugin-applicable projects
  • fix bug in gradle analyzer: "./gradlew" doesn't work as an executable, because exec on posix is relative to the working directory of the parent process

Future tickets

Features

  • analysis-target enumeration for subprojects
    • maven
    • ninja
    • nuget
    • cargo
    • sbt
  • explain why we found a project in list-targets
  • add a subproject graph to dependency graph output && eliminate SubprojectType

@skilly-lily skilly-lily self-assigned this Oct 7, 2020
Comment on lines 89 to 107
[ Bundler.discover',
Cargo.discover',
Carthage.discover',
Cocoapods.discover',
Gradle.discover',
Rebar3.discover',
Gomodules.discover',
Godep.discover',
Setuptools.discover',
Maven.discover',
Leiningen.discover',
Composer.discover',
Cabal.discover',
Stack.discover',
Yarn.discover',
Npm.discover',
Scala.discover',
RPM.discover',
RepoManifest.discover'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have we decided on a name? Are we sticking with discover'?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just renamed to discover

src/Strategy/RPM.hs Outdated Show resolved Hide resolved
@skilly-lily skilly-lily self-requested a review October 8, 2020 01:40
Copy link
Contributor

@skilly-lily skilly-lily left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs some items addressed as already discussed, but overall PR is in good shape. Last remaining bits after those changes is to confirm that all strategies have been migrated, check pathfinder still works (with licenses), check the fork safety issue you mentioned, and do a VERY short write up of how we tackle the easy discovery tests (in a later follow-up PR on the test repo, of course).

Additionally, this PR should probably update the GH actions workflow to fix our licensing issues. Since the cabal strategy has been already migrated, there should be no blocker there.

@cnr cnr merged commit 427c73e into master Oct 8, 2020
@cnr cnr deleted the archery branch October 8, 2020 17:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants