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

feat(python): support python requirements.txt #32

Merged

Conversation

Sam-Lane
Copy link
Contributor

Raising to help support this ticket in Trivy.

aquasecurity/trivy#492

Thank you.

@@ -0,0 +1,8 @@
click==8.0.0
Flask==2.0.0
itsdangerous==2.0.0
Copy link
Collaborator

@masahiro331 masahiro331 May 18, 2021

Choose a reason for hiding this comment

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

Thank you for Pull Request.

https://pip.pypa.io/en/latest/cli/pip_install/#requirements-file-format
The following patterns should need to be supported.

itsdangerous==2.0.0 # a comment 
# itsdangerous==2.0.0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added comment stripping and test coverage for this. Let me know if further test cases are required.

pkg/python/parse.go Outdated Show resolved Hide resolved
pkg/python/parse_test.go Outdated Show resolved Hide resolved
Comment on lines 22 to 23
file: "testdata/requirements_flask.txt",
want: requirementsFlask,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add more tests. requirements.txt may include requirements without version specifiers, other requirements files, etc.
https://pip.pypa.io/en/latest/cli/pip_install/#requirements-file-format

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also, the version may have spaces after == as follows.

docopt == 0.6.1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added whitespace stripping and test coverage for this. Let me know if more is required.

Added additional test coverage
@masahiro331
Copy link
Collaborator

@Sam-Lane
Sorry late response, Looks good.

Feature

Could you support this specification?
https://pip.pypa.io/en/latest/cli/pip_install/#requirement-specifiers
https://www.python.org/dev/peps/pep-0508/#id16

No support for comparison operators other than "==" is required.

Test

And add test more format. e.g

keyring >= 4.1.1            # Minimum version 4.1.1
coverage != 3.5             # Version Exclusion. Anything except version 3.5
Mopidy-Dirble ~= 1.1        # Compatible release. Same as >= 1.1, == 1.*

As the operator other that == suggests range of versions, these will be ignored
@ankk13 ankk13 force-pushed the parse-python-requirements-dot-txt branch from 50456ac to 8a0bcb5 Compare July 8, 2021 11:33
@@ -34,8 +36,8 @@ func Parse(r io.Reader) ([]types.Library, error) {
return libs, nil
}

func stripComments(line *string) {
if pos := strings.IndexAny(*line, commentRune); pos >= 0 {
func RstripByKey(line *string, key string) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do you need to export this function? And I prefer returning updated text like strings.TrimPrefix.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, it was a mistake

@ankk13 ankk13 force-pushed the parse-python-requirements-dot-txt branch from 6b7a617 to 4949673 Compare July 8, 2021 12:20
@ankk13 ankk13 force-pushed the parse-python-requirements-dot-txt branch from 4949673 to 8f18007 Compare July 8, 2021 15:29
@knqyf263 knqyf263 changed the title Update to support python requirements.txt feat(python): support python requirements.txt Jul 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants