[17.01] Fix order of PATH when doing hacky Conda activating. #3620
Conversation
| @@ -325,7 +325,7 @@ def shell_commands(self, requirement): | |||
| # On explicit testing the only such requirement I am aware of is samtools - and it seems to work | |||
| # fine with just appending the PATH as done below. Other tools may require additional | |||
There was a problem hiding this comment.
Rebased with this fix, thanks!
Thanks to @yhoogstrate's comment here galaxyproject@fe2a1d8#commitcomment-20900607. With comment fix thanks to 🦅👀.
jmchilton
force-pushed
the
fix_python_conda_hack
branch
from
97eaf40
to
c55c3ec
Compare
|
I'm not crazy about this change. IUC shouldn't be putting samtools in the global PATH in such a hacky way as we do currently. If a python binary happens to be in the samtools environment (admittedly that shouldn't happen), we're back to square one. I think the proper fix is to install samtools into __samtools@uv in the travis recipe. |
|
Regardless of how the IUC should configure its testing - don't you think binaries from "required" Conda recipes should take precedent over generic background ones that might not even be setup for Galaxy - let alone that particular requirement? |
|
But this only happens for tools that are marked to preserve the python environment. So in fact this already happens?! |
|
See also fe2a1d8#commitcomment-20931952. |
|
So the tool in question is SAM-to-BAM, which happens to be on the preserve python list. But the current version doesn't need an importable galaxy ... I'm OK to merge this, and then we can augment the mechanism to take into account the version as well. |
That actually makes sense to me - now I see. The better fix here is to apply that hack only to older versions of the tool - that logic needs to be added anyway. I'm leaning away from merging this PR. |
This is a less broken alternative to galaxyproject#3620 and something I intended to ultimately do anyway. sam-to-bam used to be broken but now it isn't, we need to express that. Ideally we would move most of the things from the unversioned list to the versioned list as they are fixed. xref galaxyproject#3620 (comment) xref galaxyproject@fe2a1d8#commitcomment-20900607 xref galaxyproject#3351 xref galaxyproject/tools-iuc#1148
Thanks to @yhoogstrate's comment here fe2a1d8#commitcomment-20900607.