Nico Huber has posted comments on this change. ( https://review.coreboot.org/22971 )
Change subject: git-hooks: Fix install script for various git versions ......................................................................
Patch Set 2:
Beside the problems with the install script, I run into more trouble lately while I moved patches from patchwork to gerrit. The whole approach with the in-tree wrapper.sh seems broken: You must never check a commit out before wrapper.sh was introduced. Otherwise not even your <hook>.local files are run. So, the lesson I've learned, the hooks must not rely on the state of the tree.
One way to mitigate that would be to copy wrapper.sh along with the flashrom hooks into .git/hooks/. We could still try to keep them up to date there (based on timestamps? not sure how much shell foo is necessary).
One problem will haunt us forever, though. Every time you run make for a commit between the introduction of wrapper.sh and a possible fix, it will overwrite existing hooks and <hook>.local files (un- conditionally). So we have to find a new name for <hook>.local and should probably keep wrapper.sh around forever to help until ano- ther run of (the updated) install script fixed things again.
An alternative: Drop all this shit. Use something less error-prone like `make gitconfig` in coreboot. Downside: After a `make` with a wrapper.sh revision broke things, you'd have to repair it manually, hopefully just run `make gitconfig` again. Upside: We don't look like fools.
-
Nico Huber (Code Review)