Probe grep capability instead of matching for GNU grep - #538
Open
arubdesu wants to merge 1 commit into
Open
Conversation
check_dependencies matched grep --version against the literal string "GNU grep", which stock macOS never reports: it identifies as "grep (BSD grep, GNU compatible)". tfenv install therefore aborted on a Mac without it from ggrep or an install from e.g. Homebrew or nix. Probe instead for the two behaviors tfenv uses beyond POSIX: \+ as a repetition operator in a BRE, per the latest regex in lib/tfenv-version-name.sh, and -o reporting every match on a line, per libexec/tfenv-list-remote. The probe also gates the ggrep branch, which previously returned on command -v ggrep alone, and now runs on all platforms rather than only Darwin. Add test/test_dependencies.sh covering the system grep, a stub whose -o truncates, a nonexistent binary, and check_dependencies end to end.
arubdesu
marked this pull request as ready for review
August 24, 2026 00:42
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
tfenv installcurrently cannot run on a macOS machine that has neither Homebrew nor nix/etcetera-installed grep, becausecheck_dependenciesin lib/helpers.sh tests for GNU grep by matchinggrep --versionagainst the literal stringGNU grep, so on Darwin the check fails and the install aborts. Stock macOS answer:This blocking/error state is only reachable on a 'manual' or git install -
test/install_deps.shrunsbrew install grepon Darwin, so CI always hasggrepand would not exercise the failing path. That is also why I hit it and most users won't: I avoid brew and installed tfenv by cloning the repo. (I understand a golang rewrite is underway, which is awesome/exciting!)Workaround
I wanted to spell out that I may very well be a relative corner case installation-wise, and before submitting this I did just workaround it: symlinking the system grep as
ggrepon my shell's PATH satisfies the existing check, because helpers.sh acceptsggrepon presence alone. I don't want to influence the use of the core maintainer teams time, your judgement call on accepting this vs. having me use this trivial stopgap would be totally understood either way.I opened this anyway for two reasons. The workaround asks every person who hits this to first work out why an install failed, and BSD grep already does what tfenv needs, so the requirement looked removable rather than something to route around, and in general lowers cognitive load for users. Worth noting the symlink keeps working after this change: it now passes on merit via the probe rather than on its name, so if anyone was like me but not mentioning it, nobody's existing workaround breaks.
Also probably a good time to mention I wrote this with the help of a clanker/coding agent, but every line of code of this description and the PR itself was reviewed by human me before submitting. Pardon I submitted as a draft since this is the first time you're seeing code submitted by me, I was a bit timid about targeting
masterright off the bat 😅Fix overview
This PR replaces the vendor-string test with a probe of the two grep behaviors tfenv actually relies on beyond POSIX:
\+as a repetition operator in a basic expression, used by thelatestregex built inlib/tfenv-version-name.sh-oreporting every match on a line rather than only the first, used to extract versions inlibexec/tfenv-list-remoteAll other grep invocations I found in
lib/andlibexec/use POSIX-guaranteed options (-e -v -q -F -E -Hn -h --). BSD grep implements both of the outliers above, which is what its "GNU compatible" claim refers to, so the "GNU grep" check rejects a working version.I want to be clear this is not proposing a return to installing ggrep automatically. The 3.0.0 note on that ("it's just not good practice") remains right to me. The argument here is narrower: the prerequisite itself appears unnecessary, because the grep already present can do the job.
Consistency with existing behavior
libexec/tfenv-installalready handles the same BSD/GNU split for a different tool by probing capability rather than identity in the case of mktemp:That difference is adapted to, whereas the grep difference was fatal.
Two secondary fixes in the same function
ggrepbranch previously returned oncommand -v ggrepalone. It is now gated by the same probe, so the check is stricter on that path than it was.if [[ "$(uname)" == 'Darwin' ]]. A minimal POSIX grep on Linux breaks the same\+regex and was never checked. The probe is cheap, so it now runs everywhere. I am happy to restore the Darwin gate if you would rather keep this PR's scope smaller.CHANGELOG
The clanker proposed I actually submit an entry for an 'UNRELEASED' version to CHANGELOG.md with the following text, which I'd not really seen before, I'll just include it on the off chance you BOTH accept and like it enough to just paste it in 😅:
Issue Link
When it comes to this issue itself, I didn't find an exact existing match, happy to open one independently if that's preferable or helpful.
For related issues, #442 removed the brew assumption but kept the GNU requirement. #488 cites "macOS needs
ggrep" as motivation for the Go rewrite.Testing
./test/run.shlocally and all tests passPlatform tested: macOS 27.0 beta (build 26A5416b), arm64, stock
/usr/bin/grep2.6.0-FreeBSDFull suite: 16 suites, 0 failures. I ran it with
ggrepremoved fromPATH, socheck_dependenciesfell through to the system grep on every invocation, which is the exact condition that aborts on unpatched tfenv. On CI, whereinstall_deps.shputsggrepon Darwin runners, theggrepbranch is what gets exercised instead; both paths are now probed identically.The run referenced above was functional rather than mocked: it downloaded real Terraform releases from
releases.hashicorp.com, verified SHA256 hashes, and extracted them, so the install, verification and extraction paths were all exercised end to end. It also incidentally confirms themktemp --tmpdirprobe cited above behaves correctly on this platform.test/test_dependencies.shis new and covers four cases:-oreturns only the first match is rejected, so the probe is shown to have teeth rather than passing everythingcheck_dependenciessucceeds end to endBehavior before and after on this machine:
Nothing here is specific to macOS 27: the grep version string is unchanged from earlier releases, so the bug and the fix are not version-dependent. That is simply where it was verified.
Quality Checklist
The GNU grep requirement was never documented in the README, only in the error message, which this updates. The CHANGELOG is left to you, as per the note above.
The claims about which grep options tfenv uses, the BSD grep behavior, and the before/after results above are all things I verified on my own machine rather than taking on faith.
Happy to adjust the target branch, approach/scope/wording, and/or split the Darwin-gate removal out if that makes review easier. Thank you for your consideration!