detect-secrets icon indicating copy to clipboard operation
detect-secrets copied to clipboard

Output "The baseline file was updated." dialogue once

Open KevinHock opened this issue 6 years ago • 1 comments

We output the same thing over and over again

e.g. for one repo, running pre-commit run detect-secrets --all-files outputs:

Detect secrets...........................................................Failed
hookid: detect-secrets

Files were modified by this hook. Additional output:

The baseline file was updated.
Probably to keep line numbers of secrets up-to-date.
Please `git add .secrets.baseline`, thank you.


The baseline file was updated.
Probably to keep line numbers of secrets up-to-date.
Please `git add .secrets.baseline`, thank you.


Your baseline file (.secrets.baseline) is unstaged.
`git add .secrets.baseline` to fix this.
Your baseline file (.secrets.baseline) is unstaged.
`git add .secrets.baseline` to fix this.
Your baseline file (.secrets.baseline) is unstaged.
`git add .secrets.baseline` to fix this.
Your baseline file (.secrets.baseline) is unstaged.
`git add .secrets.baseline` to fix this.
The baseline file was updated.
Probably to keep line numbers of secrets up-to-date.
Please `git add .secrets.baseline`, thank you.


The baseline file was updated.
Probably to keep line numbers of secrets up-to-date.
Please `git add .secrets.baseline`, thank you.

We should try to do this just once.

KevinHock avatar Mar 25 '19 21:03 KevinHock

I'm pretty sure this is an artifact of the pre-commit engine, rather than our library. These are the reasons why I believe so:

1. We only output after all files are scanned

Source: https://github.com/Yelp/detect-secrets/blob/473923ea71f1ac2b5ea1eacc49b98f97967e3d05/detect_secrets/pre_commit_hook.py#L52

We only output this per invocation of the pre-commit hook, based on the list of filenames that are given to us. While we may be able to perform the logic to get the git diff --staged --name-only files, this would be essentially monkey-patching the pre-commit engine? It might work, seeing that we scan all files anyway, but it also seems like an anti-pattern.

2. Pre-commit performs xargs optimizations for parallel processing

Stack trace:

  • https://github.com/pre-commit/pre-commit/blob/b8300268bf2d8797f28edf9063d7e5659ad4d535/pre_commit/commands/run.py#L123-L125
  • https://github.com/pre-commit/pre-commit/blob/b8300268bf2d8797f28edf9063d7e5659ad4d535/pre_commit/repository.py#L100
  • https://github.com/pre-commit/pre-commit/blob/809b7482df7b739014cb583c0793f495d9a949d0/pre_commit/languages/python.py#L148
  • https://github.com/pre-commit/pre-commit/blob/9c6a1d80d6b94c86a1785a40a51389e83accac3e/pre_commit/languages/helpers.py#L90
  • https://github.com/pre-commit/pre-commit/blob/9c6a1d80d6b94c86a1785a40a51389e83accac3e/pre_commit/xargs.py#L89

Essentially, this suggests that pre-commit does invoke our library multiple times, hence printing multiple statements.

I'll leave this issue open for discussions around whether performing the git diff ourselves is worth it; otherwise, my recommendation is to close this as "Won't Do" for the above reasons.

domanchi avatar Apr 25 '19 00:04 domanchi

We're going to close this issue as it hasn't received any update in a very long time. Feel free to re-open it if you think it's still relevant.

lorenzodb1 avatar May 09 '24 17:05 lorenzodb1