antonbabenko / pre-commit-terraform

pre-commit git hooks to take care of Terraform configurations 🇺🇦
MIT License
3.21k stars 538 forks source link

terraform_docs sed issue with v1.93.1 for mac default sed #707

Closed BeneHa closed 2 months ago

BeneHa commented 2 months ago

Describe the bug

Since the fix of a bug in https://github.com/antonbabenko/pre-commit-terraform/pull/704 by @cschroer , the empty sed -i flag does not work for me. Now, a README.md-e file is being created as well as the README.md. I am using Mac's default sed.

I know I can install GNU sed, but I think many users are using the default sed and will run into this issue.

How can we reproduce it?

Run pre-commit tf_docs on a Mac with the default sed installed.

Environment information

GNU bash, version 5.2.32(1)-release (aarch64-apple-darwin23.4.0)
pre-commit 3.8.0
zsh: command not found: tofu
Terraform v1.9.5
python SKIPPED
Python 3.12.4
checkov SKIPPED
infracost SKIPPED
terraform-docs version v0.18.0 228c7a7 darwin/arm64
terragrunt SKIPPED
terrascan SKIPPED
TFLint version 0.53.0
+ ruleset.terraform (0.9.1-bundled)
tfsec SKIPPED
trivy SKIPPED
tfupdate SKIPPED
hcledit SKIPPED
file content ```yaml repos: - repo: https://github.com/antonbabenko/pre-commit-terraform.git rev: v1.94.0 hooks: - id: terraform_docs args: - --hook-config=--create-file-if-not-exist=true ```
drAlberT commented 2 months ago

I confirm the issue ... the sed command does not raise any error (of course) but creates a new file. @MaxymVlasov @yermulnik this is why I was saying that sed -i'' -e (without a space before the '') is not working on Mac in #704.

Screenshot 2024-08-30 at 10 46 40

robinbowes commented 2 months ago

I can confirm this behaviour:

❯ ls
tt.txt

❯ gsed -i'' -e 's/foo/foobar/' tt.txt

❯ ls
tt.txt

❯ sed -i'' -e 's/foo/foobar/' tt.txt

❯ ls
tt.txt   tt.txt-e

Some useful information here about replacing strings in files: http://mywiki.wooledge.org/BashFAQ/021

tl;dr: "The preferred way to modify a file is to create a new file within the same file system, write the modified content into it, and then mv it to the original name."

MaxymVlasov commented 2 months ago

I think Apply should offer $9.99/month subscription for just replacing OSX with something like ElementaryOS with some integrations top :D

If serious, please let me know is next work on Mac or not

repos:
  - repo: https://github.com/antonbabenko/pre-commit-terraform
    rev: aee5dc9a8d6625a47396f2b4797368c57cbb5f49
    hooks:
      - id: terraform_docs
MaxymVlasov commented 2 months ago

Test this please

repos:
  - repo: https://github.com/antonbabenko/pre-commit-terraform
    rev: a76f03bfc4978741f37f79a6b4a86dc72078a78d
    hooks:
      - id: terraform_docs
drAlberT commented 2 months ago

Test this please

repos:
  - repo: https://github.com/antonbabenko/pre-commit-terraform
    rev: a76f03bfc4978741f37f79a6b4a86dc72078a78d
    hooks:
      - id: terraform_docs

Yes, it works

antonbabenko commented 2 months ago

This issue has been resolved in version 1.94.1 :tada: