Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
32 changes: 32 additions & 0 deletions hooks/_common.sh
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,38 @@ function common::parse_cmdline {
done
}

#######################################################################
# Scrub GIT_* environment variables that get inherited from `git commit`
# and corrupt child `git clone` operations spawned by `tofu init` /
# `terraform init` when fetching modules.
#
# Without this, a `git commit` from a git worktree fails at tree-build:
# error: invalid object 100644 <oid> for '<path>'
# error: Error building trees
#
# Root cause: the child `git clone` inherits `GIT_INDEX_FILE` (pointing
# at the worktree's index) from the parent `git commit` env. Most
# clone-side env vars (`GIT_DIR`, `GIT_WORK_TREE`, `GIT_OBJECT_DIRECTORY`)
# are overridden by `git clone`'s own target-dir setup; only
# `GIT_INDEX_FILE` is not, so the clone writes the cloned module's blob
# OIDs into the parent worktree's index. The next `git commit` then
# cannot resolve those OIDs in the parent's object DB.
#
# pre-commit framework deliberately does NOT scrub GIT_* for user hooks
# (only its own internal git calls) per the maintainer's stance in
# pre-commit/pre-commit#1849: "they need the same code as in our
# no_git_env helper if they are dealing with doing git writes". The
# `no_git_env` helper (pre_commit/git.py:20-38) explicitly documents
# `GIT_INDEX_FILE: Causes 'error invalid object ...' during commit`
# and `GIT_DIR: Causes git clone to clone wrong thing` — we scrub both
# (and the two other documented offenders) defensively even though
# `GIT_INDEX_FILE` is the proximate cause of the bug class observed in
# pre-commit-terraform hooks.
#######################################################################
function common::scrub_git_env {
unset GIT_INDEX_FILE GIT_DIR GIT_WORK_TREE GIT_OBJECT_DIRECTORY
Comment thread
MaxymVlasov marked this conversation as resolved.
Outdated
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

#######################################################################
# Expand environment variables definition into their values in '--args'.
# Support expansion only for ${ENV_VAR} vars, not $ENV_VAR.
Expand Down
1 change: 1 addition & 0 deletions hooks/terraform_docs.sh
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ function main {
common::parse_cmdline "$@"
common::export_provided_env_vars "${ENV_VARS[@]}"
common::parse_and_export_env_vars
common::scrub_git_env
Comment thread
yermulnik marked this conversation as resolved.
Outdated

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I can't reproduce the worktree bug in this hook.

Suggested change
common::scrub_git_env

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

There another thing that shows, both for terraform_docs and terraform_fmt hooks.

But it's not an issue at all.

18:57 repro-hoo git:(repro-hoo +) 
➜ gc       
Terraform docs...........................................................Passed
Terraform validate with tflint...........................................Passed
Terraform fmt............................................................Failed
- hook id: terraform_fmt
- files were modified by this hook

main.tf


18:57 repro-hoo git:(repro-hoo !+) 
✘1 ➜ gc
Terraform docs...........................................................Passed
Terraform validate with tflint...........................................Passed
Terraform fmt............................................................Passed
Terraform docs.......................................(no files to check)Skipped
Terraform validate with tflint.......................(no files to check)Skipped
Terraform fmt........................................(no files to check)Skipped
[repro-hoo 44cd287] fmt
 2 files changed, 2 insertions(+), 6 deletions(-)

18:58 repro-hoo git:(repro-hoo) took 9s 
➜ git fsck --no-progress | head
dangling tree bba236455e65937967031c5d25b02e11f5c252f5

18:58 repro-hoo git:(repro-hoo) 
➜ git fsck --no-progress | head
dangling tree bba236455e65937967031c5d25b02e11f5c252f5

18:58 repro-hoo git:(repro-hoo) 
➜ gc                           
Terraform docs...........................................................Failed
- hook id: terraform_docs
- files were modified by this hook
Terraform validate with tflint...........................................Passed
Terraform fmt............................................................Passed

18:58 repro-hoo git:(repro-hoo !+) 
✘1 ➜ gc
Terraform docs...........................................................Passed
Terraform validate with tflint...........................................Passed
Terraform fmt............................................................Passed
Terraform docs.......................................(no files to check)Skipped
Terraform validate with tflint.......................(no files to check)Skipped
Terraform fmt........................................(no files to check)Skipped
[repro-hoo c115ce4] docs
 2 files changed, 2 insertions(+), 2 deletions(-)

18:58 repro-hoo git:(repro-hoo) took 6s 
➜ git fsck --no-progress | head
dangling tree a13f3782d35b8ce91a9d45f5b57acd390ea671a2
dangling tree bba236455e65937967031c5d25b02e11f5c252f5

# Support for setting relative PATH to .terraform-docs.yml config.
for i in "${!ARGS[@]}"; do
ARGS[i]=${ARGS[i]/--config=/--config=$(pwd)\/}
Expand Down
1 change: 1 addition & 0 deletions hooks/terraform_tflint.sh
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ function main {
common::parse_cmdline "$@"
common::export_provided_env_vars "${ENV_VARS[@]}"
common::parse_and_export_env_vars
common::scrub_git_env

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I can't reproduce the worktree bug in this hook.

Suggested change
common::scrub_git_env


# JFYI: tflint color already suppressed via PRE_COMMIT_COLOR=never

Expand Down
1 change: 1 addition & 0 deletions hooks/terraform_validate.sh
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ function main {
common::parse_cmdline "$@"
common::export_provided_env_vars "${ENV_VARS[@]}"
common::parse_and_export_env_vars
common::scrub_git_env

# Suppress terraform validate color
if [ "$PRE_COMMIT_COLOR" = "never" ]; then
Expand Down
Loading