UCSF-Costello-Lab / LG3_Pipeline

The original LG3 pipeline
https://github.com/UCSF-Costello-Lab/LG3_Pipeline
0 stars 0 forks source link

develop_c4: Add back `# shellcheck source=scripts/utils.sh` #153

Open HenrikBengtsson opened 2 years ago

HenrikBengtsson commented 2 years ago

Hi again.

Issue

I see that you've dropped the most essential ShellCheck source directive:

# shellcheck source=scripts/utils.sh
source "${LG3_HOME:?}/scripts/utils.sh"

in basically all files. That is the reason why you got SC1091 warnings. So, the proper solution is to keep the above directive and not disable SC1091 checks.

For example,

$ git checkout develop_c4
$ git --no-pager diff 2020-05-26 -- Merge.pbs
diff --git a/Merge.pbs b/Merge.pbs
index 316ae89..a1070ba 100755
--- a/Merge.pbs
+++ b/Merge.pbs
@@ -7,7 +7,7 @@
 #PBS -l nodes=1:ppn=6,vmem=32gb
 #PBS -m ae

-# shellcheck source=scripts/utils.sh
+# shellcheck disable=SC1072,SC1073
 source "${LG3_HOME:?}/scripts/utils.sh"
 source_lg3_conf

@@ -22,12 +22,12 @@ LG3_HOME=${LG3_HOME:?}
 LG3_OUTPUT_ROOT=${LG3_OUTPUT_ROOT:-output}
 PROJECT=${PROJECT:-LG3}
 EMAIL=${EMAIL:?}
-LG3_SCRATCH_ROOT=${LG3_SCRATCH_ROOT:-/scratch/${USER:?}/${PBS_JOBID}}
+LG3_SCRATCH_ROOT=${TMPDIR:-/scratch/${SLURM_JOB_USER}/${SLURM_JOB_ID}}
 LG3_DEBUG=${LG3_DEBUG:-true}

 ### Debug
-if [[ $LG3_DEBUG ]]; then
-  echo "Settings:"
+if $LG3_DEBUG ; then
+  echo "Debug info:"
   echo "- LG3_HOME=$LG3_HOME"
   echo "- LG3_OUTPUT_ROOT=$LG3_OUTPUT_ROOT"
   echo "- EMAIL=${EMAIL}"
@@ -36,6 +36,8 @@ if [[ $LG3_DEBUG ]]; then
   echo "- USER=$USER"
   echo "- PBS_NUM_PPN=$PBS_NUM_PPN"
   echo "- hostname=$(hostname)"
+  echo "- node(s): ${SLURM_JOB_NODELIST}"
+  echo "- SLURM_NTASKS: ${SLURM_NTASKS}"
 fi

 ### Input

Action

Please add back:

# shellcheck source=scripts/utils.sh
source "${LG3_HOME:?}/scripts/utils.sh"
ivan108 commented 2 years ago

Which version of shellcheck are you using? Are you using -x flag? I use 0.7.2

[jocostello@c4-dev2 repo]$ shellcheck --version
ShellCheck - shell script analysis tool
version: 0.7.2

I started removing # shellcheck source=scripts/utils.sh because it doesn't work.

Here is an experiment with Align_fastq.pbs

Case 1

#!/bin/bash
source "${LG3_HOME:?}/scripts/utils.sh"
source_lg3_conf

[jocostello@c4-dev2 repo]$ shellcheck Align_fastq.pbs

In Align_fastq.pbs line 3:
source "${LG3_HOME:?}/scripts/utils.sh"
       ^-- SC1091: Not following: ./scripts/utils.sh was not specified as input (see shellcheck -x).

[jocostello@c4-dev2 repo]$ shellcheck -x Align_fastq.pbs [no message, so -x disables SC1091 warning ]

Case 2

#!/bin/bash
# shellcheck source=scripts/utils.sh
source "${LG3_HOME:?}/scripts/utils.sh"
source_lg3_conf

[jocostello@c4-dev2 repo]$ shellcheck Align_fastq.pbs

In Align_fastq.pbs line 4:
source "${LG3_HOME:?}/scripts/utils.sh"
       ^-- SC1091: Not following: scripts/utils.sh was not specified as input (see shellcheck -x).

So, adding "# shellcheck source=scripts/utils.sh" didn't help!?

But -x helps again: [jocostello@c4-dev2 repo]$ shellcheck -x Align_fastq.pbs [no warning]

What am I missing??

HenrikBengtsson commented 2 years ago

I started removing # shellcheck source=scripts/utils.sh because it doesn't work.

Please talk to me before you turn to these ad-hoc workarounds. I'm only adding things that I have tested very careful, so please assume it should work. If you think there's a bug, please post an issue and we'll discuss.

The checks can be run locally using:

$ make check

If you look at the Makefile, you'll see that it uses shellcheck -x ..., i.e. if you run manually, then yes, you should use -x. You can also see the CI checks that run on GitHub Actions uses this, cf. https://github.com/UCSF-Costello-Lab/LG3_Pipeline/runs/3798024504?check_suite_focus=true#step:4:1.