Skip to content

Avoid expr substr in spades_compile.sh#1038

Open
epruesse wants to merge 1 commit into
ablab:spades_3.15.5from
epruesse:patch-1
Open

Avoid expr substr in spades_compile.sh#1038
epruesse wants to merge 1 commit into
ablab:spades_3.15.5from
epruesse:patch-1

Conversation

@epruesse

@epruesse epruesse commented Nov 2, 2022

Copy link
Copy Markdown

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

asl commented Nov 2, 2022

Copy link
Copy Markdown
Member

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

@epruesse

epruesse commented Nov 3, 2022

Copy link
Copy Markdown
Author

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