-
-
Notifications
You must be signed in to change notification settings - Fork 542
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
fix(terraform_docs
): Fix non-GNU sed
issues, introduced in v1.93.0
#704
Conversation
@cschroer @MaxymVlasov @yermulnik there is a much more easy solution, variant of option 1 (I'm using it for many years and it is proven to be portable) ... When you use "-i" in sed you must always add the backup suffix otherwise it would not work on Mac for example. Adding and empty suffix does the trick: # bad
infra albert$ sed -i "s/^ASDDFGFDSGHSDFG$/PIPPOPIPPO/" "MODULE-DETAILS.md"
sed: 1: "MODULE-DETAILS.md": invalid command code M
#good
infra albert$ sed -i "" "s/^ASDDFGFDSGHSDFG$/PIPPOPIPPO/" "MODULE-DETAILS.md"
infra albert$ This way no backup file is created, of course ;) |
terraform_docs
): Fix non-GNU sed
issues, introduced in v1.93.0
And I suppose we need same fix in pre-commit-terraform/README.md Lines 585 to 590 in ad5d40b
|
Ensure sed commands are compatible with MacOS and Linux/GNO version
I personally ran into some other issues with sed on MacOS, therefor I personally prefer using GNU sed on MacOS, too. |
Co-authored-by: George L. Yermulnik <[email protected]>
@cschroer yep, you're right pre-commit-terraform/README.md Lines 207 to 213 in 99fceb8
|
## [1.93.1](v1.93.0...v1.93.1) (2024-08-29) ### Bug Fixes * **`terraform_docs`:** Fix non-GNU `sed` issues, introduced in v1.93.0 ([#704](#704)) ([3c8734d](3c8734d))
This PR is included in version 1.93.1 🎉 |
Ensure compatibility of
terraform_docs
hook on MacOS and Linux by using GNU sed.Description of your changes
In PR #701, the usage of
sed
was introduced to theterraform_docs
for in-file changes, resulting in #703MacOS requires an extension for the
-i
parameter, unlike GNU sed.As a result the hook fails on MacOS know with the following error:
I see two potential solutions for this issue.
Always pass a extension to the
-i
parameter, as this is compatible with GNU sed, too. As a result, a backup file would be created for each sed commands operation and needs to be deleted again.Ensure we always use GNU sed. GNU sed is available via brew and can easily be installed on MacOS systems. Using GNU sed on both MacOS and Linux ensures consistent behaviour across operating systems.
I'm happy to change it to option 1, but wanted to show both options I see to fix the issue. Also I'm not 100% sure about Windows support for this hooks, but it seems like ( #648 ) they only work on Linux and MacOS.