diff --git a/fdroidserver/common.py b/fdroidserver/common.py index 8069b88d..5500e821 100644 --- a/fdroidserver/common.py +++ b/fdroidserver/common.py @@ -710,6 +710,14 @@ def read_config(): if not config['archive_url'].endswith('/archive'): raise FDroidException(_('archive_url needs to end with /archive')) + repo_icon = config.get('repo_icon', '') + if '/' in repo_icon: + raise FDroidException( + _('repo_icon should be a filename ({name}), not a path!').format( + name=os.path.basename(repo_icon) + ) + ) + confignames_to_delete = set() for configname, dictvalue in config.items(): if configname == 'java_paths': diff --git a/fdroidserver/index.py b/fdroidserver/index.py index b527d6da..ead18c38 100644 --- a/fdroidserver/index.py +++ b/fdroidserver/index.py @@ -102,11 +102,10 @@ def make(apps, apks, repodir, archive): if common.config['repo_maxage'] != 0: repodict['maxage'] = common.config['repo_maxage'] + repo_icon = common.config.get('repo_icon', common.default_config['repo_icon']) if archive: repodict['name'] = common.config['archive_name'] - repodict['icon'] = common.config.get( - 'archive_icon', common.default_config['repo_icon'] - ) + repodict['icon'] = repo_icon repodict['description'] = common.config['archive_description'] archive_url = common.config.get( 'archive_url', common.config['repo_url'][:-4] + 'archive' @@ -117,9 +116,7 @@ def make(apps, apks, repodir, archive): repo_section = os.path.basename(urllib.parse.urlparse(archive_url).path) else: repodict['name'] = common.config['repo_name'] - repodict['icon'] = common.config.get( - 'repo_icon', common.default_config['repo_icon'] - ) + repodict['icon'] = repo_icon repodict['address'] = common.config['repo_url'] if 'repo_web_base_url' in common.config: repodict["webBaseUrl"] = common.config['repo_web_base_url'] @@ -146,6 +143,7 @@ def make(apps, apks, repodir, archive): signer_fingerprints = load_publish_signer_fingerprints() make_v0(sortedapps, apks, repodir, repodict, requestsdict, signer_fingerprints) + copy_repo_icon(repodir) make_v1(sortedapps, apks, repodir, repodict, requestsdict, signer_fingerprints) make_v2( sortedapps, apks, repodir, repodict, requestsdict, signer_fingerprints, archive @@ -1465,34 +1463,36 @@ def make_v0(apps, apks, repodir, repodict, requestsdict, signer_fingerprints): signindex.config = common.config signindex.sign_jar(signed, use_old_algs=True) - # Copy the repo icon into the repo directory... + +def copy_repo_icon(repodir): icon_dir = os.path.join(repodir, 'icons') - repo_icon = os.path.basename( - common.config.get('repo_icon', common.default_config['repo_icon']) - ) + repo_icon = common.config.get('repo_icon', common.default_config['repo_icon']) icon_path = os.path.join(icon_dir, repo_icon) if not os.path.exists(icon_path): - logging.warning( - _( - 'repo_icon "{repo_icon}" does not exist in "{icon_dir}", generating placeholder.' - ).format(repo_icon=repo_icon, icon_dir=icon_dir) - ) os.makedirs(icon_dir, exist_ok=True) - try: - import qrcode - - qrcode.make(common.config['repo_url']).save(icon_path) - except Exception as e: - if isinstance(e, ModuleNotFoundError): - logging.error( - _( - 'The "qrcode" Python package is not installed (e.g. apt-get install python3-qrcode)!' - ) - ) - exampleicon = os.path.join( - common.get_examples_dir(), common.default_config['repo_icon'] + if os.path.exists(repo_icon): + shutil.copy(repo_icon, icon_path) + else: + logging.warning( + _( + 'repo_icon "{repo_icon}" does not exist in "{icon_dir}", generating placeholder.' + ).format(repo_icon=repo_icon, icon_dir=icon_dir) ) - shutil.copy(exampleicon, icon_path) + try: + import qrcode + + qrcode.make(common.config['repo_url']).save(icon_path) + except Exception as e: + if isinstance(e, ModuleNotFoundError): + logging.error( + _( + 'The "qrcode" Python package is not installed (e.g. apt-get install python3-qrcode)!' + ) + ) + exampleicon = os.path.join( + common.get_examples_dir(), common.default_config['repo_icon'] + ) + shutil.copy(exampleicon, icon_path) def extract_pubkey(): diff --git a/fdroidserver/lint.py b/fdroidserver/lint.py index 65326e48..66bc6505 100644 --- a/fdroidserver/lint.py +++ b/fdroidserver/lint.py @@ -1691,7 +1691,6 @@ CHECK_CONFIG_KEYS = ( 'apk_signing_key_block_list', 'archive', 'archive_description', - 'archive_icon', 'archive_name', 'archive_older', 'archive_url', diff --git a/fdroidserver/update.py b/fdroidserver/update.py index 1569a640..d37c36b7 100644 --- a/fdroidserver/update.py +++ b/fdroidserver/update.py @@ -2796,17 +2796,17 @@ def main(): options.clean = True # check that icons exist now, rather than fail at the end of `fdroid update` - for k in ['repo_icon', 'archive_icon']: - if k in config: - icon_dir = os.path.join(k.split('_')[0], 'icons') - repo_icon = os.path.basename(common.config.get(k, '')) - icon_path = os.path.join(icon_dir, repo_icon) - if not os.path.exists(icon_path): - logging.warning( - _( - '{name} "{repo_icon}" does not exist in "{icon_dir}/"! Check "config.yml".' - ).format(name=k, icon_dir=icon_dir, repo_icon=repo_icon) - ) + icon_key = 'repo_icon' + if icon_key in config: + icon_dir = os.path.join(repodirs[0], 'icons') + repo_icon = os.path.basename(config.get(icon_key, '')) + icon_path = os.path.join(icon_dir, repo_icon) + if not os.path.exists(icon_path): + logging.warning( + _( + '{name} "{repo_icon}" does not exist in "{icon_dir}/"! Check "config.yml".' + ).format(name=icon_key, icon_dir=icon_dir, repo_icon=repo_icon) + ) # if the user asks to create a keystore, do it now, reusing whatever it can if options.create_key: diff --git a/tests/test_common.py b/tests/test_common.py index e4027fa4..558f791e 100755 --- a/tests/test_common.py +++ b/tests/test_common.py @@ -2904,6 +2904,24 @@ class CommonTest(SetUpTearDownMixin, unittest.TestCase): fdroidserver.common.read_config()['serverwebroot'], ) + def test_config_errors_ends_with_repo(self): + os.chdir(self.testdir) + fdroidserver.common.write_config_file("""repo_url: https://foo.com/fdroid""") + with self.assertRaises(FDroidException): + fdroidserver.common.read_config() + + def test_config_errors_ends_with_archive(self): + os.chdir(self.testdir) + fdroidserver.common.write_config_file("""archive_url: https://foo.com/fdroid/repo""") + with self.assertRaises(FDroidException): + fdroidserver.common.read_config() + + def test_config_errors_repo_icon(self): + os.chdir(self.testdir) + fdroidserver.common.write_config_file("""repo_icon: /path/to/icon.png""") + with self.assertRaises(FDroidException): + fdroidserver.common.read_config() + @mock.patch.dict(os.environ, {'PATH': os.getenv('PATH')}, clear=True) def test_config_serverwebroot_list_of_dicts_env(self): os.chdir(self.testdir) diff --git a/tests/test_index.py b/tests/test_index.py index 49781fa5..a466e4b6 100755 --- a/tests/test_index.py +++ b/tests/test_index.py @@ -414,6 +414,8 @@ class IndexTest(unittest.TestCase): requestsdict = {'install': [], 'uninstall': []} common.config['repo_pubkey'] = 'ffffffffffffffffffffffffffffffffff' index.make_v0({}, [], 'repo', repodict, requestsdict, {}) + with self.assertLogs(): + index.copy_repo_icon('repo') self.assertTrue(os.path.isdir(repo_icons_dir)) self.assertTrue( os.path.exists( @@ -422,6 +424,50 @@ class IndexTest(unittest.TestCase): ) self.assertTrue(os.path.exists(os.path.join('repo', 'index.xml'))) + def test_generate_repo_icon(self): + os.chdir(self.testdir) + os.mkdir('repo') + repo_icons_dir = os.path.join('repo', 'icons') + generated_path = os.path.join( + repo_icons_dir, common.default_config['repo_icon'] + ) + self.assertFalse(os.path.isdir(repo_icons_dir)) + self.assertFalse(os.path.exists(generated_path)) + with self.assertLogs(): + index.copy_repo_icon('repo') + self.assertTrue(os.path.isdir(repo_icons_dir)) + self.assertTrue(os.path.exists(generated_path)) + + def test_copy_repo_icon(self): + os.chdir(self.testdir) + os.mkdir('repo') + repo_icons_dir = Path('repo/icons') + self.assertFalse(os.path.isdir(repo_icons_dir)) + common.config['repo_icon'] = 'test_icon.png' + test_value = 'test' + Path(common.config['repo_icon']).write_text(test_value) + copied_path = repo_icons_dir / os.path.basename(common.config['repo_icon']) + self.assertFalse(os.path.isdir(repo_icons_dir)) + self.assertFalse(os.path.exists(copied_path)) + index.copy_repo_icon('repo') + self.assertTrue(os.path.isdir(repo_icons_dir)) + self.assertEqual(test_value, copied_path.read_text()) + + def test_copy_repo_icon_skipped_if_exists(self): + os.chdir(self.testdir) + test_value = 'test' + default_repo_icon = common.default_config['repo_icon'] + repo_icon = Path('repo/icons') / default_repo_icon + repo_icon.parent.mkdir(parents=True) + repo_icon.write_text(test_value) + archive_icon = Path('archive/icons') / default_repo_icon + archive_icon.parent.mkdir(parents=True) + archive_icon.write_text(test_value) + Path(default_repo_icon).write_text('this should not be copied') + index.copy_repo_icon('repo') + self.assertEqual(test_value, repo_icon.read_text()) + self.assertEqual(test_value, archive_icon.read_text()) + def test_make_v0(self): os.chdir(self.testdir) os.mkdir('metadata') @@ -477,6 +523,8 @@ class IndexTest(unittest.TestCase): common.config['repo_pubkey'] = 'ffffffffffffffffffffffffffffffffff' common.config['make_current_version_link'] = True index.make_v0(apps, [apk], 'repo', repodict, requestsdict, {}) + with self.assertLogs(): + index.copy_repo_icon('repo') self.assertTrue(os.path.isdir(repo_icons_dir)) self.assertTrue( os.path.exists(