jish / pre-commit

A slightly improved pre-commit hook for git
https://jish.github.io/pre-commit/
Other
796 stars 96 forks source link

[RFC] Make it work with Alpine Linux / BusyBox `grep` #265

Closed fgrehm closed 6 years ago

fgrehm commented 6 years ago

Hey @jish!

We use Docker for our development environments and they are all based on Alpine Linux.

Even though we can make it work by 💆‍♂️ ing configs in order to run pre commit inside our Docker containers with something along these lines:

git config pre-commit.ruby "/usr/local/bin/docker-compose -f /path/to/docker-compose.yml run --rm --no-deps $APP_NAME ruby"

We get a lot of noise in our pre commit checks. I tracked down the reason to the fact that we have a grep --version hardcoded here:

https://github.com/jish/pre-commit/blob/51f1223f335437685ee45f6735235e00eef3d709/lib/pre-commit/checks/grep.rb#L94

Unfortunately that's not a valid parameter for the grep shipped with Alpine 😢

$ docker run -ti --rm alpine grep --version
grep: unrecognized option: version
BusyBox v1.27.2 (2017-12-12 10:41:50 GMT) multi-call binary.

Usage: grep [-HhnlLoqvsriwFE] [-m N] [-A/B/C N] PATTERN/-e PATTERN.../-f FILE [FILE]...

Search for PATTERN in FILEs (or stdin)

        -H      Add 'filename:' prefix
        -h      Do not add 'filename:' prefix
        -n      Add 'line_no:' prefix
        -l      Show only names of files that match
        -L      Show only names of files that don't match
        -c      Show only count of matching lines
        -o      Show only the matching part of line
        -q      Quiet. Return 0 if PATTERN is found, 1 otherwise
        -v      Select non-matching lines
        -s      Suppress open and read errors
        -r      Recurse
        -i      Ignore case
        -w      Match whole words only
        -x      Match whole lines only
        -F      PATTERN is a literal (not regexp)
        -E      PATTERN is an extended regexp
        -m N    Match up to N times per file
        -A N    Print N lines of trailing context
        -B N    Print N lines of leading context
        -C N    Same as '-A N -B N'
        -e PTRN Pattern to match
        -f FILE Read pattern from file

Do you have any thoughts on the best way to make the gem compatible with alpine? I can try putting up a PR for that 😄

jish commented 6 years ago

"Do you have any thoughts on the best way to make the gem compatible with alpine?"

That's a good question. I believe we are only checking the version to see if grep supports the -E or the -P flag. We could probably keep the same check and just redirect output using the system command. That should solve your output problem.

As far as functionality goes, does the functionality behave as you would like? It looks like you want the -E option as well. 🤔

jish commented 6 years ago

Released v0.38.0 https://rubygems.org/gems/pre-commit

This should clear up the standard error output. Please file another issue if it is not behaving properly.

JOduMonT commented 6 years ago

must admit I'm not sure the legitimate of my hijacking this issue but it's the only seams related with seams to have an answer so here my question

grep option -P don't exist in alpine 3.8 which it use in a bash what was/is the solution ?

aka it is possible to import a static grep from alpine to alpine ?

by digging I found this echo "BusybBox grep detected, please install gnu grep, \"apk add --no-cache --upgrade grep\"" is'nt it the answer ?

Rohit-Hcs commented 4 years ago

must admit I'm not sure the legitimate of my hijacking this issue but it's the only seams related with seams to have an answer so here my question

grep option -P don't exist in alpine 3.8 which it use in a bash what was/is the solution ?

aka it is possible to import a static grep from alpine to alpine ?

by digging I found this echo "BusybBox grep detected, please install gnu grep, \"apk add --no-cache --upgrade grep\"" is'nt it the answer ?

Upgrading the grep using "apk add --no-cache --upgrade grep" has solved my problem with --exclude option in Alpine.