diff --git a/CHANGELOG.md b/CHANGELOG.md index 3deb6e79..a486c5f8 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/fdroidserver/metadata.py b/fdroidserver/metadata.py index 17a3e739..d34b824e 100644 --- a/fdroidserver/metadata.py +++ b/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) - elif ext == 'yml': + return write_txt(mf, app) + 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: - os.remove(metadatapath) - raise e + else: + raise FDroidException('ruamel.yaml not installed, can not write metadata.') warn_or_exception(_('Unknown metadata format: %s') % metadatapath) diff --git a/fdroidserver/rewritemeta.py b/fdroidserver/rewritemeta.py index 07fb7492..97d15852 100644 --- a/fdroidserver/rewritemeta.py +++ b/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) - - if ext != to_ext: - os.remove(path) + try: + metadata.write_metadata(base + '.' + to_ext, app) + # remove old format metadata if there was a format change + # and rewriting to the new format worked + if ext != to_ext: + os.remove(path) + finally: + pass logging.debug(_("Finished")) diff --git a/tests/rewritemeta.TestCase b/tests/rewritemeta.TestCase new file mode 100755 index 00000000..664f3c78 --- /dev/null +++ b/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)