Skip to content

Commit

Permalink
chore: updatehooks skips updated repos that aren't out of date by def…
Browse files Browse the repository at this point in the history
…ault (#575)

This PR adds a new argument to the updatehooks function named
force_update which is set to False by default. if force_update is false,
then we won't download/install the repo unless it is out of date. If
force_update is True, then we'd download and install the latest repo
version irrespective of the currently installed version.

There is currently no code (outside of unit tests) that's passing True
for the force_update argument.

## Changes
<!-- A detailed list of changes -->
*

## Testing
<!--
Mention updated tests and any manual testing performed.
Are aspects not yet tested or not easily testable?
Feel free to include screenshots if appropriate.
 -->
*

## Clean Code Checklist
<!-- This is here to support you. Some/most checkboxes may not apply to
your change -->
- [ ] Meets acceptance criteria for issue
- [ ] New logic is covered with automated tests
- [ ] Appropriate exception handling added
- [ ] Thoughtful logging included
- [ ] Documentation is updated
- [ ] Follow-up work is documented in TODOs
- [ ] TODOs have a ticket associated with them
- [ ] No commented-out code included


<!--
Github-flavored markdown reference:
https://docs.github.com/en/get-started/writing-on-github
-->

---------

Co-authored-by: Ian Bowden <ian.bowden@slalom>
  • Loading branch information
ian-bowden-slalom and Ian Bowden committed Jun 26, 2024
1 parent 84fef43 commit 88b3d99
Show file tree
Hide file tree
Showing 3 changed files with 229 additions and 25 deletions.
4 changes: 3 additions & 1 deletion secureli/modules/core/core_services/updater.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ def update_hooks(
bleeding_edge: bool = False,
freeze: bool = False,
repos: Optional[list] = None,
force_update: Optional[bool] = False,
):
"""
Updates the precommit hooks but executing precommit's autoupdate command. Additional info at
Expand All @@ -34,10 +35,11 @@ def update_hooks(
the latest tagged version (which is the default behavior)
:param freeze: Set to True to store "frozen" hashes in rev instead of tag names.
:param repos: Dectionary of repos to update. This is used to target specific repos instead of all repos.
:param force_update: set to True to download updates for hooks whose versions aren't out of date. False means only out-of-date repos are updated
:return: ExecuteResult, indicating success or failure.
"""
update_result = self.pre_commit.autoupdate_hooks(
folder_path, bleeding_edge, freeze, repos
folder_path, bleeding_edge, freeze, repos, force_update
)
output = update_result.output

Expand Down
45 changes: 44 additions & 1 deletion secureli/modules/shared/abstractions/pre_commit.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import datetime
from pathlib import Path
import shutil
from urllib.parse import urlparse
from git import Repo

# Note that this import is pulling from the pre-commit tool's internals.
# A cleaner approach would be to update pre-commit
Expand Down Expand Up @@ -189,8 +191,14 @@ def check_for_hook_updates(
"repo": repo_config.url
} # PreCommitSettings uses "url" instead of "repo", so we need to copy that value over
old_rev_info = HookRepoRevInfo.from_config(repo_config_dict)

# don't try and update the local repo
if old_rev_info.repo == "local":
continue

# if the revision currently specified in .pre-commit-config.yaml looks like a full git SHA
# (40-character hex string), then set freeze to True

freeze = (
bool(git_commit_sha_pattern.fullmatch(repo_config.rev))
if freeze is None
Expand All @@ -208,6 +216,7 @@ def autoupdate_hooks(
bleeding_edge: bool = False,
freeze: bool = False,
repos: Optional[list] = None,
force_update: Optional[bool] = False,
) -> ExecuteResult:
"""
Updates the precommit hooks by executing precommit's autoupdate command. Additional info at
Expand All @@ -217,8 +226,17 @@ def autoupdate_hooks(
the latest tagged version (which is the default behavior)
:param freeze: Set to True to store "frozen" hashes in rev instead of tag names.
:param repos: List of repos (url as a string) to update. This is used to target specific repos instead of all repos.
:param force_update: set to True to download updates for hooks whose versions aren't out of date. False means only out-of-date repos are updated
:return: ExecuteResult, indicating success or failure.
"""
if not force_update:
repos = self._get_outdated_repos(folder_path, bleeding_edge, freeze, repos)

# if there's no outdated repos and we're not forcing updates then there's nothing more to do
if not repos and not force_update:
output = "No changes necessary.\n"
return ExecuteResult(successful=True, output=output)

subprocess_args = [
"pre-commit",
"autoupdate",
Expand All @@ -240,7 +258,9 @@ def autoupdate_hooks(

for repo in repos:
if isinstance(repo, str):
arg = "--repo {}".format(repo)
arg = "--repo"
repo_args.append(arg)
arg = format(repo)
repo_args.append(arg)
else:
output = "Unable to update repo, string validation failed. Repo parameter should be a dictionary of strings."
Expand Down Expand Up @@ -390,3 +410,26 @@ def pre_commit_config_exists(self, folder_path: Path) -> bool:
"""
path_to_config = folder_path / ".pre-commit-config.yaml"
return path_to_config.exists()

def _get_outdated_repos(
self,
folder_path: Path,
bleeding_edge: bool = False,
freeze: bool = False,
repos: Optional[list] = None,
) -> list:
# if no repos are specified then use the pre commit config to get a list of all possible repos to update
if not repos:
precommit_config = self.get_pre_commit_config(folder_path)
outdated_repos = self.check_for_hook_updates(
precommit_config, not bleeding_edge, freeze
)
repos = [key for key in outdated_repos.keys()]
# Only check for updates for the specified repos
else:
outdated_repos = self.check_for_hook_updates(
PreCommitSettings(repos=repos), not bleeding_edge, freeze
)
repos = [key for key in outdated_repos.keys()]

return repos
Loading

0 comments on commit 88b3d99

Please sign in to comment.