Merge branch 'config-options-as-module-level-variables' into 'master'

clarify that 'config' and 'options' should only be module-level variables

See merge request fdroid/fdroidserver!1477

.gitlab-ci.yml

@@ -362,7 +362,7 @@ macOS:
     - /bin/bash -n gradlew-fdroid tests/run-tests
 
     # TODO remove the packages below once they are included in the Homebrew package
-    - $(brew --prefix fdroidserver)/libexec/bin/python3 -m pip install biplist pycountry
+    - $(brew --prefix fdroidserver)/libexec/bin/python3 -m pip install biplist oscrypto pycountry
 
     # test fdroidserver from git with current package's dependencies
     - fdroid="$(brew --prefix fdroidserver)/libexec/bin/python3 $PWD/fdroid" ./tests/run-tests

examples/fdroid_clean_repos.py

@@ -23,12 +23,11 @@ def main():
         help=_("applicationId with optional versionCode in the form APPID[:VERCODE]"),
     )
     metadata.add_metadata_arguments(parser)
-    options = parser.parse_args()
+    options = common.parse_args(parser)
-    common.options = options
     pkgs = common.read_pkg_args(options.appid, True)
     allapps = metadata.read_metadata(pkgs)
     apps = common.read_app_args(options.appid, allapps, True)
-    common.read_config(options)
+    common.read_config()
 
     for appid, app in apps.items():
         if "Builds" in app and len(app["Builds"]) > 0:

examples/fdroid_export_keystore_to_nitrokey.py

@@ -25,8 +25,8 @@ def main():
     global config
     parser = ArgumentParser()
     common.setup_global_opts(parser)
-    options = parser.parse_args()
+    common.parse_args(parser)
-    config = common.read_config(options)
+    config = common.read_config()
     destkeystore = config['keystore'].replace('.jks', '.p12').replace('/', '_')
     exportkeystore = config['keystore'].replace('.jks', '.pem').replace('/', '_')
     if os.path.exists(destkeystore) or os.path.exists(exportkeystore):

examples/fdroid_exportkeystore.py

@@ -14,8 +14,8 @@ fdroid_summary = 'export the keystore in standard PEM format'
 def main():
     parser = ArgumentParser()
     common.setup_global_opts(parser)
-    options = parser.parse_args()
+    common.parse_args(parser)
-    config = common.read_config(options)
+    config = common.read_config()
     env_vars = {'LC_ALL': 'C.UTF-8',
                 'FDROID_KEY_STORE_PASS': config['keystorepass'],
                 'FDROID_KEY_PASS': config['keypass']}

examples/fdroid_extract_repo_pubkey.py

@@ -12,8 +12,8 @@ fdroid_summary = 'export the keystore in standard PEM format'
 def main():
     parser = ArgumentParser()
     common.setup_global_opts(parser)
-    options = parser.parse_args()
+    common.parse_args(parser)
-    common.config = common.read_config(options)
+    common.read_config()
     pubkey, repo_pubkey_fingerprint = index.extract_pubkey()
     print('repo_pubkey = "%s"' % pubkey.decode())
 

examples/fdroid_fetchsrclibs.py

@@ -18,12 +18,11 @@ def main():
     common.setup_global_opts(parser)
     parser.add_argument("appid", nargs='*', help=_("applicationId with optional versionCode in the form APPID[:VERCODE]"))
     metadata.add_metadata_arguments(parser)
-    options = parser.parse_args()
+    options = common.parse_args(parser)
-    common.options = options
     pkgs = common.read_pkg_args(options.appid, True)
     allapps = metadata.read_metadata(pkgs)
     apps = common.read_app_args(options.appid, allapps, True)
-    common.read_config(options)
+    common.read_config()
     srclib_dir = os.path.join('build', 'srclib')
     os.makedirs(srclib_dir, exist_ok=True)
     srclibpaths = []

examples/fdroid_nitrokeyimport.py

@@ -11,8 +11,8 @@ fdroid_summary = 'import the local keystore into a SmartCard HSM'
 def main():
     parser = ArgumentParser()
     common.setup_global_opts(parser)
-    options = parser.parse_args()
+    common.parse_args(parser)
-    config = common.read_config(options)
+    config = common.read_config()
     env_vars = {
         'LC_ALL': 'C.UTF-8',
         'FDROID_KEY_STORE_PASS': config['keystorepass'],

fdroidserver/__main__.py

@@ -182,7 +182,7 @@ def main():
                            "can not be specified at the same time."))
         sys.exit(1)
 
-    # Trick optparse into displaying the right usage when --help is used.
+    # Trick argparse into displaying the right usage when --help is used.
     sys.argv[0] += ' ' + command
 
     del sys.argv[1]

fdroidserver/btlog.py

@@ -47,9 +47,6 @@ from . import deploy
 from .exception import FDroidException
 
 
-options = None
-
-
 def make_binary_transparency_log(
     repodirs: collections.abc.Iterable,
     btrepo: str = 'binary_transparency',
@@ -175,9 +172,8 @@ def main():
     ------
     :exc:`~fdroidserver.exception.FDroidException`
         If the specified or default Git repository does not exist.
-    """
-    global options
 
+    """
     parser = ArgumentParser()
     common.setup_global_opts(parser)
     parser.add_argument(
@@ -196,7 +192,7 @@ def main():
         default=None,
         help=_("Push the log to this git remote repository"),
     )
-    options = parser.parse_args()
+    options = common.parse_args(parser)
 
     if options.verbose:
         logging.getLogger("requests").setLevel(logging.INFO)

fdroidserver/build.py

@@ -1014,7 +1014,7 @@ def parse_commandline():
     parser.add_argument("-w", "--wiki", default=False, action="store_true",
                         help=argparse.SUPPRESS)
     metadata.add_metadata_arguments(parser)
-    options = parser.parse_args()
+    options = common.parse_args(parser)
     metadata.warnings_action = options.W
 
     # Force --stop with --on-server to get correct exit code
@@ -1076,7 +1076,7 @@ def main():
         if not options.appid and not options.all:
             parser.error("option %s: If you really want to build all the apps, use --all" % "all")
 
-    config = common.read_config(opts=options)
+    config = common.read_config()
 
     if config['build_server_always']:
         options.server = True

fdroidserver/checkupdates.py

@@ -506,7 +506,7 @@ def operate_vercode(operation: str, vercode: int) -> int:
     return vercode
 
 
-def checkupdates_app(app: metadata.App) -> None:
+def checkupdates_app(app: metadata.App, auto: bool, commit: bool = False) -> None:
     """Check for new versions and updated name of a single app.
 
     Also write back changes to the metadata file and create a Git commit if
@@ -582,7 +582,7 @@ def checkupdates_app(app: metadata.App) -> None:
         logging.info('...updating to version %s' % ver)
         commitmsg = 'Update CurrentVersion of %s to %s' % (name, ver)
 
-    if options.auto:
+    if auto:
         mode = app.AutoUpdateMode
         if not app.CurrentVersionCode:
             raise MetaDataException(
@@ -665,7 +665,7 @@ def checkupdates_app(app: metadata.App) -> None:
 
     if commitmsg:
         metadata.write_metadata(app.metadatapath, app)
-        if options.commit:
+        if commit:
             logging.info("Commiting update for " + app.metadatapath)
             gitcmd = ["git", "commit", "-m", commitmsg]
             if 'auto_author' in config:
@@ -695,7 +695,6 @@ def status_update_json(processed: list, failed: dict) -> None:
 
 
 config = None
-options = None
 start_timestamp = time.gmtime()
 
 
@@ -705,7 +704,7 @@ def main():
     The behaviour of this function is influenced by the configuration file as
     well as command line parameters.
     """
-    global config, options
+    global config
 
     # Parse command line...
     parser = ArgumentParser()
@@ -720,10 +719,10 @@ def main():
     parser.add_argument("--allow-dirty", action="store_true", default=False,
                         help=_("Run on git repo that has uncommitted changes"))
     metadata.add_metadata_arguments(parser)
-    options = parser.parse_args()
+    options = common.parse_args(parser)
     metadata.warnings_action = options.W
 
-    config = common.read_config(options)
+    config = common.read_config()
 
     if not options.allow_dirty:
         status = subprocess.check_output(['git', 'status', '--porcelain'])
@@ -749,7 +748,7 @@ def main():
         logging.info(msg)
 
         try:
-            checkupdates_app(app)
+            checkupdates_app(app, options.auto, options.commit)
             processed.append(appid)
         except Exception as e:
             msg = _("...checkupdate failed for {appid} : {error}").format(appid=appid, error=e)

fdroidserver/common.py

@@ -191,6 +191,34 @@ default_config = {
 }
 
 
+def get_options():
+    """Return options as set up by parse_args().
+
+    This provides an easy way to get the global instance without
+    having to think about very confusing import and submodule
+    visibility.  The code should be probably refactored so it does not
+    need this.  If each individual option value was always passed to
+    functions as args, for example.
+
+    https://docs.python.org/3/reference/import.html#submodules
+
+    """
+    return fdroidserver.common.options
+
+
+def parse_args(parser):
+    """Call parser.parse_args(), store result in module-level variable and return it.
+
+    This is needed to set up the copy of the options instance in the
+    fdroidserver.common module.  A subcommand only needs to call this
+    if it uses functions from fdroidserver.common that expect the
+    "options" variable to be initialized.
+
+    """
+    fdroidserver.common.options = parser.parse_args()
+    return fdroidserver.common.options
+
+
 def setup_global_opts(parser):
     try:  # the buildserver VM might not have PIL installed
         from PIL import PngImagePlugin
@@ -373,20 +401,38 @@ def fill_config_defaults(thisconfig):
         thisconfig['gradle_version_dir'] = str(Path(thisconfig['cachedir']) / 'gradle')
 
 
-def get_config(opts=None):
+def get_config():
-    """Get config instace. This function takes care of initializing config data before returning it."""
+    """Get the initalized, singleton config instance.
-    global config, options
+
+    config and options are intertwined in read_config(), so they have
+    to be here too.  In the current ugly state of things, there are
+    multiple potential instances of config and options in use:
+
+    * global
+    * module-level in the subcommand module (e.g. fdroidserver/build.py)
+    * module-level in fdroidserver.common
+
+    There are some insane parts of the code that are probably
+    referring to multiple instances of these at different points.
+    This can be super confusing and maddening.
+
+    The current intermediate refactoring step is to move all
+    subcommands to always get/set config and options via this function
+    so that there is no longer a distinction between the global and
+    module-level instances.  Then there can be only one module-level
+    instance in fdroidserver.common.
+
+    """
+    global config
 
     if config is not None:
         return config
 
-    common.read_config(opts=opts)
+    read_config()
 
     # make sure these values are available in common.py even if they didn't
     # declare global in a scope
     common.config = config
-    if opts is not None:
-        common.options = opts
 
     return config
 
@@ -419,7 +465,7 @@ def config_type_check(path, data):
         )
 
 
-def read_config(opts=None):
+def read_config():
     """Read the repository config.
 
     The config is read from config_file, which is in the current
@@ -438,13 +484,11 @@ def read_config(opts=None):
     in git, it makes sense to use a globally standard encoding.
 
     """
-    global config, options
+    global config
 
     if config is not None:
         return config
 
-    options = opts
-
     config = {}
     config_file = 'config.yml'
     old_config_file = 'config.py'
@@ -477,8 +521,8 @@ def read_config(opts=None):
     # smartcardoptions must be a list since its command line args for Popen
     smartcardoptions = config.get('smartcardoptions')
     if isinstance(smartcardoptions, str):
-        options = re.sub(r'\s+', r' ', config['smartcardoptions']).split(' ')
+        sco_items = re.sub(r'\s+', r' ', config['smartcardoptions']).split(' ')
-        config['smartcardoptions'] = [i.strip() for i in options if i]
+        config['smartcardoptions'] = [i.strip() for i in sco_items if i]
     elif not smartcardoptions and 'keystore' in config and config['keystore'] == 'NONE':
         # keystore='NONE' means use smartcard, these are required defaults
         config['smartcardoptions'] = ['-storetype', 'PKCS11', '-providerName',
@@ -570,7 +614,7 @@ def parse_mirrors_config(mirrors):
 def file_entry(filename, hash_value=None):
     meta = {}
     meta["name"] = "/" + Path(filename).as_posix().split("/", 1)[1]
-    meta["sha256"] = hash_value or common.sha256sum(filename)
+    meta["sha256"] = hash_value or sha256sum(filename)
     meta["size"] = os.stat(filename).st_size
     return meta
 

fdroidserver/deploy.py

@@ -36,7 +36,6 @@ from . import index
 from .exception import FDroidException
 
 config = None
-options = None
 start_timestamp = time.gmtime()
 
 GIT_BRANCH = 'master'
@@ -148,9 +147,10 @@ def update_awsbucket_s3cmd(repo_section):
             raise FDroidException()
 
     s3cmd_sync = s3cmd + ['sync', '--acl-public']
-    if options.verbose:
+    options = common.get_options()
+    if options and options.verbose:
         s3cmd_sync += ['--verbose']
-    if options.quiet:
+    if options and options.quiet:
         s3cmd_sync += ['--quiet']
 
     s3url = s3bucketurl + '/fdroid/'
@@ -312,6 +312,7 @@ def update_serverwebroot(serverwebroot, repo_section):
             _('rsync is missing or broken: {error}').format(error=e)
         ) from e
     rsyncargs = ['rsync', '--archive', '--delete-after', '--safe-links']
+    options = common.get_options()
     if not options or not options.no_checksum:
         rsyncargs.append('--checksum')
     if options and options.verbose:
@@ -387,7 +388,7 @@ def sync_from_localcopy(repo_section, local_copy_dir):
     # trailing slashes have a meaning in rsync which is not needed here, so
     # make sure both paths have exactly one trailing slash
     common.local_rsync(
-        options,
+        common.get_options(),
         os.path.join(local_copy_dir, repo_section).rstrip('/') + '/',
         repo_section.rstrip('/') + '/',
     )
@@ -407,7 +408,7 @@ def update_localcopy(repo_section, local_copy_dir):
 
     """
     # local_copy_dir is guaranteed to have a trailing slash in main() below
-    common.local_rsync(options, repo_section, local_copy_dir)
+    common.local_rsync(common.get_options(), repo_section, local_copy_dir)
 
     offline_copy = os.path.join(os.getcwd(), BINARY_TRANSPARENCY_DIR)
     if os.path.isdir(os.path.join(offline_copy, '.git')):
@@ -446,6 +447,8 @@ def update_servergitmirrors(servergitmirrors, repo_section):
         )
         return
 
+    options = common.get_options()
+
     # right now we support only 'repo' git-mirroring
     if repo_section == 'repo':
         git_mirror_path = 'git-mirror'
@@ -595,7 +598,7 @@ def upload_to_android_observatory(repo_section):
 
     requests  # stop unused import warning
 
-    if options.verbose:
+    if common.get_options().verbose:
         logging.getLogger("requests").setLevel(logging.INFO)
         logging.getLogger("urllib3").setLevel(logging.INFO)
     else:
@@ -849,7 +852,7 @@ def push_binary_transparency(git_repo_path, git_remote):
 
 
 def main():
-    global config, options
+    global config
 
     parser = ArgumentParser()
     common.setup_global_opts(parser)
@@ -876,8 +879,8 @@ def main():
         default=False,
         help=_("If a git mirror gets to big, allow the archive to be deleted"),
     )
-    options = parser.parse_args()
+    options = common.parse_args(parser)
-    config = common.read_config(options)
+    config = common.read_config()
 
     if config.get('nonstandardwebroot') is True:
         standardwebroot = False

fdroidserver/gpgsign.py

@@ -28,7 +28,6 @@ from .common import FDroidPopen
 from .exception import FDroidException
 
 config = None
-options = None
 start_timestamp = time.gmtime()
 
 
@@ -42,14 +41,14 @@ def status_update_json(signed):
 
 
 def main():
-    global config, options
+    global config
 
     # Parse command line...
     parser = ArgumentParser()
     common.setup_global_opts(parser)
-    options = parser.parse_args()
+    common.parse_args(parser)
 
-    config = common.read_config(options)
+    config = common.read_config()
 
     repodirs = ['repo']
     if config['archive_older'] != 0:

fdroidserver/import_subcommand.py

@@ -45,7 +45,6 @@ from .exception import FDroidException
 
 
 config = None
-options = None
 
 
 def handle_retree_error_on_windows(function, path, excinfo):
@@ -55,7 +54,7 @@ def handle_retree_error_on_windows(function, path, excinfo):
         function(path)
 
 
-def clone_to_tmp_dir(app: metadata.App) -> Path:
+def clone_to_tmp_dir(app: metadata.App, rev=None) -> Path:
     """Clone the source repository of an app to a temporary directory for further processing.
 
     Parameters
@@ -67,6 +66,7 @@ def clone_to_tmp_dir(app: metadata.App) -> Path:
     -------
     tmp_dir
         The (temporary) directory the apps source has been cloned into.
+
     """
     tmp_dir = Path('tmp')
     tmp_dir.mkdir(exist_ok=True)
@@ -76,7 +76,7 @@ def clone_to_tmp_dir(app: metadata.App) -> Path:
     if tmp_dir.exists():
         shutil.rmtree(str(tmp_dir), onerror=handle_retree_error_on_windows)
     vcs = common.getvcs(app.RepoType, app.Repo, tmp_dir)
-    vcs.gotorevision(options.rev)
+    vcs.gotorevision(rev)
 
     return tmp_dir
 
@@ -236,8 +236,9 @@ def main():
         the current directory is not a Git repository, no application ID could
         be found, no Gradle project could be found or there is already metadata
         for the found application ID.
+
     """
-    global config, options
+    global config
 
     # Parse command line...
     parser = ArgumentParser()
@@ -255,10 +256,10 @@ def main():
     parser.add_argument("--rev", default=None,
                         help=_("Allows a different revision (or git branch) to be specified for the initial import"))
     metadata.add_metadata_arguments(parser)
-    options = parser.parse_args()
+    options = common.parse_args(parser)
     metadata.warnings_action = options.W
 
-    config = common.read_config(options)
+    config = common.read_config()
 
     apps = metadata.read_metadata()
     app = None
@@ -289,7 +290,7 @@ def main():
         write_local_file = True
     elif options.url:
         app = get_app_from_url(options.url)
-        tmp_importer_dir = clone_to_tmp_dir(app)
+        tmp_importer_dir = clone_to_tmp_dir(app, options.rev)
         git_repo = git.Repo(tmp_importer_dir)
 
         if not options.omit_disable:

fdroidserver/init.py

@@ -32,7 +32,6 @@ from . import common
 from .exception import FDroidException
 
 config = {}
-options = None
 
 
 def disable_in_config(key, value):
@@ -49,7 +48,7 @@ def disable_in_config(key, value):
 
 
 def main():
-    global options, config
+    global config
 
     # Parse command line...
     parser = ArgumentParser()
@@ -81,7 +80,7 @@ def main():
         default=False,
         help=_("Do not prompt for Android SDK path, just fail"),
     )
-    options = parser.parse_args()
+    options = common.parse_args(parser)
 
     common.set_console_logging(options.verbose)
 
@@ -171,7 +170,7 @@ def main():
         raise FDroidException('Repository already exists.')
 
     # now that we have a local config.yml, read configuration...
-    config = common.read_config(options)
+    config = common.read_config()
 
     # the NDK is optional and there may be multiple versions of it, so it's
     # left for the user to configure

fdroidserver/install.py

@@ -28,7 +28,6 @@ from . import common
 from .common import SdkToolsPopen
 from .exception import FDroidException
 
-options = None
 config = None
 
 
@@ -44,7 +43,7 @@ def devices():
 
 
 def main():
-    global options, config
+    global config
 
     # Parse command line...
     parser = ArgumentParser(
@@ -63,7 +62,7 @@ def main():
         default=False,
         help=_("Install all signed applications available"),
     )
-    options = parser.parse_args()
+    options = common.parse_args(parser)
 
     common.set_console_logging(options.verbose)
 
@@ -73,7 +72,7 @@ def main():
             % "all"
         )
 
-    config = common.read_config(options)
+    config = common.read_config()
 
     output_dir = 'repo'
     if not os.path.isdir(output_dir):

fdroidserver/lint.py

@@ -31,7 +31,6 @@ from . import metadata
 from . import rewritemeta
 
 config = None
-options = None
 
 
 def enforce_https(domain):
@@ -503,7 +502,7 @@ def check_files_dir(app):
 
 
 def check_format(app):
-    if options.format and not rewritemeta.proper_format(app):
+    if common.options.format and not rewritemeta.proper_format(app):
         yield _("Run rewritemeta to fix formatting")
 
 
@@ -787,7 +786,7 @@ def lint_config(arg):
 
 
 def main():
-    global config, options
+    global config
 
     # Parse command line...
     parser = ArgumentParser()
@@ -812,10 +811,10 @@ def main():
         "appid", nargs='*', help=_("application ID of file to operate on")
     )
     metadata.add_metadata_arguments(parser)
-    options = parser.parse_args()
+    options = common.parse_args(parser)
     metadata.warnings_action = options.W
 
-    config = common.read_config(options)
+    config = common.read_config()
     load_antiFeatures_config()
     load_categories_config()
 

fdroidserver/mirror.py

@@ -15,11 +15,9 @@ from . import common
 from . import index
 from . import update
 
-options = None
 
-
+def _run_wget(path, urls, verbose=False):
-def _run_wget(path, urls):
+    if verbose:
-    if options.verbose:
         verbose = '--verbose'
     else:
         verbose = '--no-verbose'
@@ -48,8 +46,6 @@ def _run_wget(path, urls):
 
 
 def main():
-    global options
-
     parser = ArgumentParser()
     common.setup_global_opts(parser)
     parser.add_argument(
@@ -93,7 +89,7 @@ def main():
     parser.add_argument(
         "--output-dir", default=None, help=_("The directory to write the mirror to")
     )
-    options = parser.parse_args()
+    options = common.parse_args(parser)
 
     common.set_console_logging(options.verbose)
 
@@ -119,7 +115,7 @@ def main():
         )
 
     if fingerprint:
-        config = common.read_config(options)
+        config = common.read_config()
         if not ('jarsigner' in config or 'apksigner' in config):
             logging.error(
                 _('Java JDK not found! Install in standard location or set java_paths!')
@@ -229,7 +225,7 @@ def main():
                                 _append_to_url_path(section, f[:-4] + '.log.gz')
                             )
 
-        _run_wget(sectiondir, urls)
+        _run_wget(sectiondir, urls, options.verbose)
 
         for app in data['apps']:
             localized = app.get('localized')
@@ -242,7 +238,7 @@ def main():
                         if f:
                             filepath_tuple = components + (f,)
                             urls.append(_append_to_url_path(*filepath_tuple))
-                    _run_wget(os.path.join(basedir, *components), urls)
+                    _run_wget(os.path.join(basedir, *components), urls, options.verbose)
                     for k in update.SCREENSHOT_DIRS:
                         urls = []
                         filelist = d.get(k)
@@ -251,7 +247,11 @@ def main():
                             for f in filelist:
                                 filepath_tuple = components + (f,)
                                 urls.append(_append_to_url_path(*filepath_tuple))
-                            _run_wget(os.path.join(basedir, *components), urls)
+                            _run_wget(
+                                os.path.join(basedir, *components),
+                                urls,
+                                options.verbose,
+                            )
 
         urls = dict()
         for app in data['apps']:
@@ -269,7 +269,11 @@ def main():
 
         for icondir in icondirs:
             if icondir in urls:
-                _run_wget(os.path.join(basedir, section, icondir), urls[icondir])
+                _run_wget(
+                    os.path.join(basedir, section, icondir),
+                    urls[icondir],
+                    options.verbose,
+                )
 
 
 if __name__ == "__main__":

fdroidserver/nightly.py

@@ -259,8 +259,7 @@ def main():
         help=_("Set maximum releases in repo before older ones are archived"),
     )
     # TODO add --with-btlog
-    options = parser.parse_args()
+    options = common.parse_args(parser)
-    common.options = options
 
     # force a tighter umask since this writes private key material
     umask = os.umask(0o077)
@@ -428,7 +427,7 @@ Last updated: {date}'''.format(repo_git_base=repo_git_base,
         with open('config.yml', 'w') as fp:
             yaml.dump(config, fp, default_flow_style=False)
         os.chmod('config.yml', 0o600)
-        config = common.read_config(options)
+        config = common.read_config()
         common.assert_config_keystore(config)
 
         for root, dirs, files in os.walk(cibase):

fdroidserver/publish.py

@@ -49,7 +49,6 @@ from .common import FDroidPopen
 from .exception import BuildException, FDroidException
 
 config = None
-options = None
 start_timestamp = time.gmtime()
 
 
@@ -269,7 +268,7 @@ def create_key_if_not_existing(keyalias):
 
 
 def main():
-    global config, options
+    global config
 
     # Parse command line...
     parser = ArgumentParser(
@@ -289,10 +288,10 @@ def main():
         help=_("application ID with optional versionCode in the form APPID[:VERCODE]"),
     )
     metadata.add_metadata_arguments(parser)
-    options = parser.parse_args()
+    options = common.parse_args(parser)
     metadata.warnings_action = options.W
 
-    config = common.read_config(options)
+    config = common.read_config()
 
     if not ('jarsigner' in config and 'keytool' in config):
         logging.critical(

fdroidserver/readmeta.py

@@ -20,8 +20,6 @@ from argparse import ArgumentParser
 from . import common
 from . import metadata
 
-options = None
-
 
 def main():
     parser = ArgumentParser()
@@ -29,7 +27,7 @@ def main():
     metadata.add_metadata_arguments(parser)
     options = parser.parse_args()
     metadata.warnings_action = options.W
-    common.read_config(None)
+    common.read_config()
 
     metadata.read_metadata()
 

fdroidserver/rewritemeta.py

@@ -29,7 +29,6 @@ from . import common
 from . import metadata
 
 config = None
-options = None
 
 
 def proper_format(app):
@@ -62,7 +61,7 @@ def remove_blank_flags_from_builds(builds):
 
 
 def main():
-    global config, options
+    global config
 
     parser = ArgumentParser()
     common.setup_global_opts(parser)
@@ -77,10 +76,10 @@ def main():
         "appid", nargs='*', help=_("application ID of file to operate on")
     )
     metadata.add_metadata_arguments(parser)
-    options = parser.parse_args()
+    options = common.parse_args(parser)
     metadata.warnings_action = options.W
 
-    config = common.read_config(options)
+    config = common.read_config()
 
     # Get all apps...
     allapps = metadata.read_metadata(options.appid)

fdroidserver/scanner.py

@@ -40,8 +40,6 @@ from . import metadata
 from .exception import BuildException, VCSException, ConfigurationException
 from . import scanner
 
-options = None
-
 
 @dataclass
 class MessageStore:
@@ -332,8 +330,9 @@ class ScannerTool:
 
         self.scanner_data_lookup()
 
-        config = common.get_config()
+        options = common.get_options()
-        if (options and options.refresh_scanner) or config.get('refresh_scanner'):
+        options_refresh_scanner = options and options.refresh_scanner
+        if options_refresh_scanner or common.get_config().get('refresh_scanner'):
             self.refresh()
 
         self.load()
@@ -589,6 +588,7 @@ def scan_source(build_dir, build=metadata.Build(), json_per_build=None):
         -------
         0 if the problem was ignored/deleted/is only a warning, 1 otherwise
         """
+        options = common.get_options()
         if toignore(path_in_build_dir):
             return ignoreproblem(what, path_in_build_dir, json_per_build)
         if todelete(path_in_build_dir):
@@ -776,9 +776,6 @@ def scan_source(build_dir, build=metadata.Build(), json_per_build=None):
 
 
 def main():
-    global options
-
-    # Parse command line...
     parser = ArgumentParser(
         usage="%(prog)s [options] [(APPID[:VERCODE] | path/to.apk) ...]"
     )
@@ -793,7 +790,7 @@ def main():
     parser.add_argument("-e", "--exit-code", action="store_true", default=False,
                         help=_("Exit with a non-zero code if problems were found"))
     metadata.add_metadata_arguments(parser)
-    options = parser.parse_args()
+    options = common.parse_args(parser)
     metadata.warnings_action = options.W
 
     json_output = dict()
@@ -804,7 +801,7 @@ def main():
             logging.getLogger().setLevel(logging.ERROR)
 
     # initialize/load configuration values
-    common.get_config(opts=options)
+    common.get_config()
 
     probcount = 0
 

fdroidserver/signatures.py

@@ -103,11 +103,8 @@ def main():
         "APK", nargs='*', help=_("signed APK, either a file-path or HTTPS URL.")
     )
     parser.add_argument("--no-check-https", action="store_true", default=False)
-    options = parser.parse_args()
+    options = common.parse_args(parser)
-
     common.set_console_logging(options.verbose)
-
+    common.read_config()
-    # Read config.py...
-    common.read_config(options)
 
     extract(options)

fdroidserver/signindex.py

@@ -29,7 +29,6 @@ from . import metadata
 from .exception import FDroidException
 
 config = None
-options = None
 start_timestamp = time.gmtime()
 
 
@@ -175,13 +174,13 @@ def status_update_json(signed):
 
 
 def main():
-    global config, options
+    global config
 
     parser = ArgumentParser()
     common.setup_global_opts(parser)
-    options = parser.parse_args()
+    common.parse_args(parser)
 
-    config = common.read_config(options)
+    config = common.read_config()
 
     if 'jarsigner' not in config:
         raise FDroidException(

fdroidserver/update.py

@@ -2574,10 +2574,10 @@ def main():
     parser.add_argument("--allow-disabled-algorithms", action="store_true", default=False,
                         help=_("Include APKs that are signed with disabled algorithms like MD5"))
     metadata.add_metadata_arguments(parser)
-    options = parser.parse_args()
+    options = common.parse_args(parser)
     metadata.warnings_action = options.W
 
-    config = common.read_config(options)
+    config = common.read_config()
     common.setup_status_output(start_timestamp)
 
     if not (('jarsigner' in config or 'apksigner' in config)

fdroidserver/verify.py

@@ -30,7 +30,6 @@ from . import common
 from . import net
 from .exception import FDroidException
 
-options = None
 config = None
 
 
@@ -146,7 +145,7 @@ def write_json_report(url, remote_apk, unsigned_apk, compare_result):
 
 
 def main():
-    global options, config
+    global config
 
     # Parse command line...
     parser = ArgumentParser(
@@ -170,9 +169,9 @@ def main():
         default=False,
         help=_("Output JSON report to file named after APK."),
     )
-    options = parser.parse_args()
+    options = common.parse_args(parser)
 
-    config = common.read_config(options)
+    config = common.read_config()
 
     tmp_dir = 'tmp'
     if not os.path.isdir(tmp_dir):

tests/build.TestCase

@@ -4,7 +4,6 @@
 
 import inspect
 import logging
-import optparse
 import os
 import shutil
 import sys
@@ -29,7 +28,7 @@ import fdroidserver.common
 import fdroidserver.metadata
 import fdroidserver.scanner
 import fdroidserver.vmtools
-from testcommon import mkdtemp
+from testcommon import mkdtemp, parse_args_for_test
 
 
 class FakeProcess:
@@ -562,7 +561,7 @@ class BuildTest(unittest.TestCase):
         os.chdir(self.testdir)
         os.mkdir("build")
 
-        config = fdroidserver.common.get_config()
+        config = fdroidserver.common.read_config()
         config['sdk_path'] = os.getenv('ANDROID_HOME')
         config['ndk_paths'] = {'r10d': os.getenv('ANDROID_NDK_HOME')}
         fdroidserver.common.config = config
@@ -1104,15 +1103,17 @@ class BuildTest(unittest.TestCase):
 if __name__ == "__main__":
     os.chdir(os.path.dirname(__file__))
 
-    parser = optparse.OptionParser()
+    import argparse
-    parser.add_option(
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (fdroidserver.common.options, args) = parser.parse_args(['--verbose'])
+    parse_args_for_test(parser, sys.argv)
 
     newSuite = unittest.TestSuite()
     newSuite.addTest(unittest.makeSuite(BuildTest))

tests/checkupdates.TestCase

@@ -3,7 +3,6 @@
 # http://www.drdobbs.com/testing/unit-testing-with-python/240165163
 
 import logging
-import optparse
 import os
 import sys
 import unittest
@@ -30,8 +29,6 @@ class CheckupdatesTest(unittest.TestCase):
         os.chdir(self.basedir)
 
     def test_autoupdatemode_no_suffix(self):
-        fdroidserver.checkupdates.options = mock.Mock()
-        fdroidserver.checkupdates.options.auto = 'bleh'
         fdroidserver.checkupdates.config = {}
 
         app = fdroidserver.metadata.App()
@@ -52,7 +49,7 @@ class CheckupdatesTest(unittest.TestCase):
         ):
             with mock.patch('fdroidserver.metadata.write_metadata', mock.Mock()):
                 with mock.patch('subprocess.call', lambda cmd: 0):
-                    fdroidserver.checkupdates.checkupdates_app(app)
+                    fdroidserver.checkupdates.checkupdates_app(app, auto=True)
 
         build = app['Builds'][-1]
         self.assertEqual(build.versionName, '1.1.9')
@@ -64,15 +61,13 @@ class CheckupdatesTest(unittest.TestCase):
             with mock.patch('fdroidserver.metadata.write_metadata', mock.Mock()):
                 with mock.patch('subprocess.call', lambda cmd: 0):
                     with self.assertRaises(FDroidException):
-                        fdroidserver.checkupdates.checkupdates_app(app)
+                        fdroidserver.checkupdates.checkupdates_app(app, auto=True)
 
         build = app['Builds'][-1]
         self.assertEqual(build.versionName, '1.1.9')
         self.assertEqual(build.commit, '1.1.9')
 
     def test_autoupdatemode_suffix(self):
-        fdroidserver.checkupdates.options = mock.Mock()
-        fdroidserver.checkupdates.options.auto = 'bleh'
         fdroidserver.checkupdates.config = {}
 
         app = fdroidserver.metadata.App()
@@ -93,15 +88,13 @@ class CheckupdatesTest(unittest.TestCase):
         ):
             with mock.patch('fdroidserver.metadata.write_metadata', mock.Mock()):
                 with mock.patch('subprocess.call', lambda cmd: 0):
-                    fdroidserver.checkupdates.checkupdates_app(app)
+                    fdroidserver.checkupdates.checkupdates_app(app, auto=True)
 
         build = app['Builds'][-1]
         self.assertEqual(build.versionName, '1.1.9.10109-fdroid')
         self.assertEqual(build.commit, 'v1.1.9_10109')
 
     def test_autoupdate_multi_variants(self):
-        fdroidserver.checkupdates.options = mock.Mock()
-        fdroidserver.checkupdates.options.auto = 'bleh'
         fdroidserver.checkupdates.config = {}
 
         app = fdroidserver.metadata.App()
@@ -134,7 +127,7 @@ class CheckupdatesTest(unittest.TestCase):
         ):
             with mock.patch('fdroidserver.metadata.write_metadata', mock.Mock()):
                 with mock.patch('subprocess.call', lambda cmd: 0):
-                    fdroidserver.checkupdates.checkupdates_app(app)
+                    fdroidserver.checkupdates.checkupdates_app(app, auto=True)
 
         build = app['Builds'][-2]
         self.assertEqual(build.versionName, '1.1.9')
@@ -150,8 +143,6 @@ class CheckupdatesTest(unittest.TestCase):
         self.assertEqual(app.CurrentVersionCode, 101093)
 
     def test_checkupdates_app_http(self):
-        fdroidserver.checkupdates.options = mock.Mock()
-        fdroidserver.checkupdates.options.auto = 'bleh'
         fdroidserver.checkupdates.config = {}
 
         app = fdroidserver.metadata.App()
@@ -165,7 +156,7 @@ class CheckupdatesTest(unittest.TestCase):
             'fdroidserver.checkupdates.check_http', lambda app: (None, 'bla')
         ):
             with self.assertRaises(FDroidException):
-                fdroidserver.checkupdates.checkupdates_app(app)
+                fdroidserver.checkupdates.checkupdates_app(app, auto=True)
 
         with mock.patch(
             'fdroidserver.checkupdates.check_http', lambda app: ('1.1.9', 10109)
@@ -174,12 +165,10 @@ class CheckupdatesTest(unittest.TestCase):
                 'fdroidserver.metadata.write_metadata', mock.Mock()
             ) as wrmock:
                 with mock.patch('subprocess.call', lambda cmd: 0):
-                    fdroidserver.checkupdates.checkupdates_app(app)
+                    fdroidserver.checkupdates.checkupdates_app(app, auto=True)
                 wrmock.assert_called_with(app.metadatapath, app)
 
     def test_checkupdates_app_tags(self):
-        fdroidserver.checkupdates.options = mock.Mock()
-        fdroidserver.checkupdates.options.auto = 'bleh'
         fdroidserver.checkupdates.config = {}
 
         app = fdroidserver.metadata.App()
@@ -200,7 +189,7 @@ class CheckupdatesTest(unittest.TestCase):
             lambda app, pattern: (None, 'bla', None),
         ):
             with self.assertRaises(FDroidException):
-                fdroidserver.checkupdates.checkupdates_app(app)
+                fdroidserver.checkupdates.checkupdates_app(app, auto=True)
 
         with mock.patch(
             'fdroidserver.checkupdates.check_tags',
@@ -208,15 +197,13 @@ class CheckupdatesTest(unittest.TestCase):
         ):
             with mock.patch('fdroidserver.metadata.write_metadata', mock.Mock()):
                 with mock.patch('subprocess.call', lambda cmd: 0):
-                    fdroidserver.checkupdates.checkupdates_app(app)
+                    fdroidserver.checkupdates.checkupdates_app(app, auto=True)
 
         build = app['Builds'][-1]
         self.assertEqual(build.versionName, '1.1.9')
         self.assertEqual(build.commit, 'v1.1.9')
 
     def test_check_http(self):
-        fdroidserver.checkupdates.options = mock.Mock()
-
         app = fdroidserver.metadata.App()
         app.id = 'loop.starts.shooting'
         app.metadatapath = 'metadata/' + app.id + '.yml'
@@ -243,8 +230,6 @@ class CheckupdatesTest(unittest.TestCase):
                 fdroidserver.checkupdates.check_http(app)
 
     def test_check_http_ignore(self):
-        fdroidserver.checkupdates.options = mock.Mock()
-
         app = fdroidserver.metadata.App()
         app.id = 'loop.starts.shooting'
         app.metadatapath = 'metadata/' + app.id + '.yml'
@@ -260,8 +245,6 @@ class CheckupdatesTest(unittest.TestCase):
         self.assertEqual(vername, None)
 
     def test_check_tags_data(self):
-        fdroidserver.checkupdates.options = mock.Mock()
-
         app = fdroidserver.metadata.App()
         app.id = 'loop.starts.shooting'
         app.metadatapath = 'metadata/' + app.id + '.yml'
@@ -336,15 +319,18 @@ class CheckupdatesTest(unittest.TestCase):
 
 
 if __name__ == "__main__":
-    parser = optparse.OptionParser()
+    import argparse
-    parser.add_option(
+    from testcommon import parse_args_for_test
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (fdroidserver.common.options, args) = parser.parse_args(['--verbose'])
+    parse_args_for_test(parser, sys.argv)
 
     newSuite = unittest.TestSuite()
     newSuite.addTest(unittest.makeSuite(CheckupdatesTest))

tests/common.TestCase

@@ -9,7 +9,6 @@ import importlib
 import inspect
 import json
 import logging
-import optparse
 import os
 import re
 import ruamel.yaml
@@ -22,6 +21,7 @@ import unittest
 import textwrap
 import yaml
 import gzip
+from argparse import ArgumentParser
 from zipfile import BadZipFile, ZipFile
 from unittest import mock
 from pathlib import Path
@@ -38,13 +38,19 @@ import fdroidserver.index
 import fdroidserver.signindex
 import fdroidserver.common
 import fdroidserver.metadata
-from testcommon import TmpCwd, mkdtemp
+from testcommon import TmpCwd, mkdtemp, parse_args_for_test
 from fdroidserver.common import ANTIFEATURES_CONFIG_NAME, CATEGORIES_CONFIG_NAME
 from fdroidserver.exception import FDroidException, VCSException,\
     MetaDataException, VerificationException
 from fdroidserver.looseversion import LooseVersion
 
 
+def _mock_common_module_options_instance():
+    """Helper method to deal with difficult visibility of the module-level options."""
+    fdroidserver.common.options = mock.Mock()
+    fdroidserver.common.options.verbose = False
+
+
 class CommonTest(unittest.TestCase):
     '''fdroidserver/common.py'''
 
@@ -57,17 +63,22 @@ class CommonTest(unittest.TestCase):
         if not os.path.exists(self.tmpdir):
             os.makedirs(self.tmpdir)
         os.chdir(self.basedir)
+
+        # these are declared as None at the top of the module file
         fdroidserver.common.config = None
-        fdroidserver.common.options = mock.Mock()
+        fdroidserver.common.options = None
-        fdroidserver.common.options.verbose = False
         fdroidserver.metadata.srclibs = None
+
         self._td = mkdtemp()
         self.testdir = self._td.name
 
     def tearDown(self):
+        fdroidserver.common.config = None
+        fdroidserver.common.options = None
         os.chdir(self.basedir)
         self._td.cleanup()
-        shutil.rmtree(self.tmpdir)
+        if os.path.exists(self.tmpdir):
+            shutil.rmtree(self.tmpdir)
 
     def test_parse_human_readable_size(self):
         for k, v in (
@@ -356,6 +367,7 @@ class CommonTest(unittest.TestCase):
         config = dict()
         fdroidserver.common.fill_config_defaults(config)
         fdroidserver.common.config = config
+        _mock_common_module_options_instance()
 
         srclibname = 'FakeSrcLib'
         srclib_testdir = os.path.join(self.testdir, 'build', 'srclib')
@@ -397,6 +409,7 @@ class CommonTest(unittest.TestCase):
                                            onserver=True, refresh=False)  # do not clone in this test
 
     def test_prepare_sources_refresh(self):
+        _mock_common_module_options_instance()
         packageName = 'org.fdroid.ci.test.app'
         os.chdir(self.tmpdir)
         os.mkdir('build')
@@ -467,6 +480,7 @@ class CommonTest(unittest.TestCase):
         config = dict()
         fdroidserver.common.fill_config_defaults(config)
         fdroidserver.common.config = config
+        _mock_common_module_options_instance()
 
         commands = ['sh', '-c', 'echo stdout message && echo stderr message 1>&2']
 
@@ -477,7 +491,8 @@ class CommonTest(unittest.TestCase):
         self.assertEqual(p.output, 'stdout message\n')
 
     def test_signjar(self):
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        _mock_common_module_options_instance()
+        config = fdroidserver.common.read_config()
         config['jarsigner'] = fdroidserver.common.find_sdk_tools_cmd('jarsigner')
         fdroidserver.common.config = config
         fdroidserver.signindex.config = config
@@ -497,7 +512,8 @@ class CommonTest(unittest.TestCase):
                 )
 
     def test_verify_apk_signature(self):
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        _mock_common_module_options_instance()
+        config = fdroidserver.common.read_config()
         fdroidserver.common.config = config
 
         self.assertTrue(fdroidserver.common.verify_apk_signature('bad-unicode-πÇÇ现代通用字-български-عربي1.apk'))
@@ -519,7 +535,8 @@ class CommonTest(unittest.TestCase):
         self.assertFalse(fdroidserver.common.verify_apk_signature('urzip-release-unsigned.apk'))
 
     def test_verify_old_apk_signature(self):
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        _mock_common_module_options_instance()
+        config = fdroidserver.common.read_config()
         config['jarsigner'] = fdroidserver.common.find_sdk_tools_cmd('jarsigner')
         fdroidserver.common.config = config
 
@@ -540,7 +557,7 @@ class CommonTest(unittest.TestCase):
 
     def test_verify_jar_signature(self):
         """Sign entry.jar and make sure it validates"""
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        config = fdroidserver.common.read_config()
         config['jarsigner'] = fdroidserver.common.find_sdk_tools_cmd('jarsigner')
         config['keystore'] = os.path.join(self.basedir, 'keystore.jks')
         config['repo_keyalias'] = 'sova'
@@ -558,7 +575,7 @@ class CommonTest(unittest.TestCase):
 
     def test_verify_jar_signature_fails(self):
         """Test verify_jar_signature fails on unsigned and deprecated algorithms"""
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        config = fdroidserver.common.read_config()
         config['jarsigner'] = fdroidserver.common.find_sdk_tools_cmd('jarsigner')
         fdroidserver.common.config = config
         source_dir = os.path.join(self.basedir, 'signindex')
@@ -568,7 +585,7 @@ class CommonTest(unittest.TestCase):
                 fdroidserver.common.verify_jar_signature(testfile)
 
     def test_verify_deprecated_jar_signature(self):
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        config = fdroidserver.common.read_config()
         config['jarsigner'] = fdroidserver.common.find_sdk_tools_cmd('jarsigner')
         fdroidserver.common.config = config
         source_dir = os.path.join(self.basedir, 'signindex')
@@ -581,8 +598,9 @@ class CommonTest(unittest.TestCase):
             fdroidserver.common.verify_deprecated_jar_signature(testfile)
 
     def test_verify_apks(self):
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        config = fdroidserver.common.read_config()
         fdroidserver.common.config = config
+        _mock_common_module_options_instance()
 
         sourceapk = os.path.join(self.basedir, 'urzip.apk')
 
@@ -889,7 +907,8 @@ class CommonTest(unittest.TestCase):
             )
 
     def test_sign_apk(self):
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        _mock_common_module_options_instance()
+        config = fdroidserver.common.read_config()
         if 'apksigner' not in config:
             self.skipTest('SKIPPING test_sign_apk, apksigner not installed!')
 
@@ -959,7 +978,8 @@ class CommonTest(unittest.TestCase):
 
     @unittest.skipIf(os.getuid() == 0, 'This is meaningless when run as root')
     def test_sign_apk_fail(self):
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        _mock_common_module_options_instance()
+        config = fdroidserver.common.read_config()
         if 'apksigner' not in config:
             self.skipTest('SKIPPING test_sign_apk_fail, apksigner not installed!')
 
@@ -982,7 +1002,8 @@ class CommonTest(unittest.TestCase):
         self.assertFalse(os.path.isfile(signed))
 
     def test_sign_apk_corrupt(self):
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        _mock_common_module_options_instance()
+        config = fdroidserver.common.read_config()
         if 'apksigner' not in config:
             self.skipTest('SKIPPING test_sign_apk_corrupt, apksigner not installed!')
 
@@ -1008,7 +1029,8 @@ class CommonTest(unittest.TestCase):
     )
     def test_resign_apk(self):
         """When using apksigner, it should resign signed APKs"""
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        _mock_common_module_options_instance()
+        config = fdroidserver.common.read_config()
         if 'apksigner' not in config:
             self.skipTest('SKIPPING test_resign_apk, apksigner not installed!')
 
@@ -1914,7 +1936,7 @@ class CommonTest(unittest.TestCase):
         os.chdir(self.tmpdir)
         self.assertFalse(os.path.exists('config.yml'))
         self.assertFalse(os.path.exists('config.py'))
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        config = fdroidserver.common.read_config()
         self.assertFalse(config.get('update_stats'))
         self.assertIsNotNone(config.get('char_limits'))
 
@@ -1924,7 +1946,7 @@ class CommonTest(unittest.TestCase):
         open('config.yml', 'w').close()
         self.assertTrue(os.path.exists('config.yml'))
         self.assertFalse(os.path.exists('config.py'))
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        config = fdroidserver.common.read_config()
         self.assertFalse(config.get('update_stats'))
         self.assertIsNotNone(config.get('char_limits'))
 
@@ -1935,7 +1957,7 @@ class CommonTest(unittest.TestCase):
             fp.write('apksigner: yml')
         self.assertTrue(os.path.exists('config.yml'))
         self.assertFalse(os.path.exists('config.py'))
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        config = fdroidserver.common.read_config()
         self.assertEqual('yml', config.get('apksigner'))
 
     def test_with_config_yml_utf8(self):
@@ -1946,7 +1968,7 @@ class CommonTest(unittest.TestCase):
             fp.write('apksigner: ' + teststr)
         self.assertTrue(os.path.exists('config.yml'))
         self.assertFalse(os.path.exists('config.py'))
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        config = fdroidserver.common.read_config()
         self.assertEqual(teststr, config.get('apksigner'))
 
     def test_with_config_yml_utf8_as_ascii(self):
@@ -1957,7 +1979,7 @@ class CommonTest(unittest.TestCase):
             yaml.dump({'apksigner': teststr}, fp)
         self.assertTrue(os.path.exists('config.yml'))
         self.assertFalse(os.path.exists('config.py'))
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        config = fdroidserver.common.read_config()
         self.assertEqual(teststr, config.get('apksigner'))
 
     def test_with_config_yml_with_env_var(self):
@@ -1969,20 +1991,20 @@ class CommonTest(unittest.TestCase):
                 fp.write("""keypass: {'env': 'SECRET'}""")
             self.assertTrue(os.path.exists('config.yml'))
             self.assertFalse(os.path.exists('config.py'))
-            config = fdroidserver.common.read_config(fdroidserver.common.options)
+            config = fdroidserver.common.read_config()
             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)
+            fdroidserver.common.read_config()
 
     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)
+            fdroidserver.common.read_config()
 
     def test_with_config_py(self):
         """Make sure it is still possible to use config.py alone."""
@@ -1991,7 +2013,7 @@ class CommonTest(unittest.TestCase):
             fp.write('apksigner = "py"')
         self.assertFalse(os.path.exists('config.yml'))
         self.assertTrue(os.path.exists('config.py'))
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        config = fdroidserver.common.read_config()
         self.assertEqual("py", config.get('apksigner'))
 
     def test_config_perm_warning(self):
@@ -2001,7 +2023,7 @@ class CommonTest(unittest.TestCase):
             fp.write('keystore: foo.jks')
         self.assertTrue(os.path.exists(fp.name))
         os.chmod(fp.name, 0o666)
-        fdroidserver.common.read_config(fdroidserver.common.options)
+        fdroidserver.common.read_config()
         os.remove(fp.name)
         fdroidserver.common.config = None
 
@@ -2009,7 +2031,7 @@ class CommonTest(unittest.TestCase):
             fp.write('keystore = "foo.jks"')
         self.assertTrue(os.path.exists(fp.name))
         os.chmod(fp.name, 0o666)
-        fdroidserver.common.read_config(fdroidserver.common.options)
+        fdroidserver.common.read_config()
 
     def test_with_both_config_yml_py(self):
         """If config.yml and config.py are present, config.py should be ignored."""
@@ -2020,7 +2042,7 @@ class CommonTest(unittest.TestCase):
             fp.write('apksigner = "py"')
         self.assertTrue(os.path.exists('config.yml'))
         self.assertTrue(os.path.exists('config.py'))
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        config = fdroidserver.common.read_config()
         self.assertEqual('yml', config.get('apksigner'))
 
     def test_config_repo_url(self):
@@ -2071,14 +2093,14 @@ class CommonTest(unittest.TestCase):
             fp.write('apksigner: yml')
         self.assertTrue(os.path.exists(fp.name))
         self.assertFalse(os.path.exists('config.py'))
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        config = fdroidserver.common.read_config()
         self.assertFalse('keypass' in config)
         self.assertEqual('yml', config.get('apksigner'))
         fdroidserver.common.write_to_config(config, 'keypass', 'mysecretpassword')
         with open(fp.name) as fp:
             print(fp.read())
         fdroidserver.common.config = None
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        config = fdroidserver.common.read_config()
         self.assertEqual('mysecretpassword', config['keypass'])
 
     def test_write_to_config_py(self):
@@ -2087,12 +2109,12 @@ class CommonTest(unittest.TestCase):
             fp.write('apksigner = "py"')
         self.assertTrue(os.path.exists(fp.name))
         self.assertFalse(os.path.exists('config.yml'))
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        config = fdroidserver.common.read_config()
         self.assertFalse('keypass' in config)
         self.assertEqual('py', config.get('apksigner'))
         fdroidserver.common.write_to_config(config, 'keypass', 'mysecretpassword')
         fdroidserver.common.config = None
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        config = fdroidserver.common.read_config()
         self.assertEqual('mysecretpassword', config['keypass'])
 
     def test_config_dict_with_int_keys(self):
@@ -2101,7 +2123,7 @@ class CommonTest(unittest.TestCase):
             fp.write('java_paths:\n  8: /usr/lib/jvm/java-8-openjdk\n')
         self.assertTrue(os.path.exists(fp.name))
         self.assertFalse(os.path.exists('config.py'))
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        config = fdroidserver.common.read_config()
         self.assertEqual('/usr/lib/jvm/java-8-openjdk', config.get('java_paths', {}).get('8'))
 
     @mock.patch.dict(os.environ, {'PATH': os.getenv('PATH')}, clear=True)
@@ -2179,7 +2201,7 @@ class CommonTest(unittest.TestCase):
         shutil.copy(os.path.join(self.basedir, '..', 'buildserver', 'config.buildserver.yml'),
                     'config.yml')
         self.assertFalse(os.path.exists('config.py'))
-        fdroidserver.common.read_config(fdroidserver.common.options)
+        fdroidserver.common.read_config()
 
     def test_setup_status_output(self):
         os.chdir(self.tmpdir)
@@ -2532,6 +2554,7 @@ class CommonTest(unittest.TestCase):
 
     @mock.patch.dict(os.environ, clear=True)
     def test_FDroidPopen_envs_paths_can_be_pathlib(self):
+        _mock_common_module_options_instance()
         os.environ['PATH'] = '/usr/bin:/usr/sbin'
         envs = {'PATHLIB': Path('/pathlib/path'), 'STRING': '/string/path'}
         p = fdroidserver.common.FDroidPopen(['/bin/sh', '-c', 'export'], envs=envs)
@@ -3215,18 +3238,104 @@ class SignerExtractionTest(unittest.TestCase):
                 )
 
 
+class ConfigOptionsScopeTest(unittest.TestCase):
+    """Test assumptions about variable scope for "config" and "options".
+
+    The ancient architecture of config and options in fdroidserver has
+    weird issues around unexpected scope, like there are cases where
+    the global config is not the same as the module-level config, and
+    more.
+
+    This is about describing what is happening, it is not about
+    documenting behaviors that are good design. The config and options
+    handling should really be refactored into a well-known, workable
+    Pythonic pattern.
+
+    """
+
+    def setUp(self):
+        # these are declared as None at the top of the module file
+        fdroidserver.common.config = None
+        fdroidserver.common.options = None
+
+    def tearDown(self):
+        fdroidserver.common.config = None
+        fdroidserver.common.options = None
+        if 'config' in globals():
+            global config
+            del config
+        if 'options' in globals():
+            global options
+            del options
+
+    def test_parse_args(self):
+        """Test that options is properly set up at the module-level and not global."""
+        self.assertFalse('options' in globals())
+        self.assertIsNone(fdroidserver.common.options)
+        parser = ArgumentParser()
+        fdroidserver.common.setup_global_opts(parser)
+        with mock.patch('sys.argv', ['$0']):
+            o = fdroidserver.common.parse_args(parser)
+        self.assertEqual(o, fdroidserver.common.options)
+
+        # No function should set options as a global, and the global
+        # keyword does not create the variable.
+        global options
+        with self.assertRaises(NameError):
+            options
+        self.assertFalse('options' in globals())
+
+    def test_parse_args_without_args(self):
+        """Test that the parsing function works fine when there are no args."""
+        parser = ArgumentParser()
+        fdroidserver.common.setup_global_opts(parser)
+        with mock.patch('sys.argv', ['$0']):
+            o = fdroidserver.common.parse_args(parser)
+        self.assertFalse(o.verbose)
+
+    def test_parse_args_with_args(self):
+        parser = ArgumentParser()
+        fdroidserver.common.setup_global_opts(parser)
+        with mock.patch('sys.argv', ['$0', '-v']):
+            o = fdroidserver.common.parse_args(parser)
+        self.assertTrue(o.verbose)
+
+    def test_get_config(self):
+        """Show how the module-level variables are initialized."""
+        self.assertTrue('config' not in vars() and 'config' not in globals())
+        self.assertIsNone(fdroidserver.common.config)
+        config = fdroidserver.common.read_config()
+        self.assertIsNotNone(fdroidserver.common.config)
+        self.assertEqual(dict, type(config))
+        self.assertEqual(config, fdroidserver.common.config)
+
+    def test_get_config_global(self):
+        """Test assumptions about variable scope using global keyword."""
+        global config
+        self.assertTrue('config' not in vars() and 'config' not in globals())
+        self.assertIsNone(fdroidserver.common.config)
+        c = fdroidserver.common.read_config()
+        self.assertIsNotNone(fdroidserver.common.config)
+        self.assertEqual(dict, type(c))
+        self.assertEqual(c, fdroidserver.common.config)
+        self.assertTrue(
+            'config' not in vars() and 'config' not in globals(),
+            "The config should not be set in the global context, only module-level.",
+        )
+
+
 if __name__ == "__main__":
     os.chdir(os.path.dirname(__file__))
 
-    parser = optparse.OptionParser()
+    parser = ArgumentParser()
-    parser.add_option(
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (fdroidserver.common.options, args) = parser.parse_args(['--verbose'])
+    parse_args_for_test(parser, sys.argv)
 
     newSuite = unittest.TestSuite()
     newSuite.addTest(unittest.makeSuite(CommonTest))

tests/deploy.TestCase

@@ -2,7 +2,6 @@
 
 import inspect
 import logging
-import optparse
 import os
 import sys
 import tempfile
@@ -19,7 +18,7 @@ if localmodule not in sys.path:
 import fdroidserver.common
 import fdroidserver.deploy
 from fdroidserver.exception import FDroidException
-from testcommon import TmpCwd, mkdtemp
+from testcommon import TmpCwd, mkdtemp, parse_args_for_test
 
 
 class DeployTest(unittest.TestCase):
@@ -115,11 +114,11 @@ class DeployTest(unittest.TestCase):
         self.maxDiff = None
 
         # setup parameters for this test run
-        fdroidserver.deploy.options = mock.Mock()
+        fdroidserver.common.options = mock.Mock()
-        fdroidserver.deploy.options.no_checksum = True
+        fdroidserver.common.options.no_checksum = True
-        fdroidserver.deploy.options.identity_file = None
+        fdroidserver.common.options.identity_file = None
-        fdroidserver.deploy.options.verbose = False
+        fdroidserver.common.options.verbose = False
-        fdroidserver.deploy.options.quiet = True
+        fdroidserver.common.options.quiet = True
         fdroidserver.deploy.config = {'make_current_version_link': True}
         url = "example.com:/var/www/fdroid"
         repo_section = 'repo'
@@ -208,12 +207,12 @@ class DeployTest(unittest.TestCase):
 
     def test_update_serverwebroot_with_id_file(self):
         # setup parameters for this test run
-        fdroidserver.deploy.options = mock.Mock()
+        fdroidserver.common.options = mock.Mock()
-        fdroidserver.deploy.options.identity_file = None
+        fdroidserver.common.options.identity_file = None
-        fdroidserver.deploy.options.no_checksum = True
+        fdroidserver.common.options.no_checksum = True
-        fdroidserver.deploy.options.verbose = True
+        fdroidserver.common.options.verbose = True
-        fdroidserver.deploy.options.quiet = False
+        fdroidserver.common.options.quiet = False
-        fdroidserver.deploy.options.identity_file = None
+        fdroidserver.common.options.identity_file = None
         fdroidserver.deploy.config = {'identity_file': './id_rsa'}
         url = "example.com:/var/www/fdroid"
         repo_section = 'archive'
@@ -290,7 +289,7 @@ class DeployTest(unittest.TestCase):
         not os.getenv('VIRUSTOTAL_API_KEY'), 'VIRUSTOTAL_API_KEY is not set'
     )
     def test_upload_to_virustotal(self):
-        fdroidserver.deploy.options.verbose = True
+        fdroidserver.common.options.verbose = True
         virustotal_apikey = os.getenv('VIRUSTOTAL_API_KEY')
         fdroidserver.deploy.upload_to_virustotal('repo', virustotal_apikey)
 
@@ -308,12 +307,12 @@ class DeployTest(unittest.TestCase):
 
     def test_update_servergitmirrors(self):
         # setup parameters for this test run
-        fdroidserver.deploy.options = mock.Mock()
+        fdroidserver.common.options = mock.Mock()
-        fdroidserver.deploy.options.identity_file = None
+        fdroidserver.common.options.identity_file = None
-        fdroidserver.deploy.options.no_keep_git_mirror_archive = False
+        fdroidserver.common.options.no_keep_git_mirror_archive = False
-        fdroidserver.deploy.options.verbose = False
+        fdroidserver.common.options.verbose = False
-        fdroidserver.deploy.options.quiet = True
+        fdroidserver.common.options.quiet = True
-        fdroidserver.deploy.options.index_only = False
+        fdroidserver.common.options.index_only = False
 
         config = {}
         fdroidserver.common.fill_config_defaults(config)
@@ -390,15 +389,17 @@ class DeployTest(unittest.TestCase):
 if __name__ == "__main__":
     os.chdir(os.path.dirname(__file__))
 
-    parser = optparse.OptionParser()
+    import argparse
-    parser.add_option(
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (fdroidserver.common.options, args) = parser.parse_args(['--verbose'])
+    parse_args_for_test(parser, sys.argv)
 
     newSuite = unittest.TestSuite()
     newSuite.addTest(unittest.makeSuite(DeployTest))

tests/dump_internal_metadata_format.py

@@ -56,9 +56,9 @@ def _build_yaml_representer(dumper, data):
 parser = ArgumentParser()
 fdroidserver.common.setup_global_opts(parser)
 fdroidserver.metadata.add_metadata_arguments(parser)
-options = parser.parse_args()
+options = fdroidserver.common.parse_args(parser)
 fdroidserver.metadata.warnings_action = options.W
-fdroidserver.common.read_config(None)
+fdroidserver.common.read_config()
 
 if not os.path.isdir('metadata'):
     print("This script must be run in an F-Droid data folder with a 'metadata' subdir!")

tests/exception.TestCase

@@ -3,7 +3,6 @@
 # http://www.drdobbs.com/testing/unit-testing-with-python/240165163
 
 import inspect
-import optparse
 import os
 import sys
 import unittest
@@ -57,16 +56,18 @@ class ExceptionTest(unittest.TestCase):
 if __name__ == "__main__":
     os.chdir(os.path.dirname(__file__))
 
-    parser = optparse.OptionParser()
+    import argparse
-    parser.add_option(
+    from testcommon import parse_args_for_test
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (fdroidserver.exception.options, args) = parser.parse_args(['--verbose'])
+    parse_args_for_test(parser, sys.argv)
-    fdroidserver.common.options = fdroidserver.exception.options
 
     newSuite = unittest.TestSuite()
     newSuite.addTest(unittest.makeSuite(ExceptionTest))

tests/gpgsign.TestCase

@@ -3,7 +3,6 @@
 import inspect
 import json
 import logging
-import optparse
 import os
 import shutil
 import sys
@@ -33,7 +32,7 @@ class GpgsignTest(unittest.TestCase):
         self.repodir.mkdir()
 
         gpgsign.config = None
-        config = common.read_config(common.options)
+        config = common.read_config()
         config['verbose'] = True
         config['gpghome'] = str((self.basedir / 'gnupghome').resolve())
         config['gpgkey'] = '1DBA2E89'
@@ -75,15 +74,17 @@ class GpgsignTest(unittest.TestCase):
 if __name__ == "__main__":
     os.chdir(os.path.dirname(__file__))
 
-    parser = optparse.OptionParser()
+    import argparse
-    parser.add_option(
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (common.options, args) = parser.parse_args(['--verbose'])
+    common.parse_args(parser)
 
     newSuite = unittest.TestSuite()
     newSuite.addTest(unittest.makeSuite(GpgsignTest))

tests/import_subcommand.TestCase

@@ -4,7 +4,6 @@
 
 import git
 import logging
-import optparse
 import os
 import shutil
 import sys
@@ -15,7 +14,6 @@ from unittest import mock
 from pathlib import Path
 
 import requests
-from testcommon import TmpCwd, mkdtemp
 
 localmodule = Path(__file__).resolve().parent.parent
 print('localmodule: ' + str(localmodule))
@@ -26,6 +24,7 @@ import fdroidserver.common
 import fdroidserver.import_subcommand
 import fdroidserver.metadata
 from fdroidserver.exception import FDroidException
+from testcommon import TmpCwd, mkdtemp, parse_args_for_test
 
 
 class ImportTest(unittest.TestCase):
@@ -34,8 +33,6 @@ class ImportTest(unittest.TestCase):
     def setUp(self):
         logging.basicConfig(level=logging.DEBUG)
         self.basedir = localmodule / 'tests'
-        fdroidserver.import_subcommand.options = mock.Mock()
-        fdroidserver.import_subcommand.options.rev = None
         os.chdir(self.basedir)
         self._td = mkdtemp()
         self.testdir = self._td.name
@@ -146,7 +143,9 @@ class ImportTest(unittest.TestCase):
             fdroidserver.import_subcommand.main()
 
     @mock.patch('sys.argv', ['fdroid import', '-u', 'https://fake/git/url.git'])
-    @mock.patch('fdroidserver.import_subcommand.clone_to_tmp_dir', lambda a: Path('td'))
+    @mock.patch(
+        'fdroidserver.import_subcommand.clone_to_tmp_dir', lambda a, r: Path('td')
+    )
     def test_main_local_git(self):
         os.chdir(self.testdir)
         git.Repo.init('td')
@@ -161,15 +160,17 @@ class ImportTest(unittest.TestCase):
 
 
 if __name__ == "__main__":
-    parser = optparse.OptionParser()
+    import argparse
-    parser.add_option(
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (fdroidserver.common.options, args) = parser.parse_args(['--verbose'])
+    parse_args_for_test(parser, sys.argv)
 
     newSuite = unittest.TestSuite()
     newSuite.addTest(unittest.makeSuite(ImportTest))

tests/index.TestCase

@@ -5,7 +5,6 @@ import datetime
 import glob
 import inspect
 import logging
-import optparse
 import os
 import sys
 import unittest
@@ -26,7 +25,7 @@ if localmodule not in sys.path:
 
 import fdroidserver
 from fdroidserver import common, index, publish, signindex, update
-from testcommon import TmpCwd, mkdtemp
+from testcommon import TmpCwd, mkdtemp, parse_args_for_test
 from pathlib import Path
 
 
@@ -56,7 +55,7 @@ class IndexTest(unittest.TestCase):
 
         common.config = None
         common.options = Options
-        config = common.read_config(common.options)
+        config = common.read_config()
         config['jarsigner'] = common.find_sdk_tools_cmd('jarsigner')
         common.config = config
         signindex.config = config
@@ -752,7 +751,7 @@ class IndexTest(unittest.TestCase):
             yaml.dump(c, fp)
         os.system('cat config.yml')
         common.config = None
-        common.read_config(Options)
+        common.read_config()
         repodict = {'address': common.config['repo_url']}
         index.add_mirrors_to_repodict('repo', repodict)
         self.assertEqual(
@@ -942,16 +941,17 @@ class AltstoreIndexTest(unittest.TestCase):
 if __name__ == "__main__":
     os.chdir(os.path.dirname(__file__))
 
-    parser = optparse.OptionParser()
+    import argparse
-    parser.add_option(
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (options, args) = parser.parse_args(["--verbose"])
+    parse_args_for_test(parser, sys.argv)
-    Options.verbose = options.verbose
 
     newSuite = unittest.TestSuite()
     newSuite.addTest(unittest.makeSuite(IndexTest))

tests/init.TestCase

@@ -5,7 +5,6 @@
 import inspect
 import logging
 import os
-import optparse
 import shutil
 import sys
 import unittest
@@ -19,7 +18,7 @@ if localmodule not in sys.path:
     sys.path.insert(0, localmodule)
 
 import fdroidserver.init
-from testcommon import mkdtemp
+from testcommon import mkdtemp, parse_args_for_test
 
 
 class InitTest(unittest.TestCase):
@@ -43,14 +42,14 @@ class InitTest(unittest.TestCase):
             fp.write('keystore: NONE\n')
             fp.write('keypass: mysupersecrets\n')
         os.chmod('config.yml', 0o600)
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        config = fdroidserver.common.read_config()
         self.assertEqual('NONE', config['keystore'])
         self.assertEqual('mysupersecrets', config['keypass'])
         fdroidserver.init.disable_in_config('keypass', 'comment')
         with open(fp.name) as fp:
             self.assertTrue('#keypass:' in fp.read())
         fdroidserver.common.config = None
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        config = fdroidserver.common.read_config()
         self.assertIsNone(config.get('keypass'))
 
     @unittest.skipIf(os.name == 'nt', "calling main() like this hangs on Windows")
@@ -75,15 +74,17 @@ class InitTest(unittest.TestCase):
 if __name__ == "__main__":
     os.chdir(os.path.dirname(__file__))
 
-    parser = optparse.OptionParser()
+    import argparse
-    parser.add_option(
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (fdroidserver.init.options, args) = parser.parse_args(['--verbose'])
+    fdroidserver.init.options = parse_args_for_test(parser, sys.argv)
 
     newSuite = unittest.TestSuite()
     newSuite.addTest(unittest.makeSuite(InitTest))

tests/install.TestCase

@@ -3,7 +3,6 @@
 # http://www.drdobbs.com/testing/unit-testing-with-python/240165163
 
 import inspect
-import optparse
 import os
 import sys
 import unittest
@@ -38,16 +37,18 @@ class InstallTest(unittest.TestCase):
 if __name__ == "__main__":
     os.chdir(os.path.dirname(__file__))
 
-    parser = optparse.OptionParser()
+    import argparse
-    parser.add_option(
+    from testcommon import parse_args_for_test
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (fdroidserver.install.options, args) = parser.parse_args(['--verbose'])
+    fdroidserver.install.options = parse_args_for_test(parser, sys.argv)
-    fdroidserver.common.options = fdroidserver.install.options
 
     newSuite = unittest.TestSuite()
     newSuite.addTest(unittest.makeSuite(InstallTest))

tests/key-tricks.py

@@ -11,7 +11,7 @@ if os.getenv('CI') is None:
     sys.exit(1)
 
 os.chdir(os.path.dirname(__file__))
-config = fdroidserver.common.read_config(common.options)
+config = fdroidserver.common.read_config()
 nightly.PASSWORD = config['keystorepass']
 nightly.KEY_ALIAS = config['repo_keyalias']
 

tests/lint.TestCase

@@ -3,7 +3,6 @@
 # http://www.drdobbs.com/testing/unit-testing-with-python/240165163
 
 import logging
-import optparse
 import os
 import ruamel.yaml
 import shutil
@@ -11,7 +10,6 @@ import sys
 import tempfile
 import unittest
 from pathlib import Path
-from testcommon import mkdtemp
 
 localmodule = Path(__file__).resolve().parent.parent
 print('localmodule: ' + str(localmodule))
@@ -22,6 +20,7 @@ import fdroidserver.common
 import fdroidserver.lint
 import fdroidserver.metadata
 from fdroidserver.common import CATEGORIES_CONFIG_NAME
+from testcommon import mkdtemp, parse_args_for_test
 
 
 class LintTest(unittest.TestCase):
@@ -527,16 +526,17 @@ class LintAntiFeaturesTest(unittest.TestCase):
 
 
 if __name__ == "__main__":
-    parser = optparse.OptionParser()
+    import argparse
-    parser.add_option(
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (fdroidserver.lint.options, args) = parser.parse_args(['--verbose'])
+    fdroidserver.lint.options = parse_args_for_test(parser, sys.argv)
-    fdroidserver.common.options = fdroidserver.lint.options
 
     newSuite = unittest.TestSuite()
     newSuite.addTest(unittest.makeSuite(LintTest))

tests/main.TestCase

@@ -1,7 +1,6 @@
 #!/usr/bin/env python3
 
 import inspect
-import optparse
 import os
 import sys
 import pkgutil
@@ -9,7 +8,6 @@ import textwrap
 import unittest
 import tempfile
 from unittest import mock
-from testcommon import TmpCwd, TmpPyPath
 
 localmodule = os.path.realpath(
     os.path.join(os.path.dirname(inspect.getfile(inspect.currentframe())), '..')
@@ -20,6 +18,7 @@ if localmodule not in sys.path:
 
 from fdroidserver import common
 import fdroidserver.__main__
+from testcommon import TmpCwd, TmpPyPath
 
 
 class MainTest(unittest.TestCase):
@@ -265,15 +264,17 @@ class MainTest(unittest.TestCase):
 if __name__ == "__main__":
     os.chdir(os.path.dirname(__file__))
 
-    parser = optparse.OptionParser()
+    import argparse
-    parser.add_option(
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (common.options, args) = parser.parse_args(['--verbose'])
+    common.options = common.parse_args(parser)
 
     newSuite = unittest.TestSuite()
     newSuite.addTest(unittest.makeSuite(MainTest))

tests/metadata.TestCase

@@ -3,7 +3,6 @@
 import copy
 import io
 import logging
-import optparse
 import os
 import random
 import ruamel.yaml
@@ -16,8 +15,6 @@ from collections import OrderedDict
 from pathlib import Path
 from unittest import mock
 
-from testcommon import TmpCwd, mkdtemp
-
 localmodule = Path(__file__).resolve().parent.parent
 print('localmodule: ' + str(localmodule))
 if localmodule not in sys.path:
@@ -27,6 +24,7 @@ import fdroidserver
 from fdroidserver import metadata
 from fdroidserver.exception import MetaDataException
 from fdroidserver.common import DEFAULT_LOCALE
+from testcommon import TmpCwd, mkdtemp, parse_args_for_test
 
 
 def _get_mock_mf(s):
@@ -2447,15 +2445,17 @@ class PostMetadataParseTest(unittest.TestCase):
 
 
 if __name__ == "__main__":
-    parser = optparse.OptionParser()
+    import argparse
-    parser.add_option(
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (fdroidserver.common.options, args) = parser.parse_args(['--verbose'])
+    parse_args_for_test(parser, sys.argv)
 
     newSuite = unittest.TestSuite()
     newSuite.addTest(unittest.makeSuite(MetadataTest))

tests/net.TestCase

@@ -2,7 +2,6 @@
 
 import inspect
 import logging
-import optparse
 import os
 import random
 import requests
@@ -126,15 +125,17 @@ class NetTest(unittest.TestCase):
 if __name__ == "__main__":
     os.chdir(os.path.dirname(__file__))
 
-    parser = optparse.OptionParser()
+    import argparse
-    parser.add_option(
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (common.options, args) = parser.parse_args(['--verbose'])
+    common.options = common.parse_args(parser)
 
     newSuite = unittest.TestSuite()
     newSuite.addTest(unittest.makeSuite(NetTest))

tests/nightly.TestCase

@@ -2,7 +2,6 @@
 
 import inspect
 import logging
-import optparse
 import os
 import requests
 import shutil
@@ -364,15 +363,17 @@ class NightlyTest(unittest.TestCase):
 if __name__ == "__main__":
     os.chdir(os.path.dirname(__file__))
 
-    parser = optparse.OptionParser()
+    import argparse
-    parser.add_option(
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (common.options, args) = parser.parse_args(['--verbose'])
+    common.options = common.parse_args(parser)
 
     newSuite = unittest.TestSuite()
     newSuite.addTest(unittest.makeSuite(NightlyTest))

tests/parse-fdroiddata-mirror-config.py

@@ -0,0 +1,13 @@
+#!/usr/bin/env python3
+
+import ruamel.yaml
+
+from pathlib import Path
+
+mirrors_yml = Path('/home/hans/code/fdroid/fdroiddata/config/mirrors.yml')
+with mirrors_yml.open() as fp:
+    mirrors_config = ruamel.yaml.YAML(typ='safe').load(fp)
+
+for d in mirrors_config:
+    d['url'] += '/repo'
+    print(d, end=',\n')

tests/publish.TestCase

@@ -13,7 +13,6 @@
 import inspect
 import json
 import logging
-import optparse
 import os
 import shutil
 import sys
@@ -34,7 +33,7 @@ from fdroidserver import common
 from fdroidserver import metadata
 from fdroidserver import signatures
 from fdroidserver.exception import FDroidException
-from testcommon import mkdtemp
+from testcommon import mkdtemp, parse_args_for_test
 
 
 class PublishTest(unittest.TestCase):
@@ -267,7 +266,8 @@ class PublishTest(unittest.TestCase):
 
         os.chdir(self.testdir)
 
-        config = common.read_config(Options)
+        common.options = Options
+        config = common.read_config()
         if 'apksigner' not in config:
             self.skipTest('SKIPPING test_sign_then_implant_signature, apksigner not installed!')
         config['repo_keyalias'] = 'sova'
@@ -341,7 +341,8 @@ class PublishTest(unittest.TestCase):
 
         os.chdir(self.testdir)
 
-        config = common.read_config(Options)
+        common.options = Options
+        config = common.read_config()
         if 'apksigner' not in config:
             self.skipTest('SKIPPING test_error_on_failed, apksigner not installed!')
         config['repo_keyalias'] = 'sova'
@@ -413,15 +414,17 @@ class PublishTest(unittest.TestCase):
 if __name__ == "__main__":
     os.chdir(os.path.dirname(__file__))
 
-    parser = optparse.OptionParser()
+    import argparse
-    parser.add_option(
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (common.options, args) = parser.parse_args(['--verbose'])
+    parse_args_for_test(parser, sys.argv)
 
     newSuite = unittest.TestSuite()
     newSuite.addTest(unittest.makeSuite(PublishTest))

tests/rewritemeta.TestCase

@@ -1,7 +1,6 @@
 #!/usr/bin/env python3
 
 import logging
-import optparse
 import os
 import sys
 import unittest
@@ -9,14 +8,13 @@ import tempfile
 import textwrap
 from pathlib import Path
 
-from testcommon import TmpCwd, mkdtemp
-
 localmodule = Path(__file__).resolve().parent.parent
 print('localmodule: ' + str(localmodule))
 if localmodule not in sys.path:
     sys.path.insert(0, str(localmodule))
 
 from fdroidserver import common, metadata, rewritemeta
+from testcommon import TmpCwd, mkdtemp
 
 
 class RewriteMetaTest(unittest.TestCase):
@@ -265,15 +263,17 @@ class RewriteMetaTest(unittest.TestCase):
 
 
 if __name__ == "__main__":
-    parser = optparse.OptionParser()
+    import argparse
-    parser.add_option(
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (common.options, args) = parser.parse_args(['--verbose'])
+    common.options = common.parse_args(parser)
 
     newSuite = unittest.TestSuite()
     newSuite.addTest(unittest.makeSuite(RewriteMetaTest))

tests/scanner.TestCase

@@ -3,7 +3,6 @@
 import glob
 import inspect
 import logging
-import optparse
 import os
 import re
 import shutil
@@ -31,7 +30,7 @@ import fdroidserver.build
 import fdroidserver.common
 import fdroidserver.metadata
 import fdroidserver.scanner
-from testcommon import TmpCwd, mkdtemp, mock_open_to_str
+from testcommon import TmpCwd, mkdtemp, mock_open_to_str, parse_args_for_test
 
 
 class ScannerTest(unittest.TestCase):
@@ -47,8 +46,8 @@ class ScannerTest(unittest.TestCase):
         self._td.cleanup()
 
     def test_scan_source_files(self):
-        fdroidserver.scanner.options = mock.Mock()
+        fdroidserver.common.options = mock.Mock()
-        fdroidserver.scanner.options.json = False
+        fdroidserver.common.options.json = False
         source_files = os.path.join(self.basedir, 'source-files')
         projects = {
             'OtakuWorld': 2,
@@ -103,8 +102,8 @@ class ScannerTest(unittest.TestCase):
         """Check for sneaking in banned maven repos"""
         os.chdir(self.testdir)
         fdroidserver.scanner.config = None
-        fdroidserver.scanner.options = mock.Mock()
+        fdroidserver.common.options = mock.Mock()
-        fdroidserver.scanner.options.json = True
+        fdroidserver.common.options.json = True
         with open('build.gradle', 'w', encoding='utf-8') as fp:
             fp.write(
                 textwrap.dedent(
@@ -136,8 +135,8 @@ class ScannerTest(unittest.TestCase):
         os.chdir(abs_build_dir)
 
         fdroidserver.scanner.config = None
-        fdroidserver.scanner.options = mock.Mock()
+        fdroidserver.common.options = mock.Mock()
-        fdroidserver.scanner.options.json = True
+        fdroidserver.common.options.json = True
 
         keep = [
             'arg.jar',
@@ -236,7 +235,7 @@ class ScannerTest(unittest.TestCase):
         fdroidserver.build.options.scan_binary = False
         fdroidserver.build.options.notarball = True
         fdroidserver.build.options.skipscan = False
-        fdroidserver.scanner.options = fdroidserver.build.options
+        fdroidserver.common.options = fdroidserver.build.options
 
         app = fdroidserver.metadata.App()
         app.id = 'mocked.app.id'
@@ -315,7 +314,7 @@ class ScannerTest(unittest.TestCase):
         """Check that the scanner can handle scandelete with gradle files with multiple problems"""
         os.chdir(self.testdir)
         fdroidserver.scanner.config = None
-        fdroidserver.scanner.options = mock.Mock()
+        fdroidserver.common.options = mock.Mock()
         build = fdroidserver.metadata.Build()
         build.scandelete = ['build.gradle']
         with open('build.gradle', 'w', encoding='utf-8') as fp:
@@ -733,15 +732,15 @@ class Test_ScannerTool(unittest.TestCase):
             refresh.assert_not_called()
 
     def test_refresh_true(self):
-        fdroidserver.scanner.options = mock.Mock()
+        fdroidserver.common.options = mock.Mock()
-        fdroidserver.scanner.options.refresh_scanner = True
+        fdroidserver.common.options.refresh_scanner = True
         with mock.patch('fdroidserver.scanner.ScannerTool.refresh') as refresh:
             fdroidserver.scanner.ScannerTool()
             refresh.assert_called_once()
 
     def test_refresh_false(self):
-        fdroidserver.scanner.options = mock.Mock()
+        fdroidserver.common.options = mock.Mock()
-        fdroidserver.scanner.options.refresh_scanner = False
+        fdroidserver.common.options.refresh_scanner = False
         with mock.patch('fdroidserver.scanner.ScannerTool.refresh') as refresh:
             fdroidserver.scanner.ScannerTool()
             refresh.assert_not_called()
@@ -754,8 +753,8 @@ class Test_ScannerTool(unittest.TestCase):
             refresh.assert_called_once()
 
     def test_refresh_options_overrides_config(self):
-        fdroidserver.scanner.options = mock.Mock()
+        fdroidserver.common.options = mock.Mock()
-        fdroidserver.scanner.options.refresh_scanner = True
+        fdroidserver.common.options.refresh_scanner = True
         os.chdir(self.testdir)
         pathlib.Path('config.yml').write_text('refresh_scanner: false')
         with mock.patch('fdroidserver.scanner.ScannerTool.refresh') as refresh:
@@ -815,15 +814,17 @@ class Test_main(unittest.TestCase):
 if __name__ == "__main__":
     os.chdir(os.path.dirname(__file__))
 
-    parser = optparse.OptionParser()
+    import argparse
-    parser.add_option(
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (fdroidserver.common.options, args) = parser.parse_args(['--verbose'])
+    parse_args_for_test(parser, sys.argv)
 
     newSuite = unittest.TestSuite()
     newSuite.addTests(

tests/signatures.TestCase

@@ -1,7 +1,6 @@
 #!/usr/bin/env python3
 
 import inspect
-import optparse
 import os
 import sys
 import unittest
@@ -24,7 +23,7 @@ class SignaturesTest(unittest.TestCase):
     def setUp(self):
         logging.basicConfig(level=logging.DEBUG)
         common.config = None
-        config = common.read_config(common.options)
+        config = common.read_config()
         config['jarsigner'] = common.find_sdk_tools_cmd('jarsigner')
         config['verbose'] = True
         common.config = config
@@ -59,15 +58,17 @@ class SignaturesTest(unittest.TestCase):
 if __name__ == "__main__":
     os.chdir(os.path.dirname(__file__))
 
-    parser = optparse.OptionParser()
+    import argparse
-    parser.add_option(
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (common.options, args) = parser.parse_args(['--verbose'])
+    common.options = common.parse_args(parser)
 
     newSuite = unittest.TestSuite()
     newSuite.addTest(unittest.makeSuite(SignaturesTest))

tests/signindex.TestCase

@@ -3,7 +3,6 @@
 import inspect
 import json
 import logging
-import optparse
 import os
 import shutil
 import subprocess
@@ -38,7 +37,7 @@ class SignindexTest(unittest.TestCase):
 
     def setUp(self):
         signindex.config = None
-        config = common.read_config(common.options)
+        config = common.read_config()
         config['jarsigner'] = common.find_sdk_tools_cmd('jarsigner')
         config['verbose'] = True
         config['keystore'] = str(self.basedir / 'keystore.jks')
@@ -193,15 +192,17 @@ class SignindexTest(unittest.TestCase):
 if __name__ == "__main__":
     os.chdir(os.path.dirname(__file__))
 
-    parser = optparse.OptionParser()
+    import argparse
-    parser.add_option(
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (common.options, args) = parser.parse_args(['--verbose'])
+    common.options = common.parse_args(parser)
 
     newSuite = unittest.TestSuite()
     newSuite.addTest(unittest.makeSuite(SignindexTest))

tests/testcommon.py

@@ -21,6 +21,7 @@ import tempfile
 import unittest
 
 from pathlib import Path
+from unittest import mock
 
 
 class TmpCwd:
@@ -72,3 +73,16 @@ def mkdir_testfiles(localmodule, test):
     testdir = testroot / unittest.TestCase.id(test)
     testdir.mkdir(exist_ok=True)
     return tempfile.mkdtemp(dir=testdir)
+
+
+def parse_args_for_test(parser, args):
+    """Only send --flags to the ArgumentParser, not test classes, etc."""
+
+    from fdroidserver.common import parse_args
+
+    flags = []
+    for arg in args:
+        if arg[0] == '-':
+            flags.append(flags)
+    with mock.patch('sys.argv', flags):
+        parse_args(parser)

tests/update.TestCase

@@ -9,7 +9,6 @@ import hashlib
 import inspect
 import json
 import logging
-import optparse
 import os
 import random
 import shutil
@@ -24,7 +23,6 @@ import textwrap
 from binascii import hexlify
 from datetime import datetime
 from pathlib import Path
-from testcommon import TmpCwd, mkdtemp
 from unittest import mock
 
 try:
@@ -62,6 +60,7 @@ import fdroidserver.metadata
 import fdroidserver.update
 from fdroidserver.common import CATEGORIES_CONFIG_NAME
 from fdroidserver.looseversion import LooseVersion
+from testcommon import TmpCwd, mkdtemp, parse_args_for_test
 
 
 DONATION_FIELDS = ('Donate', 'Liberapay', 'OpenCollective')
@@ -1208,7 +1207,7 @@ class UpdateTest(unittest.TestCase):
 
         # Set up options
         fdroidserver.common.options = Options
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        config = fdroidserver.common.read_config()
         if 'apksigner' not in config:  # TODO remove me for buildserver-bullseye
             self.skipTest('SKIPPING test_update_with_AllowedAPKSigningKeys, apksigner not installed!')
         config['repo_keyalias'] = 'sova'
@@ -2271,15 +2270,17 @@ class TestParseFromPbxproj(unittest.TestCase):
 if __name__ == "__main__":
     os.chdir(os.path.dirname(__file__))
 
-    parser = optparse.OptionParser()
+    import argparse
-    parser.add_option(
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (fdroidserver.common.options, args) = parser.parse_args(['--verbose'])
+    parse_args_for_test(parser, sys.argv)
 
     newSuite = unittest.TestSuite()
     newSuite.addTest(unittest.makeSuite(UpdateTest))

tests/vcs.TestCase

@@ -4,7 +4,6 @@
 
 import inspect
 import logging
-import optparse
 import os
 import sys
 import unittest
@@ -22,7 +21,7 @@ import fdroidserver.build
 import fdroidserver.common
 import fdroidserver.metadata
 import fdroidserver.scanner
-from testcommon import mkdtemp
+from testcommon import mkdtemp, parse_args_for_test
 
 
 class VCSTest(unittest.TestCase):
@@ -88,15 +87,17 @@ class VCSTest(unittest.TestCase):
 if __name__ == "__main__":
     os.chdir(os.path.dirname(__file__))
 
-    parser = optparse.OptionParser()
+    import argparse
-    parser.add_option(
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (fdroidserver.common.options, args) = parser.parse_args(['--verbose'])
+    parse_args_for_test(parser, sys.argv)
 
     newSuite = unittest.TestSuite()
     newSuite.addTest(unittest.makeSuite(VCSTest))

tests/verify.TestCase

@@ -3,7 +3,6 @@
 import inspect
 import json
 import logging
-import optparse
 import os
 import shutil
 import sys
@@ -94,15 +93,17 @@ class VerifyTest(unittest.TestCase):
 if __name__ == "__main__":
     os.chdir(os.path.dirname(__file__))
 
-    parser = optparse.OptionParser()
+    import argparse
-    parser.add_option(
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (common.options, args) = parser.parse_args(['--verbose'])
+    common.options = common.parse_args(parser)
 
     newSuite = unittest.TestSuite()
     newSuite.addTest(unittest.makeSuite(VerifyTest))