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

Use shellcheck to verify shell scripts? #161

Open
kbumsik opened this issue Aug 4, 2020 · 2 comments
Open

Use shellcheck to verify shell scripts? #161

kbumsik opened this issue Aug 4, 2020 · 2 comments
Labels
Contributhon OpenSource Contributhon 2020

Comments

@kbumsik
Copy link
Contributor

kbumsik commented Aug 4, 2020

This repo has a lot of (16) shell scripts. So it would be great if we have certain rules for shell scripts to improve maintainability.
The useful tool is Shellchek. We can easily integrate this using ShellCheck Github action.
To run this locally you can install it using sudo snap install shellcheck. I use snap because the version of APT package is too old.

Here is an example of running shellcheck for all shell scripts in the repo. I excluded rule SC2164 because it triggers too much warnings that doesn't seem to worth it to me:

(click to expand) find . -name *.sh -exec shellcheck -e SC2164 {} +

In ./bash_script/example_image_segmentation_tensorflow_lite/gst-launch-image-seg-flatbuf-edgetpu-server.sh line 15:
tensor_decoder mode=flatbuf ! gdppay ! tcpserversink host=${ip} port=${port}
                                                          ^---^ SC2086: Double quote to prevent globbing and word splitting.
                                                                     ^-----^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
tensor_decoder mode=flatbuf ! gdppay ! tcpserversink host="${ip}" port="${port}"


In ./bash_script/example_image_segmentation_tensorflow_lite/gst-launch-image-seg-flatbuf-edgetpu-client.sh line 25:
    tcpclientsrc host=${ip} port=${port} ! gdpdepay ! other/flatbuf-tensor ! tensor_converter ! tee name=t \
                      ^---^ SC2086: Double quote to prevent globbing and word splitting.
                                 ^-----^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
    tcpclientsrc host="${ip}" port="${port}" ! gdpdepay ! other/flatbuf-tensor ! tensor_converter ! tee name=t \


In ./bash_script/example_models/get-model-image-classification-tflite.sh line 18:
if echo $0 | grep -q -w "get-model-image-classification-tflite.sh"; then 
        ^-- SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
if echo "$0" | grep -q -w "get-model-image-classification-tflite.sh"; then 


In ./bash_script/example_models/get-model.sh line 18:
if echo $0 | grep -q -w "get-model-image-classification-tflite.sh"; then 
        ^-- SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
if echo "$0" | grep -q -w "get-model-image-classification-tflite.sh"; then 


In ./android/IoT-ARM64/02-gst-android-with-cerbero/additional-patches/build-all.sh line 9:
if [[ $? -eq 0 ]]; then
      ^-- SC2181: Check exit code directly with e.g. 'if mycmd;', not indirectly with $?.


In ./Tizen.native/PipelineSample/fetch_shared_object.sh line 30:
if [ -f $rpmfile ]; then
        ^------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
if [ -f "$rpmfile" ]; then


In ./Tizen.native/PipelineSample/fetch_shared_object.sh line 31:
  rm $rpmfile
     ^------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
  rm "$rpmfile"


In ./Tizen.native/PipelineSample/fetch_shared_object.sh line 36:
sofilepath=$(rpm2cpio $rpmfile | cpio -t | grep $sofile)
                      ^------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
sofilepath=$(rpm2cpio "$rpmfile" | cpio -t | grep $sofile)


In ./Tizen.native/PipelineSample/fetch_shared_object.sh line 37:
rpm2cpio $rpmfile | cpio -id $sofilepath
         ^------^ SC2086: Double quote to prevent globbing and word splitting.
                             ^---------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
rpm2cpio "$rpmfile" | cpio -id "$sofilepath"


In ./Tizen.native/PipelineSample/fetch_shared_object.sh line 38:
chmod 755 $sofilepath
          ^---------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
chmod 755 "$sofilepath"


In ./Tizen.native/PipelineSample/fetch_shared_object.sh line 39:
mv -f $sofilepath ./res/.
      ^---------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
mv -f "$sofilepath" ./res/.


In ./Tizen.native/PipelineSample/fetch_shared_object.sh line 44:
rm -rf $rpmfile
       ^------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
rm -rf "$rpmfile"


In ./Tizen.platform/Tizen_IoT_text_classification_NonGUI/gen_tizen_iot_text_classification_rpm.sh line 10:
git add *
        ^-- SC2035: Use ./*glob* or -- *glob* so names with dashes won't become options.


In ./Tizen.platform/Vivante_single_NonGUI_inceptionV3/gen_vivante-inceptionv3-single_rpm.sh line 10:
git add *
        ^-- SC2035: Use ./*glob* or -- *glob* so names with dashes won't become options.


In ./Tizen.platform/Vivante_pipeline_experiments/gen_vivante-pipeline-experiment_rpm.sh line 10:
git add *
        ^-- SC2035: Use ./*glob* or -- *glob* so names with dashes won't become options.


In ./Tizen.platform/Vivante_pipeline_experiments/check_usage.sh line 33:
time vivante-pipeline-experiment $ENABLE_VNN_INCEPTION $ENABLE_VNN_YOLO $ENABLE_TFLITE_INCEPTION $SLEEP_TIME &
                                 ^-------------------^ SC2086: Double quote to prevent globbing and word splitting.
                                                       ^--------------^ SC2086: Double quote to prevent globbing and word splitting.
                                                                        ^----------------------^ SC2086: Double quote to prevent globbing and word splitting.
                                                                                                 ^---------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
time vivante-pipeline-experiment "$ENABLE_VNN_INCEPTION" "$ENABLE_VNN_YOLO" "$ENABLE_TFLITE_INCEPTION" "$SLEEP_TIME" &


In ./Tizen.platform/Vivante_pipeline_experiments/check_usage.sh line 34:
process_id=`/bin/ps -fu $USER| grep "vivante-pipeline-experiment" | grep -v "grep" | awk '{print $2}'`
           ^-- SC2006: Use $(...) notation instead of legacy backticked `...`.
                        ^---^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
process_id=$(/bin/ps -fu "$USER"| grep "vivante-pipeline-experiment" | grep -v "grep" | awk '{print $2}')


In ./Tizen.platform/Vivante_pipeline_experiments/check_usage.sh line 36:
taskset -cp 2-5 $process_id # set affinity
                ^---------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
taskset -cp 2-5 "$process_id" # set affinity


In ./Tizen.platform/Vivante_pipeline_experiments/check_usage.sh line 37:
end=$((SECONDS+$SLEEP_TIME+1))
^-^ SC2034: end appears unused. Verify use (or export if used externally).
               ^---------^ SC2004: $/${} is unnecessary on arithmetic variables.


In ./Tizen.platform/Vivante_pipeline_experiments/check_usage.sh line 39:
PEAK=0
^--^ SC2034: PEAK appears unused. Verify use (or export if used externally).


In ./Tizen.platform/Vivante_pipeline_experiments/check_usage.sh line 40:
MIN=10000000000000
^-^ SC2034: MIN appears unused. Verify use (or export if used externally).


In ./Tizen.platform/Vivante_pipeline_experiments/check_usage.sh line 41:
CNT=0
^-^ SC2034: CNT appears unused. Verify use (or export if used externally).


In ./Tizen.platform/Vivante_pipeline_experiments/check_usage.sh line 42:
SUM_MEM=0
^-----^ SC2034: SUM_MEM appears unused. Verify use (or export if used externally).


In ./Tizen.platform/Vivante_pipeline_experiments/check_usage.sh line 45:
  sleep $SLEEP_TIME
        ^---------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
  sleep "$SLEEP_TIME"


In ./Tizen.platform/Vivante_pipeline_experiments/check_usage.sh line 46:
  output=`grep VmRSS /proc/$process_id/status`
         ^-- SC2006: Use $(...) notation instead of legacy backticked `...`.
                           ^---------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
  output=$(grep VmRSS /proc/"$process_id"/status)


In ./Tizen.platform/Vivante_pipeline_experiments/check_usage.sh line 48:
  mem_usage=$((${mem_str[1]}))
               ^-----------^ SC2004: $/${} is unnecessary on arithmetic variables.


In ./Tizen.platform/Vivante_pipeline_NonGUI_yoloV3/gen_vivante-yolov3-pipeline_rpm.sh line 10:
git add *
        ^-- SC2035: Use ./*glob* or -- *glob* so names with dashes won't become options.


In ./Tizen.platform/Vivante_pipeline_NonGUI_inceptionV3/gen_vivante-inceptionv3-pipeline_rpm.sh line 10:
git add *
        ^-- SC2035: Use ./*glob* or -- *glob* so names with dashes won't become options.


In ./templates/tensor_filter_subplugin/deploy.sh line 7:
TARGET=$(pwd)
^----^ SC2034: TARGET appears unused. Verify use (or export if used externally).


In ./templates/tensor_filter_subplugin/deploy.sh line 8:
BASEPATH=`dirname "$0"`
^------^ SC2034: BASEPATH appears unused. Verify use (or export if used externally).
         ^------------^ SC2006: Use $(...) notation instead of legacy backticked `...`.

Did you mean: 
BASEPATH=$(dirname "$0")


In ./templates/tensor_filter_subplugin/deploy.sh line 9:
BASENAME=`basename "$0"`
         ^-------------^ SC2006: Use $(...) notation instead of legacy backticked `...`.

Did you mean: 
BASENAME=$(basename "$0")


In ./templates/tensor_filter_subplugin/deploy.sh line 14:
        printf "usage: ${BASENAME} <name> <path to create>\n\n"
               ^-- SC2059: Don't use variables in the printf format string. Use printf '..%s..' "$foo".


In ./templates/tensor_filter_subplugin/deploy.sh line 21:
path="$2"
^--^ SC2034: path appears unused. Verify use (or export if used externally).


In ./templates/tensor_filter_subplugin/deploy.sh line 26:
        printf "The name \"$1\" contains a whitespace. Cannot proceed.\n\n"
               ^-- SC2059: Don't use variables in the printf format string. Use printf '..%s..' "$foo".


In ./templates/tensor_filter_subplugin/deploy.sh line 32:
        printf "Please provide name with A-Z, a-z, -, _, 0-9 only. The name \"$1\" is not such a name. Cannot proceed.\n\n"
               ^-- SC2059: Don't use variables in the printf format string. Use printf '..%s..' "$foo".


In ./templates/tensor_filter_subplugin/deploy.sh line 37:
if [ -e $2 ]
        ^-- SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
if [ -e "$2" ]


In ./templates/tensor_filter_subplugin/deploy.sh line 39:
        printf "The path $2 already exists. Please designate a new path.\n\n"
               ^-- SC2059: Don't use variables in the printf format string. Use printf '..%s..' "$foo".


In ./templates/tensor_filter_subplugin/deploy.sh line 43:
mkdir -p $2
         ^-- SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
mkdir -p "$2"


In ./templates/tensor_filter_subplugin/deploy.sh line 44:
if [ ! -d $2 ]
          ^-- SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
if [ ! -d "$2" ]


In ./templates/tensor_filter_subplugin/deploy.sh line 46:
        printf "Failed to create a new directory $2.\n\n"
               ^-- SC2059: Don't use variables in the printf format string. Use printf '..%s..' "$foo".


In ./templates/tensor_filter_subplugin/deploy.sh line 50:
printf "Initializing a git repo of tensor-filter at $2.\n"
       ^-- SC2059: Don't use variables in the printf format string. Use printf '..%s..' "$foo".


In ./templates/tensor_filter_subplugin/deploy.sh line 51:
pushd $2
      ^-- SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
pushd "$2"


In ./templates/tensor_filter_subplugin/deploy.sh line 55:
cp -R src packaging meson.build $2/
                                ^-- SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
cp -R src packaging meson.build "$2"/


In ./templates/tensor_filter_subplugin/deploy.sh line 57:
pushd $2
      ^-- SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
pushd "$2"


In ./templates/tensor_filter_subplugin/deploy.sh line 60:
mv tensor-filter-TEMPLATE.manifest tensor-filter-${name}.manifest
                                                 ^-----^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
mv tensor-filter-TEMPLATE.manifest tensor-filter-"${name}".manifest


In ./templates/tensor_filter_subplugin/deploy.sh line 61:
mv tensor-filter-TEMPLATE.spec.in tensor-filter-${name}.spec
                                                ^-----^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
mv tensor-filter-TEMPLATE.spec.in tensor-filter-"${name}".spec


In ./templates/tensor_filter_subplugin/deploy.sh line 63:
sed -i "s|TEMPLATE|${name}|" tensor-filter-${name}.spec
                                           ^-----^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
sed -i "s|TEMPLATE|${name}|" tensor-filter-"${name}".spec

For more information:
  https://www.shellcheck.net/wiki/SC2034 -- BASEPATH appears unused. Verify u...
  https://www.shellcheck.net/wiki/SC2035 -- Use ./*glob* or -- *glob* so name...
  https://www.shellcheck.net/wiki/SC2059 -- Don't use variables in the printf...

Some notable rules are SC2034 (unused variables) and SC2006 (use $() instead of non-posix backticks)

What do you think about this? I could make a PR to fix warnings if you like it.

@taos-ci
Copy link
Collaborator

taos-ci commented Aug 4, 2020

:octocat: cibot: Thank you for posting issue #161. The person in charge will reply soon.

@dongju-chae dongju-chae added the Contributhon OpenSource Contributhon 2020 label Aug 5, 2020
@dongju-chae
Copy link
Member

dongju-chae commented Aug 5, 2020

Well, I'm not sure why shellcheck is disabled innnstreamer-example's CI features.

FYI, TAOS-CI already has shellcheck plugin: https://github.com/nnstreamer/TAOS-CI/blob/master/ci/taos/plugins-good/pr-prebuild-shellcheck.sh. Also, I found that nnstreamer's CI is using shellcheck.

Anyway, it would be good if any warnings can be fixed by some PRs.

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

No branches or pull requests

3 participants