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

Make scanner respect .gitignore files #191

Merged
merged 8 commits into from
Feb 8, 2023

Conversation

michaelkedar
Copy link
Member

@michaelkedar michaelkedar commented Feb 7, 2023

Closes #165

Also changes GetCommitSHA to no longer rely on the system's git executable.

By default, OSV-Scanner will skip files/directories that are ignored by a git project's .gitignore files. Added a flag --no-ignore to disable this behaviour.

cmd/osv-scanner/main.go Outdated Show resolved Hide resolved
@michaelkedar
Copy link
Member Author

michaelkedar commented Feb 7, 2023

Hmm, because the test files that should be ignored are actually in the repo, git doesn't actually recognize them as ignored files and the test doesn't work.

fixed.

Copy link
Collaborator

@oliverchang oliverchang left a comment

Choose a reason for hiding this comment

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

thanks for working on this!

Copy link
Collaborator

@another-rex another-rex left a comment

Choose a reason for hiding this comment

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

Thanks for switching the git os calls over to the library as well!

Do you think it's worth it to implement the subdir .gitignore scanning as well now? Or push it to a later PR. I think it does do the subdirectory ignores now after reading through it again. Is that right?

cmd/osv-scanner/main_test.go Show resolved Hide resolved
// We need to parse .gitignore files from the root of the git repo to correctly identify ignored files
// Defaults to current directory if dir is not in a repo or some other error
// TODO: Won't parse ignores if dir is not in a git repo, and is not under the current directory (e.g ../path/to)
// TODO: What is the desired behaviour for non git-repos?
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the current behavior of defaulting to current dir as the root is good.

Copy link
Member Author

Choose a reason for hiding this comment

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

Cool - removed that TODO

pkg/osvscanner/osvscanner.go Outdated Show resolved Hide resolved
@michaelkedar
Copy link
Member Author

I think it does do the subdirectory ignores now after reading through it again. Is that right?

Yep - gitignore.ReadPatterns evaluates subdirectories

@michaelkedar michaelkedar merged commit e4d2a02 into google:main Feb 8, 2023
Copy link
Collaborator

@oliverchang oliverchang left a comment

Choose a reason for hiding this comment

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

LGTM with one comment.

func parseGitIgnores(dir string) (*gitIgnoreMatcher, error) {
// We need to parse .gitignore files from the root of the git repo to correctly identify ignored files
// Defaults to current directory if dir is not in a repo or some other error
// TODO: Won't parse ignores if dir is not in a git repo, and is not under the current directory (e.g ../path/to)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we create a bug to track this? By "current directory", do you mean "current working directory"?

Copy link
Member Author

Choose a reason for hiding this comment

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

Made #202

cmaritan pushed a commit to cmaritan/osv-scanner that referenced this pull request Feb 12, 2023
Closes google#165 

Also changes `GetCommitSHA` to no longer rely on the system's git
executable.

By default, OSV-Scanner will skip files/directories that are ignored by
a git project's `.gitignore` files. Added a flag `--no-ignore` to
disable this behaviour.
hayleycd pushed a commit that referenced this pull request Mar 9, 2023
Closes #165 

Also changes `GetCommitSHA` to no longer rely on the system's git
executable.

By default, OSV-Scanner will skip files/directories that are ignored by
a git project's `.gitignore` files. Added a flag `--no-ignore` to
disable this behaviour.
julieqiu pushed a commit to julieqiu/osv-scanner that referenced this pull request May 2, 2023
Closes google#165 

Also changes `GetCommitSHA` to no longer rely on the system's git
executable.

By default, OSV-Scanner will skip files/directories that are ignored by
a git project's `.gitignore` files. Added a flag `--no-ignore` to
disable this behaviour.
julieqiu pushed a commit to julieqiu/osv-scanner that referenced this pull request May 2, 2023
Closes google#165 

Also changes `GetCommitSHA` to no longer rely on the system's git
executable.

By default, OSV-Scanner will skip files/directories that are ignored by
a git project's `.gitignore` files. Added a flag `--no-ignore` to
disable this behaviour.
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.

directory scanning should respect gitignore by default
3 participants