Fix hashing on Python 3.12 (#810)

* Update tested Python versions

Nothing actually uses Python 3.7, but 3.12 is becoming more common (especially
on Mac).

* Fix hashing on Python 3.12

Python 3.12 changes the default string representation of OrderedDicts, which
aliBuild relies upon (this is obviously far from ideal, but we're stuck with
it).

This patch hashes OrderedDicts in their old representation, to maintain hash
compatibility between python<3.12 and python==3.12.

This saves e.g. Mac users from having to rebuild their entire sw directory
when upgrading from [email protected] to [email protected] using brew.

Hashes are checked using unittests, so unittests passing should mean that
hashes are computed exactly the same as before. (I found this issue by running
the usual unittests on python3.12.)

* Fix unittest checking for Git calls

This happened to work previously, since mock.has_calls existed, but apparently
only returned a bool. This has been removed in Python 3.12; use the correct
assert_has_calls instead, and fix the calls we assert to be there.

* Update supported Python versions to include 3.12

.github/workflows/pr-check.yml

@@ -21,14 +21,15 @@ jobs:
     runs-on: ubuntu-20.04
 
     strategy:
+      fail-fast: false   # continue executing other checks if one fails
       matrix:
         python-version:
           - '3.6.8'   # slc7, slc8 and cs8 containers
-          - '3.7'
-          - '3.8'
-          - '3.9'
-          - '3.10'
+          - '3.8.10'  # ubuntu2004 container
+          - '3.9.16'  # slc9 container
+          - '3.10.6'  # ubuntu2204 container
           - '3.11'
+          - '3.12'
 
     steps:
       - uses: actions/checkout@v3
@@ -68,6 +69,7 @@ jobs:
     runs-on: macos-latest
 
     strategy:
+      fail-fast: false   # continue executing other checks if one fails
       matrix:
         python-version:
           - '3.11'

alibuild_helpers/build.py

@@ -174,15 +174,8 @@ def storeHashes(package, specs, isDevelPkg, considerRelocation):
   if spec.get("force_rebuild", False):
     h_all(str(time.time()))
 
-  def hash_data_for_key(key):
-    if sys.version_info[0] < 3 and key in spec and isinstance(spec[key], OrderedDict):
-      # Python 2: use YAML dict order to prevent changing hashes
-      return str(yaml.safe_load(yamlDump(spec[key])))
-    else:
-      return str(spec.get(key, "none"))
-
-  for x in ("recipe", "version", "package"):
-    h_all(hash_data_for_key(x))
+  for key in ("recipe", "version", "package"):
+    h_all(spec.get(key, "none"))
 
   # commit_hash could be a commit hash (if we're not building a tag, but
   # instead e.g. a branch or particular commit specified by its hash), or it
@@ -221,8 +214,29 @@ def h_all(data):  # pylint: disable=function-redefined
     for _, _, hasher in h_alternatives:
       hasher(data)
 
-  for x in ("env", "append_path", "prepend_path"):
-    h_all(hash_data_for_key(x))
+  for key in ("env", "append_path", "prepend_path"):
+    if sys.version_info[0] < 3 and key in spec and isinstance(spec[key], OrderedDict):
+      # Python 2: use YAML dict order to prevent changing hashes
+      h_all(str(yaml.safe_load(yamlDump(spec[key]))))
+    elif key not in spec:
+      h_all("none")
+    else:
+      # spec["env"] is of type OrderedDict[str, str].
+      # spec["*_path"] are of type OrderedDict[str, list[str]].
+      assert isinstance(spec[key], OrderedDict), \
+        "spec[%r] was of type %r" % (key, type(spec[key]))
+
+      # Python 3.12 changed the string representation of OrderedDicts from
+      # OrderedDict([(key, value)]) to OrderedDict({key: value}), so to remain
+      # compatible, we need to emulate the previous string representation.
+      h_all("OrderedDict([")
+      h_all(", ".join(
+        # XXX: We still rely on repr("str") being "'str'",
+        # and on repr(["a", "b"]) being "['a', 'b']".
+        "(%r, %r)" % (key, value)
+        for key, value in spec[key].items()
+      ))
+      h_all("])")
 
   for tag, commit_hash, hasher in h_alternatives:
     # If the commit hash is a real hash, and not a tag, we can safely assume

pyproject.toml

@@ -20,15 +20,16 @@ classifiers = [
     'License :: OSI Approved :: GNU General Public License v3 (GPLv3)',
     'Programming Language :: Python :: 2.7',   # slc6
     'Programming Language :: Python :: 3.6',   # slc7, slc8, cs8
-    'Programming Language :: Python :: 3.8',   # MacOS
-    'Programming Language :: Python :: 3.9',   # alma9
-    'Programming Language :: Python :: 3.10',
-    'Programming Language :: Python :: 3.11',
+    'Programming Language :: Python :: 3.8',   # ubuntu2004
+    'Programming Language :: Python :: 3.9',   # slc9
+    'Programming Language :: Python :: 3.10',  # ubuntu2204
+    'Programming Language :: Python :: 3.11',  # MacOS
+    'Programming Language :: Python :: 3.12',  # MacOS
 ]
 
 dependencies = [
   'pyyaml',
-  'requests', 
+  'requests',
   'distro',
   'jinja2',
   'boto3',

setup.py

@@ -54,10 +54,11 @@
         # that you indicate whether you support Python 2, Python 3 or both.
         'Programming Language :: Python :: 2.7',   # slc6
         'Programming Language :: Python :: 3.6',   # slc7, slc8, cs8
-        'Programming Language :: Python :: 3.8',   # MacOS
-        'Programming Language :: Python :: 3.9',   # alma9
-        'Programming Language :: Python :: 3.10',
-        'Programming Language :: Python :: 3.11',
+        'Programming Language :: Python :: 3.8',   # ubuntu2004
+        'Programming Language :: Python :: 3.9',   # slc9
+        'Programming Language :: Python :: 3.10',  # ubuntu2204
+        'Programming Language :: Python :: 3.11',  # MacOS
+        'Programming Language :: Python :: 3.12',  # MacOS
     ],
 
     # What does your project relate to?

tests/test_build.py

@@ -276,10 +276,10 @@ def test_coverDoBuild(self, mock_debug, mock_glob, mock_sys, mock_git_git):
         clone_args, clone_dir, clone_check = GIT_CLONE_ZLIB_ARGS
         fetch_args, fetch_dir, fetch_check = GIT_FETCH_ROOT_ARGS
         common_calls = [
+            call(("rev-parse", "HEAD"), args.configDir),
             call(list(clone_args), directory=clone_dir, check=clone_check, prompt=False),
             call(["ls-remote", "--heads", "--tags", args.referenceSources + "/zlib"],
                  directory=".", check=False, prompt=False),
-            call(["clone", "--bare", "https://github.com/star-externals/zlib", "/sw/MIRROR/zlib", "--filter=blob:none"]),
             call(["ls-remote", "--heads", "--tags", args.referenceSources + "/root"],
                  directory=".", check=False, prompt=False),
         ]
@@ -290,7 +290,7 @@ def test_coverDoBuild(self, mock_debug, mock_glob, mock_sys, mock_git_git):
         self.assertEqual(exit_code, 0)
         mock_debug.assert_called_with("Everything done")
         self.assertEqual(mock_git_git.call_count, len(common_calls))
-        mock_git_git.has_calls(common_calls)
+        mock_git_git.assert_has_calls(common_calls, any_order=True)
 
         # Force fetching repos
         mock_git_git.reset_mock()
@@ -303,9 +303,9 @@ def test_coverDoBuild(self, mock_debug, mock_glob, mock_sys, mock_git_git):
         # We can't compare directly against the list of calls here as they
         # might happen in any order.
         self.assertEqual(mock_git_git.call_count, len(common_calls) + 1)
-        mock_git_git.has_calls(common_calls + [
-            call(fetch_args, directory=fetch_dir, check=fetch_check),
-        ])
+        mock_git_git.assert_has_calls(common_calls + [
+            call(list(fetch_args), directory=fetch_dir, check=fetch_check, prompt=False),
+        ], any_order=True)
 
     def test_hashing(self):
         """Check that the hashes assigned to packages remain constant."""

tests/test_workarea.py

@@ -60,7 +60,7 @@ def test_reference_sources_updated(self, mock_git, mock_open, mock_makedirs, moc
         updateReferenceRepoSpec(referenceSources="sw/MIRROR", p="AliRoot",
                                 spec=spec, fetch=True)
         mock_exists.assert_called_with("%s/sw/MIRROR/aliroot" % getcwd())
-        mock_exists.has_calls([])
+        mock_exists.assert_has_calls([])
         mock_makedirs.assert_called_with("%s/sw/MIRROR" % getcwd())
         mock_git.assert_called_once_with([
             "fetch", "-f", "--tags", spec["source"], "+refs/heads/*:refs/heads/*",

tox.ini

@@ -1,6 +1,6 @@
 [tox]
 minversion = 3.20
-envlist = lint, py{27, 36, 37, 38, 39, 310, 311}, darwin
+envlist = lint, py{27, 36, 37, 38, 39, 310, 311, 312, 313}, darwin
 
 [gh-actions]
 # The "lint" job is run separately.
@@ -12,6 +12,8 @@ python =
     3.9: py39
     3.10: py310
     3.11: py311
+    3.12: py312
+    3.13: py313
 
 [testenv:lint]
 # Warning: This environment inherits settings from the main [testenv] section.