Enhanced detection of singularity version including a distribution detection

[salexan2001]

This is a fix for problems related to #1572 .

I refactored get_version and its cache to return not only the version number, but include the singularity "flavor" (distribution). This currently can be "apptainer", "singularity-ce", or "singularity" as far as I know. I also refactored the function to return version numbers directly as a tuple of integers to simplify subsequent checks. The get_version function currently seems to be used only from within the singularity.py module and is used by various is_singularity_X_Y_or_newer functions.

I added a new function for checking for apptainer 1.0 or higher. Apptainer 1.0 is (currently) equivalent to singularityCE 3.9.5 (see https://github.com/apptainer/apptainer/releases). Therefore I also adapted the other version checking functions to return True in case there is a compatible apptainer version installed.

Additionally I added some missing docstrings.

I tested the function manually and also successfully tried the "--singularity" option using apptainer.

I don't have singularity and singularity-ce here, yet (so untested). Are these distributions tested automatically?

I hope, merging into main is fine? I haven't found the project specific guidelines for branches.

[codecov]

Codecov Report

Merging #1654 (8dde785) into main (61c13dc) will increase coverage by 0.04%.
The diff coverage is 94.11%.

@@            Coverage Diff             @@
##             main    #1654      +/-   ##
==========================================
+ Coverage   66.81%   66.86%   +0.04%     
==========================================
  Files          93       93              
  Lines       16586    16622      +36     
  Branches     4404     4414      +10     
==========================================
+ Hits        11082    11114      +32     
- Misses       4365     4366       +1     
- Partials     1139     1142       +3     

Impacted Files	Coverage Δ
cwltool/singularity.py	61.90% <94.11%> (+3.68%)	⬆️
cwltool/cwltool/singularity.py	51.94% <0.00%> (-0.17%)	⬇️
cwltool/cwltool/job.py	62.64% <0.00%> (+0.79%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 61c13dc...8dde785. Read the comment docs.

[kinow]

Really clean and well formatted code 👏

@salexan2001 I think black found a few issues, and caused the CI build to fail. Could you try running make format locally to update the code and update the PR, please?

That should fix the build. Thanks!

[salexan2001]

Thanks for the feedback. I receive the following:

$ make format
black --exclude cwltool/schemas setup.py cwltool.py cwltool tests
reformatted cwltool/singularity.py

All done! ✨ 🍰 ✨
1 file reformatted, 100 files left unchanged.

[kinow]

1 file reformatted, 100 files left unchanged.

I think if you do a git status after that, it should display the file that was reformatted for you. Then you may need to commit & push it (easier if you could squash the commits too, otherwise we can squash it during the merge).

[salexan2001]

Yes the commit with the style fix has already been pushed.

[kinow]

Yes the commit with the style fix has already been pushed.

Ah, sorry, thought you hadn't pushed that. Let me check why GH actions appears to be still unhappy.

[kinow]

Ah, good ol' mypy 😬 let me check out the branch to see if it's an easy fix.

[kinow]

This made mypy pass for me. Can you take a look and see if this path looks OK @salexan2001, please? (feel free to modify your branch/commit it if you'd like)

diff --git a/cwltool/singularity.py b/cwltool/singularity.py
index b6a3317e..a1f871bc 100644
--- a/cwltool/singularity.py
+++ b/cwltool/singularity.py
@@ -22,13 +22,13 @@ from .utils import CWLObjectType, create_tmp_dir, ensure_non_writable, ensure_wr
 
 # Cached version number of singularity
 # This is a tuple containing major and minor versions as integer.
-_SINGULARITY_VERSION: Optional[Tuple] = None
+_SINGULARITY_VERSION: Optional[Tuple[int, int]] = None
 # Cached flavor / distribution of singularity
 # Can be singularity, singularity-ce or apptainer
 _SINGULARITY_FLAVOR: str = ""
 
 
-def get_version() -> Tuple[Tuple, str]:
+def get_version() -> Tuple[Tuple[int, int], str]:
     """
     Parse the output of 'singularity --version' to determine the singularity flavor /
     distribution (singularity, singularity-ce or apptainer) and the singularity version.
@@ -52,7 +52,7 @@ def get_version() -> Tuple[Tuple, str]:
             raise RuntimeError("Output of 'singularity --version' not recognized.")
 
         version_string = version_match.group(2)
-        _SINGULARITY_VERSION = tuple([int(i) for i in version_string.split(".")])
+        _SINGULARITY_VERSION = cast(Tuple[int, int], tuple([int(i) for i in version_string.split(".")]))
         _SINGULARITY_FLAVOR = version_match.group(1)
 
         _logger.debug(
@@ -97,7 +97,7 @@ def is_version_3_1_or_newer() -> bool:
     if is_apptainer_1_or_newer():
         return True  # this is equivalent to singularity-ce > 3.9.5
     v = get_version()
-    return v[0] >= 4 or (v[0] == 3 and v[1] >= 1)
+    return v[0][0] >= 4 or (v[0][0] == 3 and v[0][1] >= 1)
 
 
 def is_version_3_4_or_newer() -> bool:
@@ -105,7 +105,7 @@ def is_version_3_4_or_newer() -> bool:
     if is_apptainer_1_or_newer():
         return True  # this is equivalent to singularity-ce > 3.9.5
     v = get_version()
-    return v[0] >= 4 or (v[0] == 3 and v[1] >= 4)
+    return v[0][0] >= 4 or (v[0][0] == 3 and v[0][1] >= 4)
 
 
 def _normalize_image_id(string: str) -> str:
diff --git a/tests/test_environment.py b/tests/test_environment.py
index 2c67aa09..e8b77cd2 100644
--- a/tests/test_environment.py
+++ b/tests/test_environment.py
@@ -131,10 +131,10 @@ class Singularity(CheckHolder):
         }
 
         # Singularity variables appear to be in flux somewhat.
-        version = get_version().split(".")
-        vmajor = int(version[0])
+        version = get_version()
+        vmajor = version[0][0]
         assert vmajor == 3, "Tests only work for Singularity 3"
-        vminor = int(version[1])
+        vminor = version[0][1]
         sing_vars: EnvChecks = {
             "SINGULARITY_CONTAINER": None,
             "SINGULARITY_NAME": None,

[salexan2001]

Thanks, looks good. I'm struggling a little bit with fixing the number of possible version digits in the type (tuple) as the apptainer-fork of singularity uses a three-digit version number.

I think, I'll change the type to a list then.

Can I check out your changes/commit somehow? I have not found it in git.

[kinow]

Can I check out your changes/commit somehow? I have not found it in git.

Hi @salexan2001 , sorry I did not push these changes, it was only a quick test to see what was necessary to get mypy to pass on my environment. Feel free to apply the diff locally if you'd like 👍 (if you'd like, you can copy it to a file and try git apply filename.diff)

[salexan2001]

Hi, I applied the patch along with some minor amendments and furthermore added some unit tests for the functions.

[kinow]

Formatting issues @salexan2001 . tox failed in GH Actions (see its logs for a complete output). Probably a good idea to set up the dev environment, if you haven't, and try using tox and/or make locally (see CONTRIBUTING.md).

[mr-c]

@salexan2001 I pushed some fixes up; thanks a lot for doing this!

[mr-c]

I don't have singularity and singularity-ce here, yet (so untested). Are these distributions tested automatically?

Currently we test with Singularity 3.8.3:

cwltool/.github/workflows/ci-tests.yml

Line 15 in 61c13dc

singularity_version: 3.8.3