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

.git/hooks/pre-commit is far too slow #128

Closed
3 tasks done
regularfry opened this issue Sep 13, 2023 · 1 comment · Fixed by #132
Closed
3 tasks done

.git/hooks/pre-commit is far too slow #128

regularfry opened this issue Sep 13, 2023 · 1 comment · Fixed by #132
Labels
bug Something isn't working triage

Comments

@regularfry
Copy link
Contributor

Is there an existing issue for this?

  • I have searched the existing Issues

Current Behavior

make config installs a precommit hook to run a few tasks on commit. It's far too slow to be useful.

$ time .git/hooks/pre-commit
...
...
.git/hooks/pre-commit  1.35s user 0.93s system 2% cpu 1:48.39 total

Most of that time was spent in the scan-vulnerabilities.sh script.

Expected Behavior

git commit needs to be quick: if I'm not editing my commit message within a few seconds, I've probably forgotten the details of what I've just done. It's an enforced context switch. That prompts me to do commits less often, which means they're individually bigger. That creates knock-on problems.

If we want to run scan-vulnerabilities.sh on the development machine, I suspect the right place to do it is on pre-push, rather than pre-commit.

Steps To Reproduce

  1. Check out the template
  2. make config (doing whatever you need to get that working)
  3. time .git/hooks/pre-commit

Output

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct

Sensitive Information Declaration

  • I confirm that neither PII/PID nor sensitive data are included in this form
@regularfry regularfry added bug Something isn't working triage labels Sep 13, 2023
@stefaniuk
Copy link
Contributor

Grype updates its database at the first run or every 5 days. The size is approximately 1GB.

I think this pre-hook should be removed. There isn't much benefit in running a potentially outdated CVE check locally, especially considering that the GitHub Action always performs a check against the latest version.

github-merge-queue bot pushed a commit that referenced this issue Sep 21, 2023
## Description

There is not much benefit in running a potentially outdated CVE check
locally, especially considering that the GitHub Action always performs a
check against the latest version.

- Fixes #128 

## Context

Grype [updates its
database](https://github.com/anchore/grype#data-staleness) at the first
run or every 5 days. The size is approximately 1GB. It may take minutes
for the scan dependencies git hook to complete. This is not a good user
experience.

## Type of changes

- [x] Refactoring (non-breaking change)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would change existing
functionality)
- [x] Bug fix (non-breaking change which fixes an issue)

## Checklist

- [x] I am familiar with the [contributing
guidelines](../docs/CONTRIBUTING.md)
- [x] I have followed the code style of the project
- [ ] I have added tests to cover my changes
- [x] I have updated the documentation accordingly
- [ ] This PR is a result of pair or mob programming

---

## Sensitive Information Declaration

To ensure the utmost confidentiality and protect your and others
privacy, we kindly ask you to NOT including [PII (Personal Identifiable
Information) / PID (Personal Identifiable
Data)](https://digital.nhs.uk/data-and-information/keeping-data-safe-and-benefitting-the-public)
or any other sensitive data in this PR (Pull Request) and the codebase
changes. We will remove any PR that do contain any sensitive
information. We really appreciate your cooperation in this matter.

- [x] I confirm that neither PII/PID nor sensitive data are included in
this PR and the codebase changes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working triage
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants