search

qiime2 / q2cli

Command line interface for QIIME 2
BSD 3-Clause "New" or "Revised" License
19 stars 24 forks source link

TST: fix roundtrip on /bin/sh #328

Closed ebolyen closed 8 months ago

ebolyen commented 8 months ago

I believe we're running into an issue with the format of the subprocess run when testing with the new collection stuff: https://github.com/qiime2/distributions/actions/runs/7281173631/job/19841955368?pr=62#step:8:267.

This change will probably fix the issue, as shell=True is usually intended to be done with a single string. It hasn't come up as an issue but I think the new collection syntax is finally making it break (in /bin/sh only, bash works fine, hence not seeing it locally).

ebolyen commented 8 months ago

actually, I don't think this is it either

ebolyen commented 8 months ago

Seems like it's going to depend on your sh implementation, but often arrays aren't supported as they didn't exist in the original.

https://unix.stackexchange.com/questions/253892/syntax-error-unexpected-when-creating-an-array

So I guess we need to detect if it's linux and use /bin/bash as the shell executable for subprocess.run as the behavior on GH workers is just not suitable. (I would like the keep zsh around on osx, as that's useful.)