Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some Singularity versions cannot be parsed in v7.29.0 #2319

Closed
boasvdp opened this issue Jun 26, 2023 · 3 comments · Fixed by #2337
Closed

Some Singularity versions cannot be parsed in v7.29.0 #2319

boasvdp opened this issue Jun 26, 2023 · 3 comments · Fixed by #2337
Labels
bug Something isn't working

Comments

@boasvdp
Copy link

boasvdp commented Jun 26, 2023

Snakemake version

7.29.0

Describe the bug

Singularity releases are now checked according to PEP440, to which they do not comply. This appears to be due to the change made in commit 9b8c362, where distutils.version.LooseVersion is replaced by packaging.version.parse.

This happens on my work laptop (singularity --version returns singularity-ce version 3.10.4-dirty) and on our HPC (singularity --version returns singularity-ce version 3.11.1-1.el7). Many Singularity releases (e.g. all releases for RHEL) have non-PEP440-compliant suffixes.

Logs

Traceback (most recent call last):
  File "/home/boas/mambaforge/lib/python3.10/site-packages/snakemake/__init__.py", line 771, in snakemake
    success = workflow.execute(
  File "/home/boas/mambaforge/lib/python3.10/site-packages/snakemake/workflow.py", line 919, in execute
    dag.pull_container_imgs(
  File "/home/boas/mambaforge/lib/python3.10/site-packages/snakemake/dag.py", line 367, in pull_container_imgs
    img.pull(dryrun)
  File "/home/boas/mambaforge/lib/python3.10/site-packages/snakemake/deployment/singularity.py", line 46, in pull
    self.singularity.check()
  File "/home/boas/mambaforge/lib/python3.10/site-packages/snakemake/deployment/singularity.py", line 179, in check
    if parse(v) < parse("2.4.1"):
  File "/home/boas/mambaforge/lib/python3.10/site-packages/packaging/version.py", line 52, in parse
    return Version(version)
  File "/home/boas/mambaforge/lib/python3.10/site-packages/packaging/version.py", line 198, in __init__
    raise InvalidVersion(f"Invalid version: '{version}'")
packaging.version.InvalidVersion: Invalid version: '3.10.4-dirty
'

Minimal example

Using Snakefile:

rule all:
    input:
        "out.txt"

rule whalesay:
    output:
        "out.txt"
    container:
        "docker://docker/whalesay:latest"
    shell:
        """
cowsay boo > {output}
        """

And then running:

# At the time of writing, v7.29.0 is not on conda yet
pip install snakemake==7.29.0
snakemake --version
singularity --version
snakemake --use-singularity -j 1

While this works fine:

singularity pull "docker://docker/whalesay:latest"
singularity exec whalesay_latest.sif cowsay boo

Additional context

@boasvdp boasvdp added the bug Something isn't working label Jun 26, 2023
@ASLeonard
Copy link
Contributor

Not surprising, but it is the same for the other if statement checking singularity->apptainer version

packaging.version.InvalidVersion: Invalid version: '1.1.8-1.el7

Obviously not ideal, but this is an intentional decision to not handle such non-PEP440 versions. Although the minimum allowed singularity version was released in Nov 22, 2017, so maybe its just better to remove the version check and feel sorry for anyone using such an old version (which wouldn't run anyway in snakemake).

@maarten-k
Copy link
Contributor

Removing the check is a bad idea in my opinion: there are enough cluster systems around with ancient software installed on them.

There are multiple ways to solve this error:

  • strip the last bit of characters if Parse could not parse it and try again
  • use a module that implements LooseVersion (the question is how well it is maintained)
  • implement an own version of LooseVersion

I tend to the first solution, but please let me know if there is a better solution.

@boasvdp
Copy link
Author

boasvdp commented Jun 29, 2023

I'm also in favour of the first option where characters are stripped if parse does not work. I have access to at least one system which is running an unsupported version of Singularity, so I can imagine the check remains necessary

johanneskoester added a commit that referenced this issue Jun 30, 2023
### Description
Apptainer and singularity use non-PEP440 version numbers like
`1.1.8-1.el8` and `3.10.4-dirty`. packaging.version can not handle these
version numbers. To mitigate the issue, a character is stripped away at
the end of the string until it is parsable by packaging.version.

Should fix #2319  and #2334

### QC
<!-- Make sure that you can tick the boxes below. -->

* [x] The PR contains a test case for the changes or the changes are
already covered by an existing test case.
* [x] The documentation (`docs/`) is updated to reflect the changes or
this is not necessary (e.g. if the change does neither modify the
language nor the behavior or functionalities of Snakemake).

---------

Co-authored-by: Johannes Köster <johannes.koester@tu-dortmund.de>
Co-authored-by: Johannes Köster <johannes.koester@uni-due.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants