Cleanup release scripts, add more CI tests (#5629)

* Add dev/releases/requirements.txt
* dev/releases: run black on python code
* dev/releases: run isort on python code
* CI: ensure code in dev/releases is formatted
* dev/releases: fix many mypy errors, in particular add type annotations

.github/workflows/release.yml

@@ -31,8 +31,32 @@ env:
   NO_COVERAGE: 1
 
 jobs:
+  tools:
+    name: "Validate release scripts"
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Install dependencies
+        run: |
+          python -m pip install -r dev/releases/requirements.txt
+          python -m pip install pytest mock black isort
+
+      - name: Check code formatting
+        run: python -m black --check --diff dev/releases
+
+      - name: Check imports
+        run: python -m isort --check --profile black dev/releases
+
+      #- name: Validate types
+      #  run: python -m mypy --disallow-untyped-calls --disallow-untyped-defs dev/releases/*.py
+
+      #- name: Run tests
+      #  run: python -m pytest tools/tests/dev/releases*.py -vv
+
   unix:
     name: "Create Unix archives and data"
+    needs: tools
     # Don't run this twice on PRs for branches pushed to the same repository
     if: ${{ !(github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository) }}
     runs-on: ubuntu-latest

dev/releases/create_stable_branch.py

@@ -14,12 +14,12 @@
 # TODO: implement parts of the steps described in
 # <https://github.com/gap-system/gap-distribution/blob/master/DistributionUpdate/STABLE_BRANCH_CHECKLIST.md>
 
-from utils import error, notice, patchfile
-import utils
-
 import subprocess
 import sys
 
+import utils
+from utils import error, notice, patchfile
+
 # Insist on Python >= 3.6 for f-strings and other goodies
 if sys.version_info < (3, 6):
     error("Python 3.6 or newer is required")

dev/releases/make_archives.py

@@ -9,12 +9,10 @@
 ##  SPDX-License-Identifier: GPL-2.0-or-later
 ##
 ##  This script create the archives that form a GA{} release.
-##  
+##
 ##  The version of the gap release is taken from the Makefile variable
 ##  GAP_BUILD_VERSION.
-##  
-from utils import *
-
+##
 import glob
 import grp
 import gzip
@@ -26,8 +24,10 @@
 import sys
 import tarfile
 
+from utils import *
+
 # Insist on Python >= 3.6 for f-strings and other goodies
-if sys.version_info < (3,6):
+if sys.version_info < (3, 6):
     error("Python 3.6 or newer is required")
 
 notice("Checking prerequisites")
@@ -56,7 +56,7 @@
     error("make sure GAP has been compiled via './configure && make'")
 notice(f"Detected GAP version {gapversion}")
 
-if re.fullmatch( r"[1-9]+\.[0-9]+\.[0-9]+", gapversion) != None:
+if re.fullmatch(r"[1-9]+\.[0-9]+\.[0-9]+", gapversion) != None:
     notice(f"--- THIS LOOKS LIKE A RELEASE ---")
     pkg_tag = f"v{gapversion}"
 else:
@@ -68,56 +68,76 @@
 # TODO: is that really what we want? Or should it be the date this
 # script was run? Or for releases, perhaps use the TaggerDate of the
 # release tag (but then we need to find and process that tag)
-commit_date = subprocess.run(["git", "show", "-s", "--format=%as"],
-                             check=True, capture_output=True, text=True)
-commit_date = commit_date.stdout.strip()
+commit_date = subprocess.run(
+    ["git", "show", "-s", "--format=%as"], check=True, capture_output=True, text=True
+).stdout.strip()
 commit_year = commit_date[0:4]
 
 # derive tarball names
 basename = f"gap-{gapversion}"
-all_packages = f"packages-v{gapversion}" # only the pkg dir
+all_packages = f"packages-v{gapversion}"  # only the pkg dir
 all_packages_tarball = f"{all_packages}.tar.gz"
-req_packages = f"packages-required-v{gapversion}" # a subset of the above
+req_packages = f"packages-required-v{gapversion}"  # a subset of the above
 req_packages_tarball = f"{req_packages}.tar.gz"
 
-PKG_BOOTSTRAP_URL = f"https://github.com/gap-system/PackageDistro/releases/download/{pkg_tag}/"
+PKG_BOOTSTRAP_URL = (
+    f"https://github.com/gap-system/PackageDistro/releases/download/{pkg_tag}/"
+)
 PKG_MINIMAL = "packages-required.tar.gz"
 PKG_FULL = "packages.tar.gz"
 
 # Exporting repository content into tmp
 notice("Exporting repository content via `git archive`")
 rawbasename = "gap-raw"
 rawgap_tarfile = f"{tmpdir}/{rawbasename}.tar"
-subprocess.run(["git", "archive",
-                f"--prefix={basename}/",
-                f"--output={rawgap_tarfile}",
-                "HEAD"], check=True)
+subprocess.run(
+    ["git", "archive", f"--prefix={basename}/", f"--output={rawgap_tarfile}", "HEAD"],
+    check=True,
+)
 
 notice("Extracting exported content")
-shutil.rmtree(basename, ignore_errors=True) # remove any leftovers
+shutil.rmtree(basename, ignore_errors=True)  # remove any leftovers
 with tarfile.open(rawgap_tarfile) as tar:
     tar.extractall(path=tmpdir)
 os.remove(rawgap_tarfile)
 
 notice("Processing exported content")
-manifest_list = [] # collect names of assets to be uploaded to GitHub release
+manifest_list = []  # collect names of assets to be uploaded to GitHub release
 
 # download package distribution
-notice("Downloading package distribution")   # ... outside of the directory we just created
-download_with_sha256(PKG_BOOTSTRAP_URL+"package-infos.json.gz", tmpdir+"/"+"package-infos.json.gz")
+notice(
+    "Downloading package distribution"
+)  # ... outside of the directory we just created
+download_with_sha256(
+    PKG_BOOTSTRAP_URL + "package-infos.json.gz", tmpdir + "/" + "package-infos.json.gz"
+)
 manifest_list.append("package-infos.json.gz")
-download_with_sha256(PKG_BOOTSTRAP_URL+PKG_MINIMAL, tmpdir+"/"+req_packages_tarball)
+download_with_sha256(
+    PKG_BOOTSTRAP_URL + PKG_MINIMAL, tmpdir + "/" + req_packages_tarball
+)
 manifest_list.append(req_packages_tarball)
-download_with_sha256(PKG_BOOTSTRAP_URL+PKG_FULL, tmpdir+"/"+all_packages_tarball)
+download_with_sha256(PKG_BOOTSTRAP_URL + PKG_FULL, tmpdir + "/" + all_packages_tarball)
 manifest_list.append(all_packages_tarball)
 
 with working_directory(tmpdir + "/" + basename):
     # This sets the version, release day and year of the release we are
     # creating.
     notice("Patching configure.ac")
-    patchfile("configure.ac", r"m4_define\(\[gap_version\],[^\n]+", r"m4_define([gap_version], ["+gapversion+"])")
-    patchfile("configure.ac", r"m4_define\(\[gap_releaseday\],[^\n]+", r"m4_define([gap_releaseday], ["+commit_date+"])")
-    patchfile("configure.ac", r"m4_define\(\[gap_releaseyear\],[^\n]+", r"m4_define([gap_releaseyear], ["+commit_year+"])")
+    patchfile(
+        "configure.ac",
+        r"m4_define\(\[gap_version\],[^\n]+",
+        r"m4_define([gap_version], [" + gapversion + "])",
+    )
+    patchfile(
+        "configure.ac",
+        r"m4_define\(\[gap_releaseday\],[^\n]+",
+        r"m4_define([gap_releaseday], [" + commit_date + "])",
+    )
+    patchfile(
+        "configure.ac",
+        r"m4_define\(\[gap_releaseyear\],[^\n]+",
+        r"m4_define([gap_releaseyear], [" + commit_year + "])",
+    )
 
     # Building GAP
     notice("Running autogen.sh")
@@ -153,36 +173,45 @@
     shutil.rmtree("hpcgap-build")
 
     notice("Extracting package tarballs")
-    with tarfile.open(tmpdir+"/"+all_packages_tarball) as tar:
+    with tarfile.open(tmpdir + "/" + all_packages_tarball) as tar:
         tar.extractall(path="pkg")
     # for some reason pkg sometimes ends up with permission 0700 so
     # we make sure to fix that here
     subprocess.run(["chmod", "0755", "pkg"], check=True)
     # ensure all files are at readable by everyone
     subprocess.run(["chmod", "-R", "a+r", "."], check=True)
 
-    with tarfile.open(tmpdir+"/"+req_packages_tarball) as tar:
-        tar.extractall(path=tmpdir+"/"+req_packages)
+    with tarfile.open(tmpdir + "/" + req_packages_tarball) as tar:
+        tar.extractall(path=tmpdir + "/" + req_packages)
 
     notice("Building GAP's manuals")
     run_with_log(["make", "doc"], "gapdoc", "building the manuals")
 
     # Now we create the help-links.json file. We build
     # the json package, create the files, then clean up the package again.
     notice("Compiling json package")
-    path_to_json_package = glob.glob(f'{tmpdir}/{basename}/pkg/json*')[0]
+    path_to_json_package = glob.glob(f"{tmpdir}/{basename}/pkg/json*")[0]
     with working_directory(path_to_json_package):
         subprocess.run(["./configure"], check=True)
         subprocess.run(["make"], check=True)
 
     notice(f"Constructing help-links JSON file")
     json_output = subprocess.run(
-            ["./gap", "-r", "--quiet", "--quitonbreak", f"dev/releases/HelpLinks-to-JSON.g"],
-            check=True, capture_output=True, text=True)
+        [
+            "./gap",
+            "-r",
+            "--quiet",
+            "--quitonbreak",
+            f"dev/releases/HelpLinks-to-JSON.g",
+        ],
+        check=True,
+        capture_output=True,
+        text=True,
+    )
     formatted_json = json.dumps(json.loads(json_output.stdout), indent=2)
     with working_directory(tmpdir):
-        with gzip.open("help-links.json.gz", 'wb') as file:
-            file.write(formatted_json.encode('utf-8'))
+        with gzip.open("help-links.json.gz", "wb") as file:
+            file.write(formatted_json.encode("utf-8"))
         manifest_list.append("help-links.json.gz")
 
     notice("Cleaning up the json package")
@@ -193,11 +222,11 @@
 
     notice("Removing unwanted version-controlled files")
     badfiles = [
-    ".codecov.yml",
-    ".ctags",
-    ".gitattributes",
-    ".gitignore",
-    ".mailmap",
+        ".codecov.yml",
+        ".ctags",
+        ".gitattributes",
+        ".gitignore",
+        ".mailmap",
     ]
 
     shutil.rmtree("benchmark")
@@ -217,7 +246,9 @@
 # Create an archive in the current directory with shutil.make_archive, and
 # record the filename of the new archive in <manifest_list>.
 # The arguments of this function match those to shutil.make_archive.
-def make_and_record_archive(name, compression, root_dir, base_dir):
+def make_and_record_archive(
+    name: str, compression: str, root_dir: str, base_dir: str
+) -> None:
     # Deduce file extension from compression
     if compression == "gztar":
         ext = ".tar.gz"
@@ -230,25 +261,26 @@ def make_and_record_archive(name, compression, root_dir, base_dir):
     notice(f"Creating {filename}")
     owner = pwd.getpwuid(0).pw_name
     group = grp.getgrgid(0).gr_name
-    shutil.make_archive(name, compression, root_dir, base_dir, owner = owner, group = group)
+    shutil.make_archive(name, compression, root_dir, base_dir, owner=owner, group=group)
     manifest_list.append(filename)
 
+
 # Create the remaining archives
 notice("Creating remaining GAP and package archives")
 with working_directory(tmpdir):
     make_and_record_archive(basename, "gztar", ".", basename)
-    make_and_record_archive(basename, "zip",   ".", basename)
+    make_and_record_archive(basename, "zip", ".", basename)
     make_and_record_archive(all_packages, "zip", basename, "pkg")
     make_and_record_archive(req_packages, "zip", ".", req_packages)
     notice("Removing packages to facilitate creating the GAP core archives")
     shutil.rmtree(basename + "/pkg")
     make_and_record_archive(basename + "-core", "gztar", ".", basename)
-    make_and_record_archive(basename + "-core", "zip",   ".", basename)
+    make_and_record_archive(basename + "-core", "zip", ".", basename)
 
     # If you create additional archives, make sure to add them to manifest_list!
     manifest_filename = "MANIFEST"
     notice(f"Creating the manifest, with name {manifest_filename}")
-    with open(manifest_filename, 'w') as manifest:
+    with open(manifest_filename, "w") as manifest:
         for filename in manifest_list:
             manifest.write(f"{filename}\n")
 

dev/releases/make_github_release.py

@@ -15,14 +15,14 @@
 ##  If we do import * from utils, then initialize_github can't overwrite the
 ##  global CURRENT_REPO variables.
 ##
-import utils
-import utils_github
 import sys
 
+import utils
+import utils_github
 from utils import error, notice
 
 if len(sys.argv) != 3:
-    error("usage: "+sys.argv[0]+" <tag_name> <path_to_release>")
+    error("usage: " + sys.argv[0] + " <tag_name> <path_to_release>")
 
 TAG_NAME = sys.argv[1]
 PATH_TO_RELEASE = sys.argv[2]
@@ -47,16 +47,18 @@
     error(f"Github release with tag '{TAG_NAME}' already exists!")
 
 # Create release
-RELEASE_NOTE = f"For an overview of changes in GAP {VERSION} see the " \
+RELEASE_NOTE = (
+    f"For an overview of changes in GAP {VERSION} see the "
     + f"[CHANGES.md](https://github.com/gap-system/gap/blob/{TAG_NAME}/CHANGES.md) file."
+)
 notice(f"Creating release {TAG_NAME}")
-RELEASE = utils_github.CURRENT_REPO.create_git_release(TAG_NAME, TAG_NAME,
-                                                RELEASE_NOTE,
-                                                prerelease=True)
+RELEASE = utils_github.CURRENT_REPO.create_git_release(
+    TAG_NAME, TAG_NAME, RELEASE_NOTE, prerelease=True
+)
 
 with utils.working_directory(PATH_TO_RELEASE):
     manifest_filename = "MANIFEST"
-    with open(manifest_filename, 'r') as manifest_file:
+    with open(manifest_filename, "r") as manifest_file:
         manifest = manifest_file.read().splitlines()
 
     notice(f"Contents of {manifest_filename}:")