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

Realpath error with 1.63.0 #336

Closed
oksimon95 opened this issue Feb 10, 2022 · 12 comments · Fixed by #368
Closed

Realpath error with 1.63.0 #336

oksimon95 opened this issue Feb 10, 2022 · 12 comments · Fixed by #368
Assignees
Labels
area/local_installation documentation Improvements or additions to documentation

Comments

@oksimon95
Copy link

oksimon95 commented Feb 10, 2022

The latest version of pre-commit terraform throws an error due to lack of realpath command. Are there any new packages that need to be locally installed for pre-commit terraform to run successfully?

Terraform validate.......................................................Failed
- hook id: terraform_validate
- exit code: 1

/Users/.../.cache/pre-commit/repoln3cyzan/hooks/terraform_validate.sh: line 9: realpath: command not found
/Users/.../.cache/pre-commit/repoln3cyzan/hooks/terraform_validate.sh: line 11: ./_common.sh: No such file or directory
@yermulnik
Copy link
Collaborator

@oksimon95 Out of interest: what's your OS name and version?

@oksimon95
Copy link
Author

macOS Monterey 12.1

@antonbabenko
Copy link
Owner

I think you can install coreutils to get realpath (brew info coreutils) but that should not be required for this project to work.

@oksimon95
Copy link
Author

Installing coreutils seems to have fixed the realpath error, but now I'm getting the same error as this other issue: #337

@antonbabenko
Copy link
Owner

realpath should not be required. There are workarounds like this - https://stackoverflow.com/a/18443300/550451

@yermulnik
Copy link
Collaborator

yermulnik commented Feb 10, 2022

That's coming from Bash 3. I don't think it's worth supporting so much ancient versions of tools. @oksimon95 you'd better upgrade to a modern Bash as I mentioned in #337. IMHO.

ps: yep, also what @antonbabenko suggested — you can add workaround local to your setup to implement realpath via Bash function.

@oksimon95
Copy link
Author

I updated my local version of bash to 5.1.16 and this seems to have fixed the error

@MaxymVlasov
Copy link
Collaborator

realpath should not be required. There are workarounds like this - stackoverflow.com/a/18443300/550451

It is already in dependencies and was added by you 4 month ago :)
https://github.com/antonbabenko/pre-commit-terraform/pull/267/files

@MaxymVlasov
Copy link
Collaborator

MaxymVlasov commented Feb 10, 2022

Also, I wouldn't like to add this workaround to EACH hook file.

@yermulnik
Copy link
Collaborator

yermulnik commented Feb 10, 2022

Also, I wouldn't like to add this workaround to EACH hook file.

First thing I thought of was _common.sh — isn't it sourced by each hook?
Though I'm not quite sure yet how do we workaround declare -g 🤔 (apart from just dropping use of declare -g).
Hence I'm leaning to drop support for this much ancient version of Bash, even if those on MacOS have to tackle installation of newer version. Which is quite easy-peasy thing in the modern MacOS world, whereas supporting Bash 3 would mean for us a need to drop support for modern Bash features 🤷🏻

@stanzheng
Copy link

the fixes in this are related to bash and coreutils

Im running into this issue on v1.64 on different platform.

I tried digging into the source of the error but im confused at where to start. I had pre-commit terraform fmt work correctly sometime so this was confusing when I ran into this issue.

My coworker has also run into this issue so it is reproducible.

#349

@MaxymVlasov MaxymVlasov added documentation Improvements or additions to documentation and removed bug Something isn't working labels Mar 15, 2022
@antonbabenko
Copy link
Owner

This issue has been resolved in version 1.68.0 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/local_installation documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants