project: abort a bit earlier before migrating .git/
Verify all the .git/ paths will be handled by the migration logic before
starting the migration. This way we still abort & log an error, but the
user gets to see it before we put the tree into a state that they have to
manually recover. Also add a few more known-safe-to-clobber paths.
Bug: https://crbug.com/gerrit/15273
Change-Id: If49d69b341bc960ddcafa30da333fb5ec7145b51
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/327557
Reviewed-by: Colin Cross <ccross@android.com>
Tested-by: Mike Frysinger <vapier@google.com>
diff --git a/project.py b/project.py
index de593c8..4828785 100644
--- a/project.py
+++ b/project.py
@@ -2818,24 +2818,40 @@
}
# Paths that we know will be in both, but are safe to clobber in .repo/projects/.
SAFE_TO_CLOBBER = {
- 'COMMIT_EDITMSG', 'FETCH_HEAD', 'HEAD', 'index', 'ORIG_HEAD',
+ 'COMMIT_EDITMSG', 'FETCH_HEAD', 'HEAD', 'gitk.cache', 'index', 'ORIG_HEAD',
}
+ # First see if we'd succeed before starting the migration.
+ unknown_paths = []
+ for name in platform_utils.listdir(dotgit):
+ # Ignore all temporary/backup names. These are common with vim & emacs.
+ if name.endswith('~') or (name[0] == '#' and name[-1] == '#'):
+ continue
+
+ dotgit_path = os.path.join(dotgit, name)
+ if name in KNOWN_LINKS:
+ if not platform_utils.islink(dotgit_path):
+ unknown_paths.append(f'{dotgit_path}: should be a symlink')
+ else:
+ gitdir_path = os.path.join(gitdir, name)
+ if name not in SAFE_TO_CLOBBER and os.path.exists(gitdir_path):
+ unknown_paths.append(f'{dotgit_path}: unknown file; please file a bug')
+ if unknown_paths:
+ raise GitError('Aborting migration: ' + '\n'.join(unknown_paths))
+
# Now walk the paths and sync the .git/ to .repo/projects/.
for name in platform_utils.listdir(dotgit):
dotgit_path = os.path.join(dotgit, name)
- if name in KNOWN_LINKS:
- if platform_utils.islink(dotgit_path):
- platform_utils.remove(dotgit_path)
- else:
- raise GitError(f'{dotgit_path}: should be a symlink')
+
+ # Ignore all temporary/backup names. These are common with vim & emacs.
+ if name.endswith('~') or (name[0] == '#' and name[-1] == '#'):
+ platform_utils.remove(dotgit_path)
+ elif name in KNOWN_LINKS:
+ platform_utils.remove(dotgit_path)
else:
gitdir_path = os.path.join(gitdir, name)
- if name in SAFE_TO_CLOBBER or not os.path.exists(gitdir_path):
- platform_utils.remove(gitdir_path, missing_ok=True)
- platform_utils.rename(dotgit_path, gitdir_path)
- else:
- raise GitError(f'{dotgit_path}: unknown file; please file a bug')
+ platform_utils.remove(gitdir_path, missing_ok=True)
+ platform_utils.rename(dotgit_path, gitdir_path)
# Now that the dir should be empty, clear it out, and symlink it over.
platform_utils.rmdir(dotgit)
diff --git a/tests/test_project.py b/tests/test_project.py
index d578fe8..4f44922 100644
--- a/tests/test_project.py
+++ b/tests/test_project.py
@@ -347,6 +347,10 @@
}
_FILES = {
'COMMIT_EDITMSG', 'FETCH_HEAD', 'HEAD', 'index', 'ORIG_HEAD',
+ 'unknown-file-should-be-migrated',
+ }
+ _CLEAN_FILES = {
+ 'a-vim-temp-file~', '#an-emacs-temp-file#',
}
@classmethod
@@ -365,10 +369,9 @@
dotgit.mkdir(parents=True)
for name in cls._SYMLINKS:
(dotgit / name).symlink_to(f'../../../.repo/projects/src/test.git/{name}')
- for name in cls._FILES:
+ for name in cls._FILES | cls._CLEAN_FILES:
(dotgit / name).write_text(name)
- subprocess.run(['tree', '-a', str(dotgit)])
yield tempdir
def test_standard(self):
@@ -385,3 +388,24 @@
gitdir = tempdir / '.repo/projects/src/test.git'
for name in self._FILES:
self.assertEqual(name, (gitdir / name).read_text())
+ # Make sure files were removed.
+ for name in self._CLEAN_FILES:
+ self.assertFalse((gitdir / name).exists())
+
+ def test_unknown(self):
+ """A checkout with unknown files should abort."""
+ with self._simple_layout() as tempdir:
+ dotgit = tempdir / 'src/test/.git'
+ (tempdir / '.repo/projects/src/test.git/random-file').write_text('one')
+ (dotgit / 'random-file').write_text('two')
+ with self.assertRaises(error.GitError):
+ project.Project._MigrateOldWorkTreeGitDir(str(dotgit))
+
+ # Make sure no content was actually changed.
+ self.assertTrue(dotgit.is_dir())
+ for name in self._FILES:
+ self.assertTrue((dotgit / name).is_file())
+ for name in self._CLEAN_FILES:
+ self.assertTrue((dotgit / name).is_file())
+ for name in self._SYMLINKS:
+ self.assertTrue((dotgit / name).is_symlink())