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

fix: allow relative path #32

Closed
wants to merge 3 commits into from
Closed

fix: allow relative path #32

wants to merge 3 commits into from

Conversation

Kirill89
Copy link
Contributor

  • Ready for review
  • Follows CONTRIBUTING rules

What does this PR do?

Resolve root of go project according to target file.

Where should the reviewer start?

How should this be manually tested?

Any background context you want to provide?

We had discussion with @michael-go about this PR. Currently plugin implemented to resolve tree according to current project folder. If you run plugin outside of project it fails.

We have two options:

  • not to merge this PR and do not allow user to run snyk test --file=path/to/Gopkg.lock
  • to assume that we always resolve tree in the project root (and lost ability to check specific folder of the project)

@michael-go please add more details if you have time.

What are the relevant tickets?

https://snyksec.atlassian.net/browse/SC-6325

Screenshots

Additional questions

@CLAassistant
Copy link

CLAassistant commented Oct 21, 2018

CLA assistant check
All committers have signed the CLA.

@michael-go
Copy link
Contributor

michael-go commented Oct 23, 2018

My opinion on this is that the CLI plugins’ inspect() interface accepts two parameters root (cwd) & targetFile which allows a level of flexibility that this PR removes, and basically assumes that cwd == path.dirname(path.join(root, targetFile)) - and thus one parameter would have been enough . For some package-managers this flexibility doesn’t really matter, but for some, like Go it is sometimes useful. e.g. In Go it’s quite common to have several “sub-projects” share the same repository and the same lockfile - like server, a client, and a shared lib in the same repo - A user might like to scan the server separately from the client. It is possible today (by running snyk test server —file=../Gopkg.lock & snyk test client —file=../Gopkg, but won’t be possible after this PR as it will always scan both (which is still possible today by just running snyk test in the root of the project).
In addition Go is very sensitive to where you run things from due to the quirky $GOPATH.
So, I don’t think we should make this change unless there is a really good reason to, and making snyk test [root] —file=[targetFile] be always the same as snyk test —file=[root]/[targetFile] is not a good enough reason in my opinion, and especially for Go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants