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.

Sign up for GitHub

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: command not found #251

Merged
merged 1 commit into from
Oct 15, 2021
Merged

fix: command not found #251

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion terraform_tflint.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ tflint_() {

# Print checked PATH **only** if TFLint have any messages
# shellcheck disable=SC2091 # Suppress error output
$(tflint "${ARGS[@]}" 2>&1) || {
$(tflint "${ARGS[@]}" 2>&1) 2> /dev/null || {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit lost 😕 How this differs from tflint "${ARGS[@]}" 2>/dev/null?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it evaluate $(…) and then redirects its stderr to /dev/null and this helps to properly fail the hook?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha...

You can try it by yourself. https://github.com/antonbabenko/pre-commit-terraform/blob/master/.github/CONTRIBUTING.md#run-and-debug-hooks-locally

No idea why it works as it works

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm-m, it works for me both ways. Skipping the $(tflint "${ARGS[@]}" 2>&1) 2> /dev/null || { item, here's what I get with tflint "${ARGS[@]}" 2>/dev/null 2>&1 || {:

> pre-commit --version
pre-commit 2.15.0

> bash --version | head -1
GNU bash, version 5.0.17(1)-release (x86_64-pc-linux-gnu)

> (cd ../pre-commit-terraform/ && git diff)
diff --git a/terraform_tflint.sh b/terraform_tflint.sh
index 52c5756..eeba641 100755
--- a/terraform_tflint.sh
+++ b/terraform_tflint.sh
@@ -65,7 +65,7 @@ tflint_() {

     # Print checked PATH **only** if TFLint have any messages
     # shellcheck disable=SC2091 # Suppress error output
-    $(tflint "${ARGS[@]}" 2>&1) 2> /dev/null || {
+    tflint "${ARGS[@]}" 2>/dev/null 2>&1 || {
       echo >&2 -e "\033[1;31m\nERROR in $path_uniq/:\033[0m"
       tflint "${ARGS[@]}"
     }

> pre-commit try-repo ../pre-commit-terraform/ terraform_tflint
[WARNING] Creating temporary repo with uncommitted changes...
===============================================================================
Using config:
===============================================================================
repos:
-   repo: /tmp/tmpbbz0832q/shadow-repo
    rev: a744926448de51cb9b17bd27ee6e645c30d75f50
    hooks:
    -   id: terraform_tflint
===============================================================================
[INFO] Initializing environment for /tmp/tmpbbz0832q/shadow-repo.
Terraform validate with tflint...........................................Failed
- hook id: terraform_tflint
- exit code: 2

1 issue(s) found:

Warning: variable "variable" is declared but not used (terraform_unused_declarations)

  on main.tf line 9:
   9: variable "variable" {

Reference: https://github.com/terraform-linters/tflint/blob/v0.32.1/docs/rules/terraform_unused_declarations.md


ERROR in ./:
1 issue(s) found:

Warning: variable "variable" is declared but not used (terraform_unused_declarations)

  on main.tf line 9:
   9: variable "variable" {

Reference: https://github.com/terraform-linters/tflint/blob/v0.32.1/docs/rules/terraform_unused_declarations.md

Also here's the other approach which eliminates second tflint exec:

> (cd ../pre-commit-terraform/ && git diff)
diff --git a/terraform_tflint.sh b/terraform_tflint.sh
index 52c5756..ce8443f 100755
--- a/terraform_tflint.sh
+++ b/terraform_tflint.sh
@@ -65,9 +65,9 @@ tflint_() {

     # Print checked PATH **only** if TFLint have any messages
     # shellcheck disable=SC2091 # Suppress error output
-    $(tflint "${ARGS[@]}" 2>&1) 2> /dev/null || {
-      echo >&2 -e "\033[1;31m\nERROR in $path_uniq/:\033[0m"
-      tflint "${ARGS[@]}"
+    TFLINT_ERROR=$(tflint "${ARGS[@]}" 2>&1) || {
+      echo >&2 -e "\033[1;31m\nERROR in $path_uniq/:\033[0m\n$TFLINT_ERROR"
+      return 1
     }

     popd > /dev/null

> pre-commit try-repo ../pre-commit-terraform/ terraform_tflint
[WARNING] Creating temporary repo with uncommitted changes...
===============================================================================
Using config:
===============================================================================
repos:
-   repo: /tmp/tmp2az_kuc_/shadow-repo
    rev: 16a6d9b4c397facf9f44eeb44f8beeec8c829a90
    hooks:
    -   id: terraform_tflint
===============================================================================
[INFO] Initializing environment for /tmp/tmp2az_kuc_/shadow-repo.
Terraform validate with tflint...........................................Failed
- hook id: terraform_tflint
- exit code: 1


ERROR in ./:
1 issue(s) found:

Warning: variable "variable" is declared but not used (terraform_unused_declarations)

  on main.tf line 9:
   9: variable "variable" {

Reference: https://github.com/terraform-linters/tflint/blob/v0.32.1/docs/rules/terraform_unused_declarations.md

Both fail as expected whereas second approach looks to be more optimized in my opinion.

Thanks for being patient and explaining all the caveats to me 🙇

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, the tflint "${ARGS[@]}" 2>/dev/null 2>&1 || { should obviously be tflint "${ARGS[@]}" >/dev/null 2>&1 || {

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

your tflint hook config

I meant pre-commit tflint hook config

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be keen to take a look into this bash thing but I don't have respective pro-commit configuration to mimic your setup.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

  - id: terraform_tflint
    args:
      - '--args=--config=__GIT_WORKING_DIR__/.tflint.hcl'

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's either I'm dumb with this or the time is late, but I can't get how do you configure tflint pre-commit hook to recurse into subdirs so that it throws errors like ERROR in environment/prd/: 😕
I most probably am missing something very simple, but — my bad — I don't get it 🤦

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pre-commit run -a
or --all

echo >&2 -e "\033[1;31m\nERROR in $path_uniq/:\033[0m"
tflint "${ARGS[@]}"
}
Expand Down