Proper quoting of bash variables enables correct tests in bash

[nylander]

Description of the bug

While error checking I noticed the same error in several of the .command.log files: "unary operator expected".

The reason seems to be that in the corresponding .command.sh files, a variable, preserve5p, was empty, leading to a (generated) code like this (notice the empty space before the first =):

if [   = "--preserve5p" ] && [ N = "N" ]; then
...

On line #1161 in main.nf, you use a ternary operator to set the variable to either params.preserve5p or '' (i.e., empty string).

The problem arises later (on line #1171 in main.nf), where the first part of the test fails

if [ ${preserve5p}  = "--preserve5p" ] && [ ${mergedonly} = "N" ]; then
...

Possible solution?

Make sure variables are quoted correctly in the generated bash scripts. For example:

if [ "${preserve5p}"  = "--preserve5p" ] && [ "${mergedonly}" = "N" ]; then
...

Disclaimer: I do not know the reason why the preserve5p variable was empty in the first case. Proper quoting might take care of the testing (of an empty string), but the empty variable may have been unintended and induced some other way. I have not checked further.

Steps to reproduce

$ preserve5p=''
$ if [ ${preserve5p} = "foo" ] ; then echo ; fi
bash: [: =: unary operator expected
$ if [ "${preserve5p}" = "foo" ] ; then echo ; fi

Steps to reproduce the behaviour:

1. Command line:

    nextflow run nf-core/eager \
        -c config/My.config \
        -profile My,docker \
        --input 'data/samples.tsv'

2. See error: Please provide your error message

Expected behaviour

Log files

Have you provided the following extra information/files:

• The command used to run the pipeline
• The .nextflow.log file
• The exact error:

System

• Hardware: Desktop
• OS: Linux
• nf-core/eager v2.3.1
• nf-core/eager [pensive_kalam] - revision: 29b6e14 [master]
• Version: 20.10.0 build 5430
• Created: 01-11-2020 15:14 UTC (16:14 CEST)
• System: Linux 5.4.0-60-generic
• Runtime: Groovy 3.0.5 on OpenJDK 64-Bit Server VM 11.0.9.1+1-Ubuntu-0ubuntu1.20.04
• Encoding: UTF-8 (UTF-8)
• Process: 4019923@s [127.0.1.1]
• CPUs: 48 - Mem: 251.6 GB (50.6 GB) - Swap: 2 GB (2 GB)

Container engine

• Engine: Docker
• version:
• Image tag:

Additional context

[jfy133]

Ha! I'm glad someone else has found this.

I'm having the exact same problem on a new cluster I'm setting up eager on. I have absolutely no clue why this issue is coming up now, as I don't have this problem on my computer nor 5 other clusters I have set it up on.

But if this issue is replicable, as in your case, then it's good to know it's not some weird edge case.

I'll let you know if I find a solution. I have already tried fixing this as you suggest, but for some reason the process results in exit code of 1 even though everything completes properly.... No clue why

[jfy133]

Hi @nylander does this version work for you: nextflow run nf-core/eager -r java-tool-fixes <...>? You might need to run nextflow pull nf-core/eager -r java-tool-fixes beforehand

[apeltzer]

Maybe another option would be to abandon this as Bash code and either have a separate AdapterRemoval merging process that does the merging based on the parameters set / used and/or add this in groovy syntax? That seems to be more stable 😓

[jfy133]

Agreed, I think it's safer. This will be a bit of change, but worth it in the long-run I guess.

From Alex's list (sent via slack), I will update the following:

• adapter_removal (quite a lot)
• samtools_filter (quite a lot)
• dedup (renaming can be done via nextflow more easily)
• markduplicates (renaming as in dedup)
• sex_det (only for loop)

[jfy133]

This should be fixed now by replacing all bash logic with groovy. @nylander would be grateful if you could test the latest dev branch for fthis - reopen the issue if needs be.