From 6f9a6f70a6eeb6a43a91b4b2151bacdf6c5a8180 Mon Sep 17 00:00:00 2001 From: Timo Wilken Date: Wed, 8 Nov 2023 14:57:27 +0100 Subject: [PATCH] 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 python@3.11 to python@3.12 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 | 10 ++++++---- alibuild_helpers/build.py | 36 +++++++++++++++++++++++----------- pyproject.toml | 11 ++++++----- setup.py | 9 +++++---- tests/test_build.py | 10 +++++----- tests/test_workarea.py | 2 +- tox.ini | 4 +++- 7 files changed, 51 insertions(+), 31 deletions(-) diff --git a/.github/workflows/pr-check.yml b/.github/workflows/pr-check.yml index f934b2ef..d4650784 100644 --- a/.github/workflows/pr-check.yml +++ b/.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' diff --git a/alibuild_helpers/build.py b/alibuild_helpers/build.py index 7a2bfd1f..8b23faf9 100644 --- a/alibuild_helpers/build.py +++ b/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 diff --git a/pyproject.toml b/pyproject.toml index bf011b59..81d0976f 100644 --- a/pyproject.toml +++ b/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', diff --git a/setup.py b/setup.py index 04689a6b..d466f1ba 100644 --- a/setup.py +++ b/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? diff --git a/tests/test_build.py b/tests/test_build.py index 67172213..d1ea2f7b 100644 --- a/tests/test_build.py +++ b/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.""" diff --git a/tests/test_workarea.py b/tests/test_workarea.py index 0bde82cb..7b284cd6 100644 --- a/tests/test_workarea.py +++ b/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/*", diff --git a/tox.ini b/tox.ini index 44bee403..9d5f87aa 100644 --- a/tox.ini +++ b/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.