From 5818eeb9ddf945b4e397a7bfbd222a6cc6151299 Mon Sep 17 00:00:00 2001
From: Erik Flodin <erik@ejohansson.se>
Date: Mon, 4 Jan 2021 00:33:38 +0100
Subject: [PATCH] Change handling of submodules at upgrade

Start with doing "submodule absorbgitdirs" as otherwise "submodule
deinit" will fail if a module has been cloned first and later added as
a submodule (as it will then contain the .git dir instead of it being
under the superprojects .git dir).

Then try to deinit the submodules before moving the repo and abort the
upgrade if it fails for any submodule. Then do the move and finally
initialize the submodules that were initialized before the upgrade.

See #285
---
 test/test_unit_upgrade.py | 17 +++++++++++------
 yadm                      | 37 ++++++++++++++++++++++++++-----------
 2 files changed, 37 insertions(+), 17 deletions(-)

diff --git a/test/test_unit_upgrade.py b/test/test_unit_upgrade.py
index fa86e05..96119bf 100644
--- a/test/test_unit_upgrade.py
+++ b/test/test_unit_upgrade.py
@@ -59,12 +59,16 @@ def test_upgrade(tmpdir, runner, yadm, condition):
             yadm_legacy.join(path).write(path, ensure=True)
 
     mock_git = ""
-    if condition in ['tracked', 'submodules']:
+    if condition != 'no-paths':
         mock_git = f'''
             function git() {{
                 echo "$@"
-                if [[ "$*" == *.gitmodules* ]]; then
-                    return { '0' if condition == 'submodules' else '1' }
+                if [[ "$*" = *"submodule status" ]]; then
+                    { 'echo " 1234567 mymodule (1.0)"'
+                        if condition == 'submodules' else ':' }
+                fi
+                if [[ "$*" = *ls-files* ]]; then
+                    return { 1 if condition == 'untracked' else 0 }
                 fi
                 return 0
             }}
@@ -111,8 +115,9 @@ def test_upgrade(tmpdir, runner, yadm, condition):
             assert expected in run.out
             assert 'files tracked by yadm have been renamed' in run.out
             if condition == 'submodules':
-                assert 'submodule deinit -f .' in run.out
-                assert 'submodule update --init --recursive' in run.out
+                assert 'submodule deinit -- mymodule' in run.out
+                assert 'submodule update --init --recursive -- mymodule' \
+                    in run.out
             else:
-                assert 'submodule deinit -f .' not in run.out
+                assert 'submodule deinit' not in run.out
                 assert 'submodule update --init --recursive' not in run.out
diff --git a/yadm b/yadm
index 64640d4..1d68418 100755
--- a/yadm
+++ b/yadm
@@ -122,7 +122,7 @@ function main() {
           -d) # used by all commands
             DEBUG="YES"
           ;;
-          -f) # used by init() and clone()
+          -f) # used by init(), clone() and upgrade()
             FORCE="YES"
           ;;
           -l) # used by decrypt()
@@ -1296,7 +1296,7 @@ function perms() {
 function upgrade() {
 
   local actions_performed=0
-  local repo_moved=0
+  local -a submodules
   local repo_updates=0
 
   [[ -n "${YADM_OVERRIDE_REPO}${YADM_OVERRIDE_ARCHIVE}" || "$YADM_DATA" = "$YADM_DIR" ]] && \
@@ -1315,8 +1315,27 @@ function upgrade() {
       error_out "Unable to upgrade. '$YADM_REPO' already exists. Refusing to overwrite it."
     else
       actions_performed=1
-      repo_moved=1
       echo "Moving $LEGACY_REPO to $YADM_REPO"
+
+      export GIT_DIR="$LEGACY_REPO"
+
+      # Must absorb git dirs, otherwise deinit below will fail for modules that have
+      # been cloned first and then added as a submodule.
+      "$GIT_PROGRAM" submodule absorbgitdirs
+
+      while read -r sha submodule rest; do
+          if [[ "$sha" = -* ]]; then
+              continue
+          fi
+          "$GIT_PROGRAM" -C "$YADM_WORK" submodule deinit ${FORCE:+-f} -- "$submodule" || {
+              for other in "${submodules[@]}"; do
+                  "$GIT_PROGRAM" -C "$YADM_WORK" submodule update --init --recursive -- "$other"
+              done
+              error_out "Unable to upgrade. Could not deinit submodule $submodule"
+          }
+          submodules+=("$submodule")
+      done < <("$GIT_PROGRAM" -C "$YADM_WORK" submodule status)
+
       assert_parent "$YADM_REPO"
       mv "$LEGACY_REPO" "$YADM_REPO"
     fi
@@ -1366,13 +1385,9 @@ function upgrade() {
   done
 
   # handle submodules, which need to be reinitialized
-  if [ "$repo_moved" -ne 0 ]; then
-    cd_work "Upgrade submodules"
-    if "$GIT_PROGRAM" ls-files --error-unmatch .gitmodules &> /dev/null; then
-      "$GIT_PROGRAM" submodule deinit -f .
-      "$GIT_PROGRAM" submodule update --init --recursive
-    fi
-  fi
+  for submodule in "${submodules[@]}"; do
+      "$GIT_PROGRAM" -C "$YADM_WORK" submodule update --init --recursive -- "$submodule"
+  done
 
   [ "$actions_performed" -eq 0 ] && \
     echo "No legacy paths found. Upgrade is not necessary"
@@ -1610,7 +1625,7 @@ function issue_legacy_path_warning() {
 
   To remove this warning do one of the following:
     * Run "yadm upgrade" to move the yadm data to the new paths. (RECOMMENDED)
-    * Manually move yadm data to new default paths.
+    * Manually move yadm data to new default paths and reinit any submodules.
     * Specify your preferred paths with --yadm-data and --yadm-archive each execution.
 
   Legacy paths detected: