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

License scan dependencies #257

Merged
merged 13 commits into from
Jun 22, 2021
Merged

License scan dependencies #257

merged 13 commits into from
Jun 22, 2021

Conversation

zlav
Copy link
Member

@zlav zlav commented Jun 15, 2021

Overview

This PR adds support for license scanning vendored dependencies in a user's project. This initial implementation relies on the archive uploader.

These dependencies can be specified in the fossa-deps.yml file as vendored-dependencies

vendored-dependencies:
- name: Django
  path: vendor/Django-3.4.16.tar

Acceptance criteria

Adding a dependency to the fossa-deps.yml file causes it to be archive uploaded, license scanned on the backend, and displayed in the projects list of dependencies.

Testing plan

Validate that the archive upload works end to end

  • Create a fossa-deps.yml file with a dependency in the vendored section
  • Run fossa analyze
  • Inspect the project in the FOSSA UI and validate that the vendored dependency is present.=

Need to make sure files that don't exist are handled correctly.

Risks

I am most concerned about decisions that won't be reversible in the future. The biggest risk is unintuitive behavior to the user, so if anyone sees a spot a warning or an error should exist let me know.

References

#245

Closes https://github.com/fossas/team-analysis/issues/544

Checklist

  • I added tests for this PR's change.
  • I linted and formatted (via haskell-language-server) any files I touched in this PR.
  • If this PR introduced a user-visible change, I added documentation into docs/.
  • I updated Changelog.md. If this PR did not mark a release, I added my changes into an # Unreleased section at the top.
  • I linked this PR to any referenced GitHub issues.

@zlav zlav requested a review from cnr June 15, 2021 23:51
src/App/Fossa/Analyze.hs Outdated Show resolved Hide resolved
Copy link
Contributor

@cnr cnr left a comment

Choose a reason for hiding this comment

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

Overall looks really good -- one major suggestion and some nits

src/App/Fossa/ArchiveUploader.hs Show resolved Hide resolved
src/App/Fossa/FossaAPIV1.hs Outdated Show resolved Hide resolved
src/App/Fossa/FossaAPIV1.hs Outdated Show resolved Hide resolved
src/App/Fossa/FossaAPIV1.hs Outdated Show resolved Hide resolved
src/App/Fossa/FossaAPIV1.hs Outdated Show resolved Hide resolved
src/App/Fossa/FossaAPIV1.hs Outdated Show resolved Hide resolved
@zlav zlav requested a review from skilly-lily June 21, 2021 17:55
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.

Left a ton of nits, but found a major issue that might allow files with no dependencies.

This should also be formatted

Changelog.md Show resolved Hide resolved
docs/userguide.md Outdated Show resolved Hide resolved
src/App/Fossa/Analyze.hs Outdated Show resolved Hide resolved
src/App/Fossa/Analyze.hs Outdated Show resolved Hide resolved
src/App/Fossa/ArchiveUploader.hs Show resolved Hide resolved
docs/userguide.md Show resolved Hide resolved
src/App/Fossa/Analyze.hs Outdated Show resolved Hide resolved
src/App/Fossa/Analyze.hs Outdated Show resolved Hide resolved
src/App/Fossa/ArchiveUploader.hs Outdated Show resolved Hide resolved
src/App/Fossa/FossaAPIV1.hs Outdated Show resolved Hide resolved
@zlav zlav requested a review from skilly-lily June 21, 2021 23:49
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.

LGTM, a few leftover nits, but this is ready to go.

src/App/Fossa/ArchiveUploader.hs Outdated Show resolved Hide resolved
src/App/Fossa/ArchiveUploader.hs Outdated Show resolved Hide resolved
src/App/Fossa/ArchiveUploader.hs Outdated Show resolved Hide resolved
src/App/Fossa/ArchiveUploader.hs Outdated Show resolved Hide resolved
@zlav zlav merged commit 714b35c into master Jun 22, 2021
@zlav zlav deleted the feat/license-scan-deps branch June 22, 2021 18:59
This pull request was closed.
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.

3 participants