From 067196c508c1c81e0b362abcdcda46353c755a4d Mon Sep 17 00:00:00 2001
From: Steve Howell <showell@zulipchat.com>
Date: Thu, 16 Apr 2020 11:49:21 +0000
Subject: [PATCH] provision: Simplify `is_force` codepaths.

I remove `is_force` from `file_or_package_hash_updated`
and modernize its mypy annotations.

If `is_force` is `True`, we just now run the thing
we want to force-run without having to call
`file_or_package_hash_updated` to expensively
and riskily return `True`.

Another nice outcome of this change is that if
`file_or_package_hash_updated` returns `True`,
you can know that the file or package has
indeed been updated.

For the case of `build_pygments_data` we also
skip an `os.path.exists` check when `is_force`
is `True`.

We will short-circuit more logic in the next
few commits, as well as cleaning up some of
the long/wrapper lines in the `if` statements.
---
 scripts/lib/zulip_tools.py   |  7 ++++---
 tools/lib/provision_inner.py | 12 +++++++-----
 zerver/lib/test_fixtures.py  |  3 +--
 3 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/scripts/lib/zulip_tools.py b/scripts/lib/zulip_tools.py
index 7ccb661074..07a7fd94d8 100755
--- a/scripts/lib/zulip_tools.py
+++ b/scripts/lib/zulip_tools.py
@@ -390,8 +390,9 @@ def os_families() -> Set[str]:
     distro_info = parse_os_release()
     return {distro_info["ID"], *distro_info.get("ID_LIKE", "").split()}
 
-def file_or_package_hash_updated(paths, hash_name, is_force, package_versions=[]):
-    # type: (List[str], str, bool, List[str]) -> bool
+def file_or_package_hash_updated(paths: List[str],
+                                 hash_name: str,
+                                 package_versions: List[str]=[]) -> bool:
     # Check whether the files or package_versions passed as arguments
     # changed compared to the last execution.
     sha1sum = hashlib.sha1()
@@ -410,7 +411,7 @@ def file_or_package_hash_updated(paths, hash_name, is_force, package_versions=[]
         hash_file.seek(0)
         last_hash = hash_file.read()
 
-        if is_force or (new_hash != last_hash):
+        if (new_hash != last_hash):
             hash_file.seek(0)
             hash_file.truncate()
             hash_file.write(new_hash)
diff --git a/tools/lib/provision_inner.py b/tools/lib/provision_inner.py
index 7efa49101f..80c2467e31 100755
--- a/tools/lib/provision_inner.py
+++ b/tools/lib/provision_inner.py
@@ -127,16 +127,18 @@ def main(options: argparse.Namespace) -> int:
 
     build_pygments_data_paths = ["tools/setup/build_pygments_data", "tools/setup/lang.json"]
     from pygments import __version__ as pygments_version
-    if not os.path.exists("static/generated/pygments_data.json") or file_or_package_hash_updated(
-            build_pygments_data_paths, "build_pygments_data_hash", options.is_force,
-            [pygments_version]):
+    if (options.is_force or
+            not os.path.exists("static/generated/pygments_data.json") or
+            file_or_package_hash_updated(
+                build_pygments_data_paths, "build_pygments_data_hash", [pygments_version])):
         run(["tools/setup/build_pygments_data"])
     else:
         print("No need to run `tools/setup/build_pygments_data`.")
 
     email_source_paths = ["scripts/setup/inline_email_css.py", "templates/zerver/emails/email.css"]
     email_source_paths += glob.glob('templates/zerver/emails/*.source.html')
-    if file_or_package_hash_updated(email_source_paths, "last_email_source_files_hash", options.is_force):
+    if (options.is_force or
+            file_or_package_hash_updated(email_source_paths, "last_email_source_files_hash")):
         run(["scripts/setup/inline_email_css.py"])
     else:
         print("No need to run `scripts/setup/inline_email_css.py`.")
@@ -191,7 +193,7 @@ def main(options: argparse.Namespace) -> int:
         paths += glob.glob('locale/*/LC_MESSAGES/*.po')
         paths += glob.glob('locale/*/translations.json')
 
-        if file_or_package_hash_updated(paths, "last_compilemessages_hash", options.is_force):
+        if (options.force or file_or_package_hash_updated(paths, "last_compilemessages_hash")):
             run(["./manage.py", "compilemessages"])
         else:
             print("No need to run `manage.py compilemessages`.")
diff --git a/zerver/lib/test_fixtures.py b/zerver/lib/test_fixtures.py
index f6a0d021c2..6d916f0ded 100644
--- a/zerver/lib/test_fixtures.py
+++ b/zerver/lib/test_fixtures.py
@@ -242,8 +242,7 @@ def template_database_status(database_type: str) -> str:
         # migrations without spending a few 100ms parsing all the
         # Python migration code.
         paths = glob.glob('*/migrations/*.py')
-        check_migrations = file_or_package_hash_updated(paths, "migrations_hash_" + database.database_name,
-                                                        is_force=False)
+        check_migrations = file_or_package_hash_updated(paths, "migrations_hash_" + database.database_name)
         if not check_migrations:
             return 'current'