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

Avoid expr substr in spades_compile.sh #1038

Open
wants to merge 1 commit into
base: spades_3.15.5
Choose a base branch
from

Conversation

epruesse
Copy link

@epruesse epruesse commented Nov 2, 2022

expr substr is not defined in posix and not implemented in MacOS /bin/expr. The much simpler bash substring expansion should be more widely compatible. It does work with MacOS' ancient /bin/bash.

`expr substr` is not defined in posix and not implemented in MacOS /bin/expr. The much simpler bash substring expansion should be more widely compatible. It does work with MacOS' ancient /bin/bash.
@asl
Copy link
Member

asl commented Nov 2, 2022

Great, thanks! Yes, spades_compile.sh is a bit complicated. Tagging @uncerso for review

@epruesse
Copy link
Author

epruesse commented Nov 3, 2022

Great, thanks! Yes, spades_compile.sh is a bit complicated. Tagging @uncerso for review

For what it does, yes. Looks like someone had some fun with bash, but didn't know case/esac could do glob matching. Something like below would be easier to read. But then as long as the script works, it's all fine.

PASS_THROUGH_OPTS=()
while [[ $# -gt 0 ]]; do
  case $1 in
    -j|--threads)
      THREADS="$2"
      shift
      shift
   ;;
    *)
      PASS_THROUGH_OPTS+=("$1")
      shift
    ;;
  esac
done

Mostly I was just passing on patch from bioconda I had to add to get spades to even start compiling :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants