bugfixes and test goes through for Global Install

- pre-existing local isntall now deleted when global install is
  performed in overwrite mode

Author: perrette

papers/__main__.py

@@ -18,7 +18,7 @@
 from papers.config import bcolors, Config, search_config, CONFIG_FILE, CONFIG_FILE_LOCAL, DATA_DIR, CONFIG_FILE_LEGACY
 from papers.duplicate import list_duplicates, list_uniques, edit_entries
 from papers.bib import Biblio, FUZZY_RATIO, DEFAULT_SIMILARITY, entry_filecheck, backupfile as backupfile_func, isvalidkey
-from papers.utils import move
+from papers.utils import move, checksum
 from papers import __version__
 
 
@@ -45,6 +45,8 @@ def get_biblio(config):
 
 
 def _backup_bib(biblio, config, message=None):
+    if not config.git:
+        raise PapersExit('cannot backup without --git enabled')
     # backupdir = Path(config.file).parent
     backupdir = Path(config.gitdir)
     backupdir.mkdir(exist_ok=True)
@@ -55,6 +57,7 @@ def _backup_bib(biblio, config, message=None):
 
     ## Here we could create a copy of biblio since it is modified in place
     ## For now, we exit the program after saving, so don't bother
+    logger.debug(f'BACKUP: cp {config.bibtex} {config.backupfile}')
     shutil.copy(config.bibtex, config.backupfile)
     config.gitcmd(f"add {config.backupfile.name}")
 
@@ -196,6 +199,8 @@ def savebib(biblio, config):
         biblio.save(config.bibtex)
     if config.file and config.git:
         _backup_bib(biblio, config)
+    else:
+        logger.debug(f'do not backup bib: {config.file}, {config.git}')
     # if config.git:
         # config.gitcommit()
 
@@ -239,12 +244,18 @@ def installcmd(parser, o, config):
         o.edit = ans.lower() == 'e'
 
     if not o.edit:
+        if config.local and os.path.exists(config.file):
+            # if we don't do that the local install will take precedence over global install
+            logger.warning(f"remove pre-existing local configuration file: {config.file}")
+            os.remove(config.file)
         config = Config()
 
     if o.local is None:
         if config.local is not None:
+            logger.debug(f'keep config to pre-existing: {config.local}')
             o.local = config.local
         else:
+            logger.debug(f'default to global config')
             # default ?
             o.local = False
 
@@ -269,6 +280,7 @@ def installcmd(parser, o, config):
         workdir = Path(DATA_DIR)
         bibtex_files = [str(f) for f in sorted(Path('.').glob("*.bib"))] + [str(f) for f in sorted(workdir.glob("*.bib"))]
         checkdirs = [os.path.join(DATA_DIR, "files")] + checkdirs
+        config.gitdir = config.data = DATA_DIR
 
         if o.absolute_paths is None:
             o.absolute_paths = True
@@ -324,7 +336,6 @@ def installcmd(parser, o, config):
     config.bibtex = o.bibtex
     config.filesdir = o.filesdir
     config.file = papersconfig
-    config.gitdir = config.data = os.path.dirname(config.file)
     config.local = o.local
     config.absolute_paths = o.absolute_paths
 
@@ -395,11 +406,6 @@ def installcmd(parser, o, config):
             config.gitcmd('lfs track "files/"')
             config.gitcmd('add .gitattributes')
 
-        with open(Path(config.gitdir)/'.gitignore', 'a+') as f:
-            lines = f.readlines()
-            if 'futures.txt' not in (l.strip() for l in lines):
-                f.write('futures.txt\n')
-        config.gitcmd('add .gitignore')
         config.gitcmd(f'add {os.path.abspath(config.file)}')
         message = f'papers ' +' '.join(sys.argv[1:])
         config.gitcmd(f'commit -m "new install: config file"', check=False)
@@ -432,7 +438,7 @@ def uninstallcmd(parser, o, config):
         config.file = check_legacy_config(config.file)
         uninstallcmd(parser, o, config)
 
-def check_install(parser, o, config):
+def check_install(parser, o, config, bibtex_must_exist=True):
     """
     Given an option and config, checks to see if the install is done correctly on this filesystem.
     """
@@ -450,7 +456,8 @@ def check_install(parser, o, config):
         parser.print_help()
         print(f"--bibtex must be specified, or {install_doc}")
         raise PapersExit()
-    elif not os.path.exists(config.bibtex):
+
+    if bibtex_must_exist and not os.path.exists(config.bibtex):
         print(f'papers: error: no bibtex file found, do `touch {config.bibtex}` or {install_doc}')
         raise PapersExit()
     logger.info(f'bibtex: {config.bibtex!r}')

papers/config.py

@@ -17,10 +17,10 @@
 CACHE_HOME = os.environ.get('XDG_CACHE_HOME', os.path.join(HOME, '.cache'))
 DATA_HOME = os.environ.get('XDG_DATA_HOME', os.path.join(HOME, '.local','share'))
 
-CONFIG_FILE_LEGACY = os.path.join(CONFIG_HOME, 'papersconfig.json')
-CONFIG_FILE = os.path.join(DATA_HOME, 'config.json')
 CONFIG_FILE_LOCAL = '.papers/config.json'
 DATA_DIR = os.path.join(DATA_HOME, 'papers')
+CONFIG_FILE = os.path.join(DATA_DIR, 'config.json')
+CONFIG_FILE_LEGACY = os.path.join(CONFIG_HOME, 'papersconfig.json')
 CACHE_DIR = os.path.join(CACHE_HOME, 'papers')
 
 
@@ -83,6 +83,7 @@ def root(self):
             return Path(os.path.sep)
 
     def gitcmd(self, cmd, check=True, **kw):
+        logger.debug(f"git add {self.backupfile.name}")
         return (sp.check_call if check else sp.call)(f"git {cmd}", shell=True, cwd=self.gitdir, **kw)
 
 

tests/common.py

@@ -15,7 +15,7 @@
 from papers import logger, logging
 from papers.utils import set_directory
 from papers.__main__ import main
-from papers.config import CONFIG_FILE, CONFIG_FILE_LOCAL, Config
+from papers.config import CONFIG_FILE, CONFIG_FILE_LOCAL, Config, DATA_DIR
 from papers.bib import Biblio
 
 debug_level = logging.DEBUG
@@ -129,6 +129,39 @@ class BibTest(unittest.TestCase):
     """base class for bib tests: create a new bibliography
     """
 
+    @classmethod
+    def setUpClass(cls):
+        """ Move aside any pre-existing global config
+        """
+        if os.path.exists(CONFIG_FILE):
+            cls._backup = tempfile.mktemp(prefix='papers.bib.backup')
+            shutil.move(CONFIG_FILE, cls._backup)
+        else:
+            cls._backup = None
+
+        if os.path.exists(DATA_DIR):
+            cls._backup_dir = tempfile.TemporaryDirectory()
+            shutil.move(DATA_DIR, cls._backup_dir.name)
+        else:
+            cls._backup_dir = None
+
+    @classmethod
+    def tearDownClass(cls):
+        """ Restore any pre-existing global config
+        """
+        if os.path.exists(CONFIG_FILE):
+            os.remove(CONFIG_FILE)
+        if cls._backup:
+            shutil.move(cls._backup, CONFIG_FILE)
+
+        if os.path.exists(DATA_DIR):
+            shutil.rmtree(DATA_DIR)
+
+        if cls._backup_dir is not None:
+            shutil.move(os.path.join(cls._backup_dir.name, os.path.basename(DATA_DIR)), DATA_DIR)
+            cls._backup_dir.cleanup()
+
+
     def assertMultiLineEqual(self, first, second, msg=None):
         """Assert that two multi-line strings are equal.
 
@@ -181,14 +214,12 @@ class BaseTest(BibTest):
     anotherbib_content = bibtex
     initial_content = None
 
+
     def setUp(self):
-        if os.path.exists(CONFIG_FILE):
-            self.backup = tempfile.mktemp(prefix='papers.bib.backup')
-            shutil.move(CONFIG_FILE, self.backup)
-        else:
-            self.backup = None
+
 
         self.temp_dir = tempfile.TemporaryDirectory()
+        print(self, 'setUp', self.temp_dir.name)
 
         if self.anotherbib_content is not None:
             open(self._path(self.anotherbib), 'w').write(self.anotherbib_content)
@@ -197,11 +228,9 @@ def setUp(self):
             open(self._path(self.mybib), 'w').write(self.initial_content)
 
     def tearDown(self):
-        if os.path.exists(CONFIG_FILE):
-            os.remove(CONFIG_FILE)
-        if self.backup:
-            shutil.move(self.backup, CONFIG_FILE)
+        print(self, 'cleanup', self.temp_dir.name)
         self.temp_dir.cleanup()
+        assert not os.path.exists(self.temp_dir.name)
 
 
     def _path(self, p):
@@ -222,38 +251,38 @@ class LocalInstallTest(BaseTest):
     def setUp(self):
         super().setUp()
         self.papers(f'install --force --local --bibtex {self.mybib} --files {self.filesdir}')
-        self.config = Config.load(self._path(CONFIG_FILE_LOCAL))
+
+    @property
+    def config(self):
+        return Config.load(self._path(CONFIG_FILE_LOCAL))
 
 class GlobalInstallTest(BaseTest):
     def setUp(self):
         super().setUp()
         self.papers(f'install --force --bibtex {self.mybib} --files {self.filesdir}')
-        self.config = Config.load(CONFIG_FILE)
+
+    @property
+    def config(self):
+        return Config.load(CONFIG_FILE)
 
 
 class LocalGitInstallTest(LocalInstallTest):
     def setUp(self):
         super().setUp()
         self.papers(f'install --force --local --git --bibtex {self.mybib} --files {self.filesdir}')
-        self.config = Config.load(self._path(CONFIG_FILE_LOCAL))
 
-
-class GlobalGitInstallTest(LocalInstallTest):
+class GlobalGitInstallTest(GlobalInstallTest):
     def setUp(self):
         super().setUp()
         self.papers(f'install --force --git --bibtex {self.mybib} --files {self.filesdir}')
-        self.config = Config.load(self._path(CONFIG_FILE))
-
 
 class LocalGitLFSInstallTest(LocalInstallTest):
     def setUp(self):
         super().setUp()
         self.papers(f'install --force --local --git --git-lfs --bibtex {self.mybib} --files {self.filesdir}')
-        self.config = Config.load(self._path(CONFIG_FILE_LOCAL))
 
 
-class GlobalGitLFSInstallTest(LocalInstallTest):
+class GlobalGitLFSInstallTest(GlobalInstallTest):
     def setUp(self):
         super().setUp()
         self.papers(f'install --force --git --git-lfs --bibtex {self.mybib} --files {self.filesdir}')
-        self.config = Config.load(self._path(CONFIG_FILE))

tests/test_undo.py

@@ -19,24 +19,41 @@
  year = {2000}
 }"""
 
-class TestTimeTravelGitLocal(LocalGitInstallTest):
+class TimeTravelBase:
 
     def get_commit(self):
         return sp.check_output(f"git rev-parse HEAD", shell=True, cwd=self.config.gitdir).strip().decode()
 
     def test_undo(self):
         ## Make sure git undo / redo travels as expected
 
+        print(self.config.status(verbose=True))
+        self.assertTrue(self.config.git)
+
         commits = []
         commits.append(self.get_commit())
 
         self.papers(f'add {self.anotherbib}')
+        self.assertTrue(self.config.git)
         commits.append(self.get_commit())
+        print("bib add paper:", self._path(self.mybib))
+        print(open(self._path(self.mybib)).read())
+        print("backup after add paper:", self.config.backupfile_clean)
+        print(open(self.config.backupfile_clean).read())
 
         self.papers(f'list --add-tag change')
+        self.assertTrue(self.config.git)
         commits.append(self.get_commit())
+        print("bib add-tag:", self._path(self.mybib))
+        print(open(self._path(self.mybib)).read())
+        print("backup after add-tag:", self.config.backupfile_clean)
+        print(open(self.config.backupfile_clean).read())
 
         # make sure we have 3 distinct commits
+        self.config.gitcmd('log')
+        print(commits)
+        print(self.config.gitdir)
+        sp.check_call(f'ls {self.config.gitdir}', shell=True)
         self.assertEqual(len(set(commits)), 3)
 
         self.papers(f'undo')
@@ -72,6 +89,13 @@ def test_undo(self):
         self.assertEqual(current, commits[-1])
 
 
+class TestTimeTravelGitLocal(LocalGitInstallTest, TimeTravelBase):
+    pass
+
+class TestTimeTravelGitGlobal(GlobalGitInstallTest, TimeTravelBase):
+    pass
+
+
 
 class TestUndoGitLocal(LocalGitLFSInstallTest):