Skip to content

Commit

Permalink
Don't overwrite hooks without --force
Browse files Browse the repository at this point in the history
Currently, `bark install` silently overwrites any
`reference-transaction` hook that may be present. This is a
potentially very destructive action, so we should take proper care
before doing so.

I don't think there's a good way to automatically resolve the conflict
in case the file already exists, so this implementation simply lists
the conflicting files and leaves it to the user to resolve the
conflict manually. (For example, we could move both hook files into
`hooks/reference-transaction.d/` and make
`hooks/reference-transaction` a script that calls all scripts in
`reference-transaction.d/`, but it's not obvious that things like
order of execution wouldn't matter.)
  • Loading branch information
emlun committed Jan 24, 2024
1 parent 0c53530 commit dd3f4e3
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 9 deletions.
12 changes: 10 additions & 2 deletions gitbark/cli/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,15 @@ def protect(ctx):

@cli.command()
@click.pass_context
def install(ctx):
@click.option(
"-f",
"--force",
is_flag=True,
show_default=True,
default=False,
help="Overwrite existing hooks.",
)
def install(ctx, force):
"""
Install GitBark modules in repo.
Expand All @@ -193,7 +201,7 @@ def install(ctx):
ensure_bootstrap_verified(project)

try:
install_cmd(project)
install_cmd(project, force)
logger.info("Hooks installed successfully")
except RuleViolation as e:
pp_violation(e)
Expand Down
25 changes: 18 additions & 7 deletions gitbark/commands/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.

from ..cli.util import CliFail
from ..project import Project

import pkg_resources
Expand All @@ -22,12 +23,22 @@
logger = logging.getLogger(__name__)


def install(project: Project) -> None:
def install(project: Project, force: bool) -> None:
"""
Installs GitBark
"""
if not hooks_installed(project):
install_hooks(project)
hooks_missing = hooks_not_installed(project)
if hooks_missing:
hooks_conflicting = [hook_path for hook_path in hooks_missing if os.path.exists(hook_path)]
if force or not hooks_conflicting:
install_hooks(project)

else:
conflict_list = '\n'.join(f'{hook_path}' for hook_path in hooks_conflicting)
raise CliFail(
'Hooks already exist:\n\n'
f'{conflict_list}\n\n'
'Please delete them or re-run this command with the --force flag.')


def install_hooks(project: Project):
Expand All @@ -54,7 +65,7 @@ def make_executable(path: str):
os.chmod(path, new_permissions)


def hooks_installed(project: Project):
def hooks_not_installed(project: Project) -> list[str]:
reference_transaction_data = pkg_resources.resource_string(
__name__, "hooks/reference_transaction"
).decode()
Expand All @@ -63,10 +74,10 @@ def hooks_installed(project: Project):
reference_transaction_path = f"{hooks_path}/reference-transaction"

if not os.path.exists(reference_transaction_path):
return False
return [reference_transaction_path]

with open(reference_transaction_path, "r") as f:
if not f.read() == reference_transaction_data:
return False
return [reference_transaction_path]

return True
return []

0 comments on commit dd3f4e3

Please sign in to comment.