From 00aa595f37ca648d4e3592f2a4b6ece7740739de Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Thu, 1 Jun 2023 20:23:00 +0200 Subject: [PATCH 1/4] deploy: give useful error if rsync is not installed --- fdroidserver/deploy.py | 6 ++++++ tests/deploy.TestCase | 8 ++++++++ 2 files changed, 14 insertions(+) diff --git a/fdroidserver/deploy.py b/fdroidserver/deploy.py index 54496f78..7cc709d6 100644 --- a/fdroidserver/deploy.py +++ b/fdroidserver/deploy.py @@ -277,6 +277,12 @@ def update_serverwebroot(serverwebroot, repo_section): has a low resolution timestamp """ + try: + subprocess.run(['rsync', '--version'], capture_output=True, check=True) + except Exception as e: + raise FDroidException( + _('rsync is missing or broken: {error}').format(error=e) + ) from e rsyncargs = ['rsync', '--archive', '--delete-after', '--safe-links'] if not options.no_checksum: rsyncargs.append('--checksum') diff --git a/tests/deploy.TestCase b/tests/deploy.TestCase index d35200be..5539af4c 100755 --- a/tests/deploy.TestCase +++ b/tests/deploy.TestCase @@ -18,6 +18,7 @@ if localmodule not in sys.path: import fdroidserver.common import fdroidserver.deploy +from fdroidserver.exception import FDroidException from testcommon import TmpCwd, mkdtemp @@ -56,6 +57,13 @@ class DeployTest(unittest.TestCase): fdroidserver.deploy.update_serverwebroot(str(serverwebroot), 'repo') self.assertTrue(dest_apk.is_file()) + @mock.patch.dict(os.environ, clear=True) + def test_update_serverwebroot_no_rsync_error(self): + os.environ['PATH'] = self.testdir + os.chdir(self.testdir) + with self.assertRaises(FDroidException): + fdroidserver.deploy.update_serverwebroot('serverwebroot', 'repo') + def test_update_serverwebroot_make_cur_version_link(self): # setup parameters for this test run fdroidserver.deploy.options.no_checksum = True From 64b8ee772c494fa84d3e14387842ee4a7b6a11d7 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Wed, 7 Jun 2023 11:03:14 +0200 Subject: [PATCH 2/4] throw useful error if a config YAML file is not a dict --- fdroidserver/common.py | 8 ++++++++ tests/common.TestCase | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/fdroidserver/common.py b/fdroidserver/common.py index b2d1ae85..a0940154 100644 --- a/fdroidserver/common.py +++ b/fdroidserver/common.py @@ -399,6 +399,11 @@ def read_config(opts=None): config = yaml.safe_load(fp) if not config: config = {} + if not isinstance(config, dict): + msg = _('{path} is not "key: value" dict, but a {datatype}!') + raise TypeError( + msg.format(path=config_file, datatype=type(config).__name__) + ) elif os.path.exists(old_config_file): logging.warning(_("""{oldfile} is deprecated, use {newfile}""") .format(oldfile=old_config_file, newfile=config_file)) @@ -528,6 +533,9 @@ def load_localized_config(name, repodir): locale = DEFAULT_LOCALE with open(f, encoding="utf-8") as fp: elem = yaml.safe_load(fp) + if not isinstance(elem, dict): + msg = _('{path} is not "key: value" dict, but a {datatype}!') + raise TypeError(msg.format(path=f, datatype=type(elem).__name__)) for afname, field_dict in elem.items(): if afname not in ret: ret[afname] = dict() diff --git a/tests/common.TestCase b/tests/common.TestCase index 66c5f184..d98ed24b 100755 --- a/tests/common.TestCase +++ b/tests/common.TestCase @@ -1919,6 +1919,18 @@ class CommonTest(unittest.TestCase): config = fdroidserver.common.read_config(fdroidserver.common.options) self.assertEqual(os.getenv('SECRET', 'fail'), config.get('keypass')) + def test_with_config_yml_is_dict(self): + os.chdir(self.tmpdir) + Path('config.yml').write_text('apksigner = /placeholder/path') + with self.assertRaises(TypeError): + fdroidserver.common.read_config(fdroidserver.common.options) + + def test_with_config_yml_is_not_mixed_type(self): + os.chdir(self.tmpdir) + Path('config.yml').write_text('k: v\napksigner = /placeholder/path') + with self.assertRaises(yaml.scanner.ScannerError): + fdroidserver.common.read_config(fdroidserver.common.options) + def test_with_config_py(self): """Make sure it is still possible to use config.py alone.""" os.chdir(self.tmpdir) @@ -2716,6 +2728,27 @@ class CommonTest(unittest.TestCase): ) self.assertEqual(['en-US'], list(categories['GuardianProject']['name'].keys())) + def test_load_localized_config_0_file(self): + os.chdir(self.testdir) + os.mkdir('config') + Path('config/categories.yml').write_text('') + with self.assertRaises(TypeError): + fdroidserver.common.load_localized_config(CATEGORIES_CONFIG_NAME, 'repo') + + def test_load_localized_config_string(self): + os.chdir(self.testdir) + os.mkdir('config') + Path('config/categories.yml').write_text('this is a string') + with self.assertRaises(TypeError): + fdroidserver.common.load_localized_config(CATEGORIES_CONFIG_NAME, 'repo') + + def test_load_localized_config_list(self): + os.chdir(self.testdir) + os.mkdir('config') + Path('config/categories.yml').write_text('- System') + with self.assertRaises(TypeError): + fdroidserver.common.load_localized_config(CATEGORIES_CONFIG_NAME, 'repo') + if __name__ == "__main__": os.chdir(os.path.dirname(__file__)) From 2c566cf68f40e91ee62ec20d72d5a9777a79d8ec Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Wed, 7 Jun 2023 13:36:02 +0200 Subject: [PATCH 3/4] update: add all categories in metadata files to repo definitions !1366 makes it so categories are now defined by the repo. Categories can be defined in the config so that lint has a list of categories to enforce. This also provides a place for localization and icons for the categories. The old way of defining categories was just listing them in app metadata files. This restores that way of functioning when using index-v2. closes #1137 --- fdroidserver/index.py | 11 ++++- tests/repo/entry.json | 4 +- tests/repo/index-v2.json | 7 ++- tests/update.TestCase | 94 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 111 insertions(+), 5 deletions(-) diff --git a/fdroidserver/index.py b/fdroidserver/index.py index ddb51546..d05d159e 100644 --- a/fdroidserver/index.py +++ b/fdroidserver/index.py @@ -711,6 +711,7 @@ def make_v2(apps, packages, repodir, repodict, requestsdict, fdroid_signing_key_ output_packages = collections.OrderedDict() output['packages'] = output_packages + categories_used_by_apps = set() for package in packages: packageName = package['packageName'] if packageName not in apps: @@ -730,7 +731,9 @@ def make_v2(apps, packages, repodir, repodict, requestsdict, fdroid_signing_key_ else: packagelist = {} output_packages[packageName] = packagelist - packagelist["metadata"] = package_metadata(apps[packageName], repodir) + app = apps[packageName] + categories_used_by_apps.update(app.get('Categories', [])) + packagelist["metadata"] = package_metadata(app, repodir) if "signer" in package: packagelist["metadata"]["preferredSigner"] = package["signer"] @@ -738,6 +741,12 @@ def make_v2(apps, packages, repodir, repodict, requestsdict, fdroid_signing_key_ packagelist["versions"][package["hash"]] = convert_version(package, apps[packageName], repodir) + if categories_used_by_apps and not output['repo'].get(CATEGORIES_CONFIG_NAME): + output['repo'][CATEGORIES_CONFIG_NAME] = dict() + for category in sorted(categories_used_by_apps): + if category not in output['repo'][CATEGORIES_CONFIG_NAME]: + output['repo'][CATEGORIES_CONFIG_NAME][category] = dict() + entry = {} entry["timestamp"] = repodict["timestamp"] diff --git a/tests/repo/entry.json b/tests/repo/entry.json index 5c0ca528..0e8828e0 100644 --- a/tests/repo/entry.json +++ b/tests/repo/entry.json @@ -3,8 +3,8 @@ "version": 20002, "index": { "name": "/index-v2.json", - "sha256": "7117ee6ff4ff2dd71ec3f3d3ad2ef7e9fd4afead9b1f2d39d0b224a1812e78b5", - "size": 53233, + "sha256": "b613858aa7a2ec476fcef5c841a5b8ff4b3b0f67f07678da981e2843f49c71ba", + "size": 53283, "numPackages": 10 }, "diffs": {} diff --git a/tests/repo/index-v2.json b/tests/repo/index-v2.json index 6ea92407..f45c1514 100644 --- a/tests/repo/index-v2.json +++ b/tests/repo/index-v2.json @@ -533,7 +533,10 @@ "name": { "en-US": "System" } - } + }, + "1": {}, + "2.0": {}, + "tests": {} }, "requests": { "install": [ @@ -1443,4 +1446,4 @@ } } } -} +} \ No newline at end of file diff --git a/tests/update.TestCase b/tests/update.TestCase index 391dec93..e859e04f 100755 --- a/tests/update.TestCase +++ b/tests/update.TestCase @@ -53,6 +53,7 @@ import fdroidserver.common import fdroidserver.exception import fdroidserver.metadata import fdroidserver.update +from fdroidserver.common import CATEGORIES_CONFIG_NAME DONATION_FIELDS = ('Donate', 'Liberapay', 'OpenCollective') @@ -1804,6 +1805,99 @@ class UpdateTest(unittest.TestCase): fdroidserver.update.main() self.assertFalse(categories_txt.exists()) + def test_no_blank_auto_defined_categories(self): + """When no app has Categories, there should be no definitions in the repo.""" + os.chdir(self.testdir) + os.mkdir('metadata') + os.mkdir('repo') + Path('config.yml').write_text( + 'repo_pubkey: ffffffffffffffffffffffffffffffffffffffff' + ) + + testapk = os.path.join('repo', 'com.politedroid_6.apk') + shutil.copy(os.path.join(self.basedir, testapk), testapk) + Path('metadata/com.politedroid.yml').write_text('Name: Polite') + + with mock.patch('sys.argv', ['fdroid update', '--delete-unknown', '--nosign']): + fdroidserver.update.main() + with open('repo/index-v2.json') as fp: + index = json.load(fp) + self.assertFalse(CATEGORIES_CONFIG_NAME in index['repo']) + + def test_auto_defined_categories(self): + """Repos that don't define categories in config/ should use auto-generated.""" + os.chdir(self.testdir) + os.mkdir('metadata') + os.mkdir('repo') + Path('config.yml').write_text( + 'repo_pubkey: ffffffffffffffffffffffffffffffffffffffff' + ) + + testapk = os.path.join('repo', 'com.politedroid_6.apk') + shutil.copy(os.path.join(self.basedir, testapk), testapk) + Path('metadata/com.politedroid.yml').write_text('Categories: [Time]') + + with mock.patch('sys.argv', ['fdroid update', '--delete-unknown', '--nosign']): + fdroidserver.update.main() + with open('repo/index-v2.json') as fp: + index = json.load(fp) + self.assertEqual( + {'Time': dict()}, + index['repo'][CATEGORIES_CONFIG_NAME], + ) + + def test_auto_defined_categories_two_apps(self): + """Repos that don't define categories in config/ should use auto-generated.""" + os.chdir(self.testdir) + os.mkdir('metadata') + os.mkdir('repo') + Path('config.yml').write_text( + 'repo_pubkey: ffffffffffffffffffffffffffffffffffffffff' + ) + + testapk = os.path.join('repo', 'com.politedroid_6.apk') + shutil.copy(os.path.join(self.basedir, testapk), testapk) + Path('metadata/com.politedroid.yml').write_text('Categories: [bar]') + testapk = os.path.join('repo', 'souch.smsbypass_9.apk') + shutil.copy(os.path.join(self.basedir, testapk), testapk) + Path('metadata/souch.smsbypass.yml').write_text('Categories: [foo, bar]') + + with mock.patch('sys.argv', ['fdroid update', '--delete-unknown', '--nosign']): + fdroidserver.update.main() + with open('repo/index-v2.json') as fp: + index = json.load(fp) + self.assertEqual( + {'bar': dict(), 'foo': dict()}, + index['repo'][CATEGORIES_CONFIG_NAME], + ) + + def test_auto_defined_categories_mix_into_config_categories(self): + """Repos that don't define all categories in config/ also use auto-generated.""" + os.chdir(self.testdir) + os.mkdir('config') + Path('config/categories.yml').write_text('System: {name: System Apps}') + os.mkdir('metadata') + os.mkdir('repo') + Path('config.yml').write_text( + 'repo_pubkey: ffffffffffffffffffffffffffffffffffffffff' + ) + + testapk = os.path.join('repo', 'com.politedroid_6.apk') + shutil.copy(os.path.join(self.basedir, testapk), testapk) + Path('metadata/com.politedroid.yml').write_text('Categories: [Time]') + testapk = os.path.join('repo', 'souch.smsbypass_9.apk') + shutil.copy(os.path.join(self.basedir, testapk), testapk) + Path('metadata/souch.smsbypass.yml').write_text('Categories: [System, Time]') + + with mock.patch('sys.argv', ['fdroid update', '--delete-unknown', '--nosign']): + fdroidserver.update.main() + with open('repo/index-v2.json') as fp: + index = json.load(fp) + self.assertEqual( + {'System': {'name': {'en-US': 'System Apps'}}, 'Time': dict()}, + index['repo'][CATEGORIES_CONFIG_NAME], + ) + if __name__ == "__main__": os.chdir(os.path.dirname(__file__)) From 48559ecec5a1c402f4d4bf0dc6c8b1c09c555b1a Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Wed, 7 Jun 2023 15:57:58 +0200 Subject: [PATCH 4/4] category with no apps should be ignored, even if defined in config https://gitlab.com/fdroid/fdroidclient/-/issues/2619#note_1421280589 The test needed to change because the test index files contained category definitions that were not ever used in the "copy tests/repo, generate java/gpg keys, update, and gpgsign" test in tests/run-tests. --- fdroidserver/index.py | 7 +++++++ tests/metadata.TestCase | 2 ++ tests/metadata/com.politedroid.yml | 2 ++ tests/metadata/dump/com.politedroid.yaml | 2 ++ tests/repo/entry.json | 2 +- tests/repo/index-v1.json | 2 ++ tests/repo/index-v2.json | 2 ++ tests/repo/index.xml | 4 ++-- tests/update.TestCase | 24 ++++++++++++++++++++++++ 9 files changed, 44 insertions(+), 3 deletions(-) diff --git a/fdroidserver/index.py b/fdroidserver/index.py index d05d159e..bef83ff6 100644 --- a/fdroidserver/index.py +++ b/fdroidserver/index.py @@ -743,9 +743,16 @@ def make_v2(apps, packages, repodir, repodict, requestsdict, fdroid_signing_key_ if categories_used_by_apps and not output['repo'].get(CATEGORIES_CONFIG_NAME): output['repo'][CATEGORIES_CONFIG_NAME] = dict() + # include definitions for "auto-defined" categories, e.g. just used in app metadata for category in sorted(categories_used_by_apps): if category not in output['repo'][CATEGORIES_CONFIG_NAME]: output['repo'][CATEGORIES_CONFIG_NAME][category] = dict() + # do not include defined categories if no apps use them + for category in list(output['repo'].get(CATEGORIES_CONFIG_NAME, list())): + if category not in categories_used_by_apps: + del output['repo'][CATEGORIES_CONFIG_NAME][category] + msg = _('Category "{category}" defined but not used for any apps!') + logging.warning(msg.format(category=category)) entry = {} entry["timestamp"] = repodict["timestamp"] diff --git a/tests/metadata.TestCase b/tests/metadata.TestCase index 15e2eddd..c46d5bbf 100755 --- a/tests/metadata.TestCase +++ b/tests/metadata.TestCase @@ -1863,6 +1863,8 @@ class MetadataTest(unittest.TestCase): AntiFeatures: - NonFreeNet Categories: + - Multimedia + - Security - Time License: GPL-3.0-only SourceCode: https://github.com/miguelvps/PoliteDroid diff --git a/tests/metadata/com.politedroid.yml b/tests/metadata/com.politedroid.yml index 669520a6..cd474d6c 100644 --- a/tests/metadata/com.politedroid.yml +++ b/tests/metadata/com.politedroid.yml @@ -1,6 +1,8 @@ AntiFeatures: - NonFreeNet Categories: + - Multimedia + - Security - Time License: GPL-3.0-only SourceCode: https://github.com/miguelvps/PoliteDroid diff --git a/tests/metadata/dump/com.politedroid.yaml b/tests/metadata/dump/com.politedroid.yaml index 17e6a8f3..57cce841 100644 --- a/tests/metadata/dump/com.politedroid.yaml +++ b/tests/metadata/dump/com.politedroid.yaml @@ -161,6 +161,8 @@ Builds: versionCode: 6 versionName: '1.5' Categories: +- Multimedia +- Security - Time Changelog: '' CurrentVersion: '1.5' diff --git a/tests/repo/entry.json b/tests/repo/entry.json index 0e8828e0..56249552 100644 --- a/tests/repo/entry.json +++ b/tests/repo/entry.json @@ -3,7 +3,7 @@ "version": 20002, "index": { "name": "/index-v2.json", - "sha256": "b613858aa7a2ec476fcef5c841a5b8ff4b3b0f67f07678da981e2843f49c71ba", + "sha256": "5e3c0eaafd99d3518da2bb2bc7565b2ebcb17775a2f4ccc33b7336901ec71a6f", "size": 53283, "numPackages": 10 }, diff --git a/tests/repo/index-v1.json b/tests/repo/index-v1.json index 4b845994..33d0f9ce 100644 --- a/tests/repo/index-v1.json +++ b/tests/repo/index-v1.json @@ -174,6 +174,8 @@ "NonFreeNet" ], "categories": [ + "Multimedia", + "Security", "Time" ], "suggestedVersionName": "1.5", diff --git a/tests/repo/index-v2.json b/tests/repo/index-v2.json index f45c1514..d41b95b5 100644 --- a/tests/repo/index-v2.json +++ b/tests/repo/index-v2.json @@ -553,6 +553,8 @@ "metadata": { "added": 1498176000000, "categories": [ + "Multimedia", + "Security", "Time" ], "issueTracker": "https://github.com/miguelvps/PoliteDroid/issues", diff --git a/tests/repo/index.xml b/tests/repo/index.xml index 6935f675..24ebea24 100644 --- a/tests/repo/index.xml +++ b/tests/repo/index.xml @@ -311,8 +311,8 @@ APK is called F-Droid Privileged Extension. com.politedroid.6.png Activates silent mode during calendar events. GPL-3.0-only - Time - Time + Multimedia,Security,Time + Multimedia https://github.com/miguelvps/PoliteDroid https://github.com/miguelvps/PoliteDroid/issues diff --git a/tests/update.TestCase b/tests/update.TestCase index e859e04f..ac9a6f5e 100755 --- a/tests/update.TestCase +++ b/tests/update.TestCase @@ -1898,6 +1898,30 @@ class UpdateTest(unittest.TestCase): index['repo'][CATEGORIES_CONFIG_NAME], ) + def test_empty_categories_not_in_index(self): + """A category with no apps should be ignored, even if defined in config.""" + os.chdir(self.testdir) + os.mkdir('config') + Path('config/categories.yml').write_text('System: {name: S}\nTime: {name: T}\n') + os.mkdir('metadata') + os.mkdir('repo') + Path('config.yml').write_text( + 'repo_pubkey: ffffffffffffffffffffffffffffffffffffffff' + ) + + testapk = os.path.join('repo', 'com.politedroid_6.apk') + shutil.copy(os.path.join(self.basedir, testapk), testapk) + Path('metadata/com.politedroid.yml').write_text('Categories: [Time]') + + with mock.patch('sys.argv', ['fdroid update', '--delete-unknown', '--nosign']): + fdroidserver.update.main() + with open('repo/index-v2.json') as fp: + index = json.load(fp) + self.assertEqual( + {'Time': {'name': {'en-US': 'T'}}}, + index['repo'][CATEGORIES_CONFIG_NAME], + ) + if __name__ == "__main__": os.chdir(os.path.dirname(__file__))