easybuild-framework icon indicating copy to clipboard operation
easybuild-framework copied to clipboard

[bug|feature] Fixing version parsing of easyconfigs in `tweak.py`

Open robogast opened this issue 2 years ago • 0 comments

TL;DR:

  • tweak.py obtains a EasyConfig's version parameter from fetch_parameters_from_easyconfig, but this doesn't work.
  • Advice: replace with a call to process_easyconfig
  • tweak.py compares EasyConfig's version parameter with distutils.version.LooseVersion, but this also doesn't work.
  • Advice: replace with a call to packaging.version.parse

Context

I am running eb/4.6.0, and I was trying to update OpenCV-4.5.5-foss-2021b-contrib.eb to v4.6.0 on foss/2022a with updated deps, i.e.:

eb OpenCV-4.5.5-foss-2021b-contrib.eb --try-software-version=4.6.0 --try-toolchain-version=2022a --try-update-deps -Dr

But I quickly run into the following error:

Traceback (most recent call last):
  File "/usr/lib64/python3.6/runpy.py", line 193, in _run_module_as_main
    "__main__", mod_spec)
  File "/usr/lib64/python3.6/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "/sw/noarch/Centos8/2021/software/EasyBuild/4.6.0/lib/python3.6/site-packages/easybuild/main.py", line 597, in <module>
    main()
  File "/sw/noarch/Centos8/2021/software/EasyBuild/4.6.0/lib/python3.6/site-packages/easybuild/main.py", line 440, in main
    easyconfigs = tweak(easyconfigs, build_specs, modtool, targetdirs=tweaked_ecs_paths)
  File "/sw/noarch/Centos8/2021/software/EasyBuild/4.6.0/lib/python3.6/site-packages/easybuild/framework/easyconfig/tweak.py", line 203, in tweak
    ignore_versionsuffixes=ignore_versionsuffixes)
  File "/sw/noarch/Centos8/2021/software/EasyBuild/4.6.0/lib/python3.6/site-packages/easybuild/framework/easyconfig/tweak.py", line 1064, in map_easyconfig_to_target_tc_hierarchy
    ignore_versionsuffixes=ignore_versionsuffixes
  File "/sw/noarch/Centos8/2021/software/EasyBuild/4.6.0/lib/python3.6/site-packages/easybuild/framework/easyconfig/tweak.py", line 1223, in find_potential_version_mappings
    LooseVersion(version) > LooseVersion(highest_version_ignoring_versionsuffix):
  File "/usr/lib64/python3.6/distutils/version.py", line 64, in __gt__
    c = self._cmp(other)
  File "/usr/lib64/python3.6/distutils/version.py", line 337, in _cmp
    if self.version < other.version:
TypeError: '<' not supported between instances of 'int' and 'str'

I'm clearly not the first/only one who ran into this issue: #3996

As I will show, the reason for the above error is two-fold:

  1. EasyConfig (ec) version string substitution doesn't work in the regex which is used to retrieve the software version in tweak.py.
  2. Correctly substituted strings still are not comparable.

Issue 1.

For a few ec's, version string substitution is used, e.g. Java-1.8.0_292-OpenJDK.eb. The problem is that the un-substituted version string is obtained from fetch_parameters_from_easyconfig, which simply regex-es the ec for the version.

https://github.com/easybuilders/easybuild-framework/blob/b6daaacf08ac0b6ebec6c191f4359e2dcbfff5a0/easybuild/framework/easyconfig/tweak.py#L1213-L1214

For the above Java-1.8.0_292-OpenJDK.eb, the obtained version is: "'1.%s.0_%s' % (local_java_version, local_patch_version)"

Proposed fix:

We could replace this function call with calling process_easyconfig, which actually does perform substitution:

# Replace the below:
# version, newversionsuffix = fetch_parameters_from_easyconfig(read_file(path), ['versionsuffix'])
#
# With the following snippet:
ec = process_easyconfig(path)[0]['ec']
version, newversionsuffix = (
    ec.get('version', None),
    ec.get('versionsuffix', None)
)

Replacing the fetch_parameters_from_easyconfig call with the following snippet, version 1.8.0_292 is correctly substituted and passed to the rest of the function.

Issue 2.

Even with correctly substituted version string, we run into another issue: LooseVersion is not able to compare all version permutations, e.g. comparing Java-1.8_265-b01-OpenJDK-aarch64.eb to Java-1.8.0_292-OpenJDK.eb, i.e. 1.8_265 to 1.8.0_292, resulting in the above error TypeError: '<' not supported between instances of 'int' and 'str'

This is not surprising, since distutils.version.LooseVersion is rather limited in its scope, and doesn't even adhere to Python's well-defined (and 8 year old) versioning scheme PEP-0440.

Proposed fix:

In the Python-3.10 accepted PEP-0632, which deprecated the distutils module entirely, it is recommended to switch to packaging.version as analogue to distutils.version.

Taking the example Java version strings from above (without versionsuffixes), we can see that packaging.version.parse is able to correctly compare version strings, where distutils.version.LooseVersion is not:

$ python
Python 3.10.4 (main, Jul 13 2022, 12:16:05) [GCC 11.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from distutils.version import LooseVersion
>>> from packaging.version import parse
>>> 
>>> v1, v2 = "1.8_265", "1.8.0_292"
>>>
>>> LooseVersion(v1) < LooseVersion(v2)
<stdin>:1: DeprecationWarning: distutils Version classes are deprecated. Use packaging.version instead.
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/robertsc/.local/easybuild/Centos8/2021/software/Python/3.10.4-GCCcore-11.3.0/lib/python3.10/site-packages/setuptools/_distutils/version.py", line 72, in __lt__
    c = self._cmp(other)
  File "/home/robertsc/.local/easybuild/Centos8/2021/software/Python/3.10.4-GCCcore-11.3.0/lib/python3.10/site-packages/setuptools/_distutils/version.py", line 357, in _cmp
    if self.version < other.version:
TypeError: '<' not supported between instances of 'str' and 'int'
>>>
>>> parse(v1) < parse(v2)
True
>>> 

After replacing all LooseVersion calls in tweak.py with parse calls, I'm able to correctly perform a dry-run of my eb command.

Roadblocks:

I forsee a few (minor) roadblocks before adapting the above solutions:

  • process_easyconfig may be slower than fetch_parameters_from_easyconfig, resulting in longer build times
  • easybuild-framework requires a new python dependency (packaging, although it is already included in setuptools), with the following subtlety:
    • packaging == 20.9 for python == 2.7, 3.5
    • packaging == 21.3 is the latest release for python >= 3.6

Although my personal opinion is that we shouldn't support EOL software which has up-to-date alternatives (i.e. Python <= 3.6, let alone Python == 2.7), but this may not be the place to hold that discussion.

robogast avatar Sep 06 '22 13:09 robogast