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

Static defaultExclusions rules are not congruent #54

Conversation

brandonc
Copy link
Contributor

@brandonc brandonc commented Jan 30, 2024

The default rule set did not reflect the necessary "negationsAfter" state for rules that appear before the default negation rule.

This caused Pack to skip the entire .terraform/modules directory when it was encountered because it believed the previous rule about .terraform/ to be dominant.

Any time a .terraformignore file containing a negation was evaluated, these default rules would be corrected by the parsing algorithm. This is why this bug was not caught by the tests. So I added another Pack test that relies only on the default rules.

In the second commit, moving the .git/ exclusion rule to the end of the default list, makes it possible to skip the entire .git/ directory walk because no exclusions appear after it. This should slightly speed up packing performance when the default rules are used or when no additional exclusion rules are set by .terraformignore.

The field dirs on each rule was not used so I removed it entirely.

The default rule set did not reflect the necessary "negationsAfter"
state for rules that appear before the default negation rule.

This caused Pack to skip the entire ".terraform/modules" directory
when it was encountered because it believed the rule to be dominant.

Any time a .terraformignore file containing a negation was evaluated,
these default rules would be corrected by the parsing algorithm. This
is why this bug was not caught by the tests. So I added another Pack
test that relies only on the default rules.
By moving the .git/ exclusion rule to the end of the default list,
it's possible to skip the entire .git/ directory walk because no
exclusions appear after it. This should slightly speed up packing
performance when the default rules are used or when no additional
exclusion rules are set by .terraformignore.
Copy link
Contributor

@alisdair alisdair left a comment

Choose a reason for hiding this comment

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

Nice! Change looks good, and I verified that a build of Terraform using this fixed the issue for me.

@brandonc brandonc merged commit 0480d9e into main Jan 30, 2024
9 checks passed
@brandonc brandonc deleted the IPL-5717-github-is-not-able-to-download-terraform-module-as-of-v-1-7-0 branch January 30, 2024 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants