Skip to content

fix(wip): Access .pre-commit-hooks.yaml as artifact #740

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

Merged
MaxymVlasov merged 4 commits into from
Dec 31, 2024
Merged

fix(wip): Access .pre-commit-hooks.yaml as artifact #740

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
51cd1a0
 Access `.pre-commit-hooks.yaml` as artifact
webknjaz Dec 31, 2024
f5157cd
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Dec 31, 2024
38476a3
Merge branch 'rewrite_hooks_on_python' into bugfixes/hooks-cfg-packag…
MaxymVlasov Dec 31, 2024
7403b66
Merge branch 'rewrite_hooks_on_python' into bugfixes/hooks-cfg-packag…
MaxymVlasov Dec 31, 2024
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
4 changes: 4 additions & 0 deletions hatch.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
[build.targets.sdist]
include = [
'.pre-commit-hooks.yaml',
'src/',
]

Expand All @@ -8,6 +9,9 @@ packages = [
'src/pre_commit_terraform/',
]

[build.targets.wheel.force-include]
'.pre-commit-hooks.yaml' = 'pre_commit_terraform/_artifacts/.pre-commit-hooks.yaml'

@yermulnik yermulnik Dec 31, 2024 •
edited
Loading

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'm a way too unfamiliar with all this Python magic, though I'm looking into how we can skip the non-obvious step:

The only possibly unobvious quirk is that during development, whenever this file changes in the Git repository, the pip install command must be executed again since this is what refreshes the file contents in the installed location.

Can this be remediated by e.g. symlinking the /.pre-commit-hooks.yaml into /pre_commit_terraform/_artifacts/.pre-commit-hooks.yaml?
Else we need to either add a GHA to get the command run automatically (and changed pushed to branch) or drop a comment into /.pre-commit-hooks.yaml file about a need to run a command once the file is changed. WDYT?

@webknjaz webknjaz Dec 31, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't think this is necessary. The GHA bit would be a part of the test infra PR that I didn't get to yet. The local quirk is already mentioned in the readme. And the pre-commit invocations do this under the hood, we don't have any control over it, this is always done.

@webknjaz webknjaz Dec 31, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Also note that this is a PR against a non-master branch.

@webknjaz webknjaz Dec 31, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Symlinking won't work because those are turned into regular files during packaging and aren't preserved outside the repo.

@yermulnik yermulnik Dec 31, 2024

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.

Great. Thanks for details. That all makes sense. Especially the "the pre-commit invocations do this under the hood, we don't have any control over it, this is always done" bit which just discards my inquiry as the automation is kinda there already 👍🏻

@webknjaz webknjaz Dec 31, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah.. To expand on this, basically, pre-commit uses pip install as a method to source the dependencies of your hook. As a side effect, it also installs it as a regular Python package distribution. To install a regular folder or a Git repo this way, a dist should first be built. That dist is a wheel (.whl: https://packaging.python.org/en/latest/glossary/#term-Wheel) — a zip file with your project in it's “built” state; it's essentially a zip archive that gets unpacked into site-packages/ (plus some metadata / deps handling on install).

When you what to test your app locally w/o pre-commit, you can reproduce a similar thing via pip install . and also pip install pytest when you get to using pytest for testing. However, during development you usually use editable mode pip install -e . which allows changing files in your Git repo without reinstalling (mostly) — it also builds wheels under the hood, but those wheels are special and almost don't contain any payload, short of the metadata and the artifacts. This is one of those artifacts that gets into these wheels and then is copied into site-packages/.


[metadata.hooks.vcs.urls]
'Source Archive' = 'https://github.com/antonbabenko/pre-commit-terraform/archive/{commit_hash}.tar.gz'
'GitHub: repo' = 'https://github.com/antonbabenko/pre-commit-terraform'
Expand Down
17 changes: 10 additions & 7 deletions src/pre_commit_terraform/common.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
import shutil
import subprocess
from collections.abc import Sequence
from pathlib import Path
from importlib.resources import files as access_artifacts_of
from typing import Callable

import yaml
Expand Down Expand Up @@ -305,15 +305,18 @@ def is_hook_run_on_whole_repo(hook_id: str, file_paths: list[str]) -> bool:
"""
logger.debug('Hook ID: %s', hook_id)

# Get the directory containing `.pre-commit-hooks.yaml` file
git_repo_root = Path(__file__).resolve().parents[5]
hook_config_path = os.path.join(git_repo_root, '.pre-commit-hooks.yaml')
# Get the directory containing the packaged `.pre-commit-hooks.yaml` copy
artifacts_root_path = access_artifacts_of('pre_commit_terraform') / '_artifacts'
pre_commit_hooks_yaml_path = artifacts_root_path / '.pre-commit-hooks.yaml'
pre_commit_hooks_yaml_path.read_text(encoding='utf-8')

logger.debug('Hook config path: %s', hook_config_path)
logger.debug('Hook config path: %s', pre_commit_hooks_yaml_path)

# Read the .pre-commit-hooks.yaml file
with open(hook_config_path, 'r', encoding='utf-8') as pre_commit_hooks_yaml:
hooks_config = yaml.safe_load(pre_commit_hooks_yaml)
pre_commit_hooks_yaml_txt = pre_commit_hooks_yaml_path.read_text(
encoding='utf-8',
)
hooks_config = yaml.safe_load(pre_commit_hooks_yaml_txt)

# Get the included and excluded file patterns for the given hook_id
for hook in hooks_config:
Expand Down