Skip to content

install() overwrites existing hooks despite README claiming idempotent behavior #2

@jgeluk

Description

@jgeluk

Problem

The README states:

The install() call is idempotent. It won't modify the existing setup and hooks.

However, the implementation in src/lib.rs uses fs::write() directly without checking if files already exist:

fs::write(
    self._hooks_abs_dir(repo_path.as_str()).join("pre-commit"),
    r#"#!/bin/sh
. "$(dirname "$0")/_/sloughi.sh"

echo "Running Sloughi pre-commit..."
"#,
)?;

fs::write() truncates and overwrites existing files, meaning any custom pre-commit hook content is lost on every cargo build.

Expected Behavior

If a hook file already exists, install() should either:

  1. Skip writing to that file entirely (true idempotent behavior)
  2. Only update the _/sloughi.sh helper while preserving user hooks
  3. Provide an option to force overwrite vs preserve existing

Reproduction

  1. Add sloughi to build-dependencies with .custom_path(".githooks")
  2. Create a custom .githooks/pre-commit with additional logic (fmt, clippy, etc.)
  3. Run cargo build
  4. Observe that .githooks/pre-commit is overwritten with the minimal template

Suggested Fix

Check if the file exists before writing:

let pre_commit_path = self._hooks_abs_dir(repo_path.as_str()).join("pre-commit");
if !pre_commit_path.exists() {
    fs::write(pre_commit_path, r#"#!/bin/sh..."#)?;
}

Or use OpenOptions with create_new(true) to fail if the file exists.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions