bioconda / bioconda-recipes

Conda recipes for the bioconda channel.
https://bioconda.github.io
MIT License
1.61k stars 3.2k forks source link

spades v3.15.x incompatible with setuptools v70.x #49120

Open dfornika opened 1 month ago

dfornika commented 1 month ago

We noticed an issue with our installation of spades in a conda environment used to test an influenza virus analysis tool called FluViewer here:

https://github.com/BCCDC-PHL/FluViewer/issues/37

The error message from spades was:

'dict' object has no attribute 'add' 
Traceback (most recent call last):
  File "bin/spades.py", line 626, in main
    pipeline.generate_configs(cfg, spades_home, tmp_configs_dir)
  File "share/spades/spades_pipeline/stages/pipeline.py", line 43, in generate_configs
    stage.generate_config(cfg)
  File "share/spades/spades_pipeline/stages/spades_stage.py", line 379, in generate_config
    stage.generate_config(self.cfg)
  File "share/spades/spades_pipeline/stages/spades_iteration_stage.py", line 144, in generate_config
    dir_util.copy_tree(os.path.join(self.tmp_configs_dir, "debruijn"), dst_configs, preserve_times=False)
  File "lib/python3.9/site-packages/setuptools/_distutils/dir_util.py", line 146, in copy_tree
    mkpath(dst, verbose=verbose)
  File "lib/python3.9/site-packages/setuptools/_distutils/dir_util.py", line 82, in mkpath
    _path_created.add(abs_head) 
AttributeError: 'dict' object has no attribute 'add'

after a bit of trial-and-error I've found that this:

conda create -n spades-3.15.3-test spades=3.15.3

...produces a broken spades installation, while this:

conda create -n spades-3.15.3-test spades=3.15.3 setuptools=69

...produces a working installation.

This doesn't seem to affect spades v4.0.0, so the latest version isn't affected.

Could we deploy a fix for the last couple of releases in the spades v3.15.x series (v3.15.3, v3.15.4, v3.15.5)? If others are able to reproduce/confirm the issue I'd imagine there are a lot of environments out there using those versions of spades.

martin-g commented 1 month ago

Could we deploy a fix for the last couple of releases in the spades v3.15.x series (v3.15.3, v3.15.4, v3.15.5)?

Sure! Would you like to send a Pull Request ? One question: Why several versions ? Wouldn't 3.15.5 be enough ?

dfornika commented 1 month ago

I could potentially submit a PR but if someone else gets to it first I'd be fine with that.

I guess my reasoning for doing multiple versions is this:

There must be many environment.yaml files out there that list those versions of spades, and many of them are probably being used to create new conda envs repeatedly (various people setting up their dev environments, deployments on cloud platforms, etc.) If I understand correctly, any env that pulls in any of those versions of spades (possibly going back further, I haven't checked) and also includes setuptools 70 or later, will be broken.

So one possibility would be to just fix 3.15.5, and expect that anyone on an older version with a broken installation should just update their spades version to that. But if they're using the conda env for any sort of "production" workflow that has been validated, they may not be free to do that so easily. They may need to set up a verification experiment just to demonstrate that there isn't some small difference in that version of spades that matters to their analysis.

Another option would be to have everyone with those older versions of spades in their environment.yml files also add setuptools=69 to their env. That's what we've done here. But it seems to me that it's the spades recipe's responsibility to make sure it is asking for specific versions of anything it needs to avoid breaking incompatibilities.

...so by that reasoning, I think it would make sense to fix the most recent few releases in the v3.15.x series. It would also seem completely reasonable to just start with v3.15.5, and have this issue and the subsequent PR to fix that version as "breadcrumbs" for the next person in case they actually run in to the specific scenario I've described.

corneliusroemer commented 1 month ago

To be clear, this requires a repodata patch, correct?