Merge branch 'fix-disapearing-metadata' into 'master'

keep yaml metadata when rewrite failed

Closes #645

See merge request fdroid/fdroidserver!658

CHANGELOG.md

@@ -15,6 +15,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 * checkupdates: UpdateCheckIngore gets properly observed now
   ([!659](https://gitlab.com/fdroid/fdroidserver/merge_requests/659),
   [!660](https://gitlab.com/fdroid/fdroidserver/merge_requests/660))
+* keep yaml metadata when rewrite failed
+  ([!658](https://gitlab.com/fdroid/fdroidserver/merge_requests/658))
 * import: `template.yml` now supports omitting values
   ([!657](https://gitlab.com/fdroid/fdroidserver/merge_requests/657))
 * build: deploying buildlogs with rsync

fdroidserver/metadata.py

@@ -27,6 +27,7 @@ import logging
 import textwrap
 import io
 import yaml
+import importlib
 from collections import OrderedDict
 
 import fdroidserver.common
@@ -1584,15 +1585,15 @@ def write_metadata(metadatapath, app):
         warn_or_exception(_('Cannot write "{path}", not an accepted format, use: {formats}')
                           .format(path=metadatapath, formats=', '.join(accepted)))
 
-    try:
+    if ext == 'txt':
         with open(metadatapath, 'w') as mf:
-            if ext == 'txt':
+            return write_txt(mf, app)
-                return write_txt(mf, app)
+    elif ext == 'yml':
-            elif ext == 'yml':
+        if importlib.util.find_spec('ruamel.yaml'):
+            with open(metadatapath, 'w') as mf:
                 return write_yaml(mf, app)
-    except FDroidException as e:
+        else:
-        os.remove(metadatapath)
+            raise FDroidException('ruamel.yaml not installed, can not write metadata.')
-        raise e
 
     warn_or_exception(_('Unknown metadata format: %s') % metadatapath)
 

fdroidserver/rewritemeta.py

@@ -30,6 +30,9 @@ config = None
 options = None
 
 
+SUPPORTED_FORMATS = ['txt', 'yml']
+
+
 def proper_format(app):
     s = io.StringIO()
     # TODO: currently reading entire file again, should reuse first
@@ -50,15 +53,13 @@ def main():
 
     global config, options
 
-    supported = ['txt', 'yml']
-
     # Parse command line...
     parser = ArgumentParser(usage="%(prog)s [options] [APPID [APPID ...]]")
     common.setup_global_opts(parser)
     parser.add_argument("-l", "--list", action="store_true", default=False,
                         help=_("List files that would be reformatted"))
     parser.add_argument("-t", "--to", default=None,
-                        help=_("Rewrite to a specific format: ") + ', '.join(supported))
+                        help=_("Rewrite to a specific format: ") + ', '.join(SUPPORTED_FORMATS))
     parser.add_argument("appid", nargs='*', help=_("applicationId in the form APPID"))
     metadata.add_metadata_arguments(parser)
     options = parser.parse_args()
@@ -73,14 +74,14 @@ def main():
     if options.list and options.to is not None:
         parser.error(_("Cannot use --list and --to at the same time"))
 
-    if options.to is not None and options.to not in supported:
+    if options.to is not None and options.to not in SUPPORTED_FORMATS:
         parser.error(_("Unsupported metadata format, use: --to [{supported}]")
-                     .format(supported=' '.join(supported)))
+                     .format(supported=' '.join(SUPPORTED_FORMATS)))
 
     for appid, app in apps.items():
         path = app.metadatapath
         base, ext = common.get_extension(path)
-        if not options.to and ext not in supported:
+        if not options.to and ext not in SUPPORTED_FORMATS:
             logging.info(_("Ignoring {ext} file at '{path}'").format(ext=ext, path=path))
             continue
         elif options.to is not None:
@@ -108,10 +109,14 @@ def main():
             newbuilds.append(new)
         app.builds = newbuilds
 
-        metadata.write_metadata(base + '.' + to_ext, app)
+        try:
-
+            metadata.write_metadata(base + '.' + to_ext, app)
-        if ext != to_ext:
+            # remove old format metadata if there was  a format change
-            os.remove(path)
+            # and rewriting to the new format worked
+            if ext != to_ext:
+                os.remove(path)
+        finally:
+            pass
 
     logging.debug(_("Finished"))
 

tests/rewritemeta.TestCase

@@ -0,0 +1,153 @@
+#!/usr/bin/env python3
+
+#
+#  command which created the keystore used in this test case:
+#
+#  $ for ALIAS in 'repokey a163ec9b d2d51ff2 dc3b169e 78688a0f'; \
+#        do keytool -genkey -keystore dummy-keystore.jks \
+#        -alias $ALIAS -keyalg 'RSA' -keysize '2048' \
+#        -validity '10000' -storepass 123456 \
+#        -keypass 123456 -dname 'CN=test, OU=F-Droid'; done
+#
+
+import inspect
+import logging
+import optparse
+import os
+import sys
+import unittest
+import tempfile
+import textwrap
+from unittest import mock
+from testcommon import TmpCwd
+
+localmodule = os.path.realpath(
+    os.path.join(os.path.dirname(inspect.getfile(inspect.currentframe())), '..'))
+print('localmodule: ' + localmodule)
+if localmodule not in sys.path:
+    sys.path.insert(0, localmodule)
+
+from fdroidserver import common
+from fdroidserver import rewritemeta
+from fdroidserver.exception import FDroidException
+
+
+class RewriteMetaTest(unittest.TestCase):
+    '''fdroidserver/publish.py'''
+
+    def setUp(self):
+        logging.basicConfig(level=logging.DEBUG)
+        self.basedir = os.path.join(localmodule, 'tests')
+        self.tmpdir = os.path.abspath(os.path.join(self.basedir, '..', '.testfiles'))
+        if not os.path.exists(self.tmpdir):
+            os.makedirs(self.tmpdir)
+        os.chdir(self.basedir)
+
+    def test_rewrite_scenario_trivial(self):
+
+        sys.argv = ['rewritemeta', 'a', 'b']
+
+        with tempfile.TemporaryDirectory() as tmpdir, TmpCwd(tmpdir):
+            os.mkdir('metadata')
+            with open('metadata/a.txt', 'w') as f:
+                f.write('Auto Name:a')
+            with open('metadata/b.yml', 'w') as f:
+                f.write('AutoName: b')
+
+            rewritemeta.main()
+
+            with open('metadata/a.txt') as f:
+                self.assertEqual(f.read(), textwrap.dedent('''\
+                    Categories:
+                    License:Unknown
+                    Web Site:
+                    Source Code:
+                    Issue Tracker:
+
+                    Auto Name:a
+
+                    Auto Update Mode:None
+                    Update Check Mode:None
+                    '''))
+
+            with open('metadata/b.yml') as f:
+                self.assertEqual(f.read(), textwrap.dedent('''\
+                    License: Unknown
+
+                    AutoName: b
+
+                    AutoUpdateMode: None
+                    UpdateCheckMode: None
+                    '''))
+
+    def test_rewrite_scenario_txt_to_yml(self):
+
+        sys.argv = ['rewritemeta', '--to', 'yml', 'a']
+
+        with tempfile.TemporaryDirectory() as tmpdir, TmpCwd(tmpdir):
+            os.mkdir('metadata')
+            with open('metadata/a.txt', 'w') as f:
+                f.write('Auto Name:a')
+
+            rewritemeta.main()
+
+            with open('metadata/a.yml') as f:
+                self.assertEqual(f.read(), textwrap.dedent('''\
+                    License: Unknown
+
+                    AutoName: a
+
+                    AutoUpdateMode: None
+                    UpdateCheckMode: None
+                    '''))
+
+    def test_rewrite_scenario_txt_to_yml_no_ruamel(self):
+
+        sys.argv = ['rewritemeta', '--to', 'yml', 'a']
+
+        with tempfile.TemporaryDirectory() as tmpdir, TmpCwd(tmpdir):
+            os.mkdir('metadata')
+            with open('metadata/a.txt', 'w') as f:
+                f.write('Auto Name:a')
+
+            def boom(*args):
+                raise FDroidException(' '.join((str(x) for x in args)))
+
+            with mock.patch('fdroidserver.metadata.write_yaml', boom):
+                with self.assertRaises(FDroidException):
+                    rewritemeta.main()
+
+            with open('metadata/a.txt') as f:
+                self.assertEqual(f.read(), textwrap.dedent('''\
+                    Auto Name:a'''))
+
+    def test_rewrite_scenario_yml_no_ruamel(self):
+        sys.argv = ['rewritemeta', 'a']
+        with tempfile.TemporaryDirectory() as tmpdir, TmpCwd(tmpdir):
+            os.mkdir('metadata')
+            with open('metadata/a.yml', 'w') as f:
+                f.write('AutoName: a')
+
+            def boom(*args):
+                raise FDroidException(' '.join((str(x) for x in args)))
+
+            with mock.patch('importlib.util.find_spec', boom):
+                with self.assertRaises(FDroidException):
+                    rewritemeta.main()
+
+            with open('metadata/a.yml') as f:
+                self.assertEqual(f.read(), textwrap.dedent('''\
+                    AutoName: a'''))
+
+
+if __name__ == "__main__":
+    os.chdir(os.path.dirname(__file__))
+
+    parser = optparse.OptionParser()
+    parser.add_option("-v", "--verbose", action="store_true", default=False,
+                      help="Spew out even more information than normal")
+    (common.options, args) = parser.parse_args(['--verbose'])
+
+    newSuite = unittest.TestSuite()
+    newSuite.addTest(unittest.makeSuite(RewriteMetaTest))
+    unittest.main(failfast=False)