From 66e509d2e4316ea589b4f3a85a511918c7013907 Mon Sep 17 00:00:00 2001
From: Erik Flodin <erik@flodin.me>
Date: Sun, 6 Apr 2025 08:58:40 +0200
Subject: [PATCH] Don't always run auto-alt (and -perms) after invoking git
 command

Improve performance (#505) by only running auto-alt (and auto-perms)
when HEAD has changed after calling a git command, e.g. after committing
or pulling new changes.

Also use this new information to remove stale symlinks when an alt file
has been removed.
---
 test/test_alt.py                      | 31 +++++++++++++-
 test/test_config.py                   |  4 +-
 test/test_perms.py                    | 10 +++--
 test/test_unit_report_invalid_alts.py |  2 +-
 yadm                                  | 61 ++++++++++++++++++---------
 5 files changed, 81 insertions(+), 27 deletions(-)

diff --git a/test/test_alt.py b/test/test_alt.py
index ed66bf4..c651736 100644
--- a/test/test_alt.py
+++ b/test/test_alt.py
@@ -248,7 +248,12 @@ def test_auto_alt(runner, yadm_cmd, paths, autoalt):
         os.system(" ".join(yadm_cmd("config", "yadm.auto-alt", autoalt)))
 
     utils.create_alt_files(paths, "##default")
-    run = runner(yadm_cmd("status"))
+
+    # Add and commit a file to trigger auto alt
+    paths.work.join("somefile").write("somedata")
+    runner(yadm_cmd("add", "somefile"), report=False)
+
+    run = runner(yadm_cmd("commit", "-m", "msg"))
     assert run.success
     assert run.err == ""
     linked = utils.parse_alt_output(run.out)
@@ -353,6 +358,26 @@ def test_stale_link_removal(runner, yadm_cmd, paths):
         assert str(source_file) not in linked
 
 
+@pytest.mark.usefixtures("ds1_copy")
+def test_deleted_file_link_removal(runner, yadm_cmd, paths):
+    """Link to deleted file is removed"""
+
+    utils.create_alt_files(paths, "##default")
+    run = runner(yadm_cmd("alt", "-d"))
+    assert run.success
+
+    # Remove one alt file
+    run = runner(yadm_cmd("rm", TEST_PATHS[0] + "##default"))
+    assert run.success
+    run = runner(yadm_cmd("commit", "-m", "remove"))
+    assert run.success
+
+    # And verify that the symlink has been removed
+    link_file = paths.work.join(TEST_PATHS[0])
+    with pytest.raises(OSError):
+        link_file.lstat()
+
+
 @pytest.mark.usefixtures("ds1_copy")
 def test_legacy_dir_link_removal(runner, yadm_cmd, paths):
     """Legacy link to alternative dir is removed
@@ -414,6 +439,8 @@ def test_template_overwrite_symlink(runner, yadm_cmd, paths, tst_sys):
     assert run.success
     assert run.err == ""
     assert run.out == ""
+    run = runner(yadm_cmd("commit", "-m", "msg"))
+    assert run.success
     assert not link.islink()
     assert target.read().strip() == "target"
     assert link.read().strip() == "test-data"
@@ -432,6 +459,8 @@ def test_ensure_alt_path(runner, paths, style):
     assert run.success
     assert run.err == ""
     assert run.out == ""
+    run = runner([paths.pgm, "-Y", yadm_dir, "--yadm-data", yadm_data, "commit", "-m", "msg"])
+    assert run.success
     assert paths.work.join(filename).read().strip() == "test-data"
 
 
diff --git a/test/test_config.py b/test/test_config.py
index c43d81c..b3aab02 100644
--- a/test/test_config.py
+++ b/test/test_config.py
@@ -32,12 +32,12 @@ def test_config_read_missing(runner, yadm_cmd):
     """Read missing attribute
 
     Display an empty value
-    Exit with 0
+    Exit with 1
     """
 
     run = runner(yadm_cmd("config", TEST_KEY))
 
-    assert run.success
+    assert not run.success
     assert run.err == ""
     assert run.out == ""
 
diff --git a/test/test_perms.py b/test/test_perms.py
index 1491c16..6170b38 100644
--- a/test/test_perms.py
+++ b/test/test_perms.py
@@ -35,13 +35,15 @@ def test_perms(runner, yadm_cmd, paths, ds1, autoperms):
     for private in privatepaths + insecurepaths:
         assert not oct(private.stat().mode).endswith("00"), "Path started secured"
 
-    cmd = "perms"
+    cmd = ["perms"]
     if autoperms != "notest":
-        cmd = "status"
-    run = runner(yadm_cmd(cmd), env={"HOME": paths.work})
+        run = runner(yadm_cmd("add", "efile1"), report=False)
+        assert run.success
+        cmd = ["-c", "user.name=Yadm", "commit", "-m", "msg"]
+    run = runner(yadm_cmd(*cmd), env={"HOME": paths.work})
     assert run.success
     assert run.err == ""
-    if cmd == "perms":
+    if autoperms == "notest":
         assert run.out == ""
 
     # these paths should be secured if processing perms
diff --git a/test/test_unit_report_invalid_alts.py b/test/test_unit_report_invalid_alts.py
index 3c7145f..98985d1 100644
--- a/test/test_unit_report_invalid_alts.py
+++ b/test/test_unit_report_invalid_alts.py
@@ -22,7 +22,7 @@ def test_report_invalid_alts(runner, yadm, valid, previous):
         report_invalid_alts
     """
     run = runner(command=["bash"], inp=script)
-    assert run.success
+    assert run.success == (valid and not previous)
     assert run.out == ""
     if not valid and not previous:
         assert "WARNING" in run.err
diff --git a/yadm b/yadm
index 9757345..a2cc146 100755
--- a/yadm
+++ b/yadm
@@ -74,6 +74,7 @@ USE_CYGPATH=0
 
 # flag when something may have changes (which prompts auto actions to be performed)
 CHANGES_POSSIBLE=0
+OLD_HEAD=""
 
 # flag when a bootstrap should be performed after cloning
 # 0: skip auto_bootstrap, 1: ask, 2: perform bootstrap, 3: prevent bootstrap
@@ -100,7 +101,6 @@ function main() {
   [ -d "$YADM_DATA" ] || mkdir -p "$YADM_DATA"
 
   # parse command line arguments
-  local retval=0
   internal_commands="^(alt|bootstrap|clean|clone|config|decrypt|encrypt|enter|git-crypt|help|--help|init|introspect|list|perms|transcrypt|upgrade|version|--version)$"
   if [ -z "$*" ]; then
     # no argumnts will result in help()
@@ -152,9 +152,10 @@ function main() {
     HOOK_COMMAND="$1"
     invoke_hook "pre"
     git_command "$@"
-    retval="$?"
   fi
 
+  local retval="$?"
+
   # process automatic events
   auto_alt
   auto_perms
@@ -573,6 +574,13 @@ function alt() {
     tracked_files+=("$tracked_file")
   done
 
+  if [ -n "$OLD_HEAD" ]; then
+    for deleted_file in \
+      $("$GIT_PROGRAM" diff --diff-filter=D --no-renames --name-only "$OLD_HEAD.." -- ':/*##*'); do
+      tracked_files+=("${deleted_file/\##/##~u.$local_user,}")
+    done
+  fi
+
   local alt_targets=()
   local alt_sources=()
   local alt_scores=()
@@ -603,8 +611,8 @@ function alt() {
       target="$target$suffix"
     fi
 
-    # Remove target if it's a symlink pointing at source
-    if [ -L "$target" ] && [ "$target" -ef "$source" ]; then
+    # Remove target if it's a symlink pointing at source or a dangling symlink
+    if [ -L "$target" ] && { [ "$target" -ef "$source" ] || [ ! -e "$target" ]; }; then
       rm -f "$target"
     fi
 
@@ -618,7 +626,7 @@ function alt() {
 }
 
 function report_invalid_alts() {
-  [ "$LEGACY_WARNING_ISSUED" = "1" ] && return
+  [ "$LEGACY_WARNING_ISSUED" = "1" ] && return 1
   [ "${#INVALID_ALT[@]}" = "0" ] && return
   local path_list=""
   local invalid
@@ -649,6 +657,8 @@ ${path_list}
 ***********
 EOF
   printf '%s\n' "$msg" >&2
+
+  return 1
 }
 
 function set_local_alt_values() {
@@ -909,12 +919,12 @@ EOF
 
     require_repo
 
+    CHANGES_POSSIBLE=1
+
     # operate on the yadm repo's configuration file
     # this is always local to the machine
     "$GIT_PROGRAM" config "$@"
 
-    CHANGES_POSSIBLE=1
-
   else
     # make sure parent folder of config file exists
     assert_parent "$YADM_CONFIG"
@@ -1120,17 +1130,21 @@ function enter() {
   GIT_WORK_TREE="$YADM_WORK"
   export GIT_WORK_TREE
 
+  OLD_HEAD="$("$GIT_PROGRAM" rev-parse HEAD 2>/dev/null)"
+
   [ "${#shell_cmd[@]}" -eq 0 ] && echo "Entering yadm repo"
 
   yadm_prompt="yadm shell ($YADM_REPO) $shell_path > "
   PROMPT="$yadm_prompt" PS1="$yadm_prompt" "$SHELL" "${shell_opts[@]}" "${shell_cmd[@]}"
-  return_code="$?"
+  local return_code="$?"
 
-  if [ "${#shell_cmd[@]}" -eq 0 ]; then
-    echo "Leaving yadm repo"
-  else
-    exit_with_hook "$return_code"
-  fi
+  [ "${#shell_cmd[@]}" -eq 0 ] && echo "Leaving yadm repo"
+
+  local new_head
+  new_head="$("$GIT_PROGRAM" rev-parse HEAD 2>/dev/null)"
+  [ "$OLD_HEAD" != "$new_head" ] && CHANGES_POSSIBLE=1
+
+  return "$return_code"
 }
 
 function git_command() {
@@ -1154,12 +1168,18 @@ function git_command() {
     fi
   fi
 
-  CHANGES_POSSIBLE=1
+  OLD_HEAD="$("$GIT_PROGRAM" rev-parse HEAD 2>/dev/null)"
 
   # pass commands through to git
   debug "Running git command $GIT_PROGRAM $*"
   "$GIT_PROGRAM" "$@"
-  return "$?"
+  local return_code="$?"
+
+  local new_head
+  new_head="$("$GIT_PROGRAM" rev-parse HEAD 2>/dev/null)"
+  [ "$OLD_HEAD" != "$new_head" ] && CHANGES_POSSIBLE=1
+
+  return "$return_code"
 }
 
 function help() {
@@ -1363,6 +1383,7 @@ function perms() {
   chmod -f go-rwx ${GLOBS[@]} &>/dev/null
   # TODO: detect and report changing permissions in a portable way
 
+  return 0
 }
 
 function upgrade() {
@@ -1871,8 +1892,8 @@ function invoke_hook() {
   exit_status="$2"
   hook_command="${YADM_HOOKS}/${mode}_$HOOK_COMMAND"
 
-  if [ -x "$hook_command" ] ||
-    { [[ $OPERATING_SYSTEM == MINGW* ]] && [ -f "$hook_command" ]; }; then
+  if [ -f "$hook_command" ] &&
+    { [ -x "$hook_command" ] || [[ $OPERATING_SYSTEM == MINGW* ]]; }; then
     debug "Invoking hook: $hook_command"
 
     # expose some internal data to all hooks
@@ -1886,6 +1907,7 @@ function invoke_hook() {
 
     # pack array to export it; filenames including a newline character (\n)
     # are NOT supported
+    parse_encrypt
     YADM_ENCRYPT_INCLUDE_FILES=$(join_string $'\n' "${ENCRYPT_INCLUDE_FILES[@]}")
 
     export YADM_HOOK_COMMAND
@@ -2102,10 +2124,11 @@ function auto_perms() {
 
 function auto_bootstrap() {
 
-  bootstrap_available || return
-
   [ "$DO_BOOTSTRAP" -eq 0 ] && return
   [ "$DO_BOOTSTRAP" -eq 3 ] && return
+
+  bootstrap_available || return 0
+
   [ "$DO_BOOTSTRAP" -eq 2 ] && bootstrap
   if [ "$DO_BOOTSTRAP" -eq 1 ]; then
     echo "Found $YADM_BOOTSTRAP"