Skip to content

Commit

Permalink
[egs] Use shellcheck to eliminate static analysis warnings in bash.
Browse files Browse the repository at this point in the history
This commit is not yet complete. There is a balance between following
the warnings provided by shellcheck and simply disabling some, since we
don't want to wipe out too much of git blame's history (such as not
quoting filesystem paths, since unquoted paths are all over the codebase
by now).

Right now, only egs/wsj/s5/{utils,steps} are checked. I don't know
whether it's worth it to check local/ directories.

Right now, to find static analysis warnings, I am running
tools/extras/travis_script.sh from the top-level directory.
Note that some warnings are already disabled. Shellcheck warning codes
are described in more detail at: https://github.com/koalaman/shellcheck/wiki

We will need to uncomment some code in tools/extras/travis_script.sh
before pushing this.

I have not yet tested that .travis.yml is correct yet, since I don't
know travis CI very well.
  • Loading branch information
galv committed Sep 15, 2017
1 parent ca871fb commit 656ba01
Show file tree
Hide file tree
Showing 14 changed files with 45 additions and 38 deletions.
1 change: 1 addition & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ addons:
- gfortran-4.9
- liblapack-dev
- clang-3.8
- shellcheck

branches:
only:
Expand Down
18 changes: 8 additions & 10 deletions egs/wsj/s5/utils/combine_data.sh
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ skip_fix=false # skip the fix_data_dir.sh in the end
echo "$0 $@" # Print the command line for logging

if [ -f path.sh ]; then . ./path.sh; fi
. parse_options.sh || exit 1;
. utils/parse_options.sh || exit 1;

if [ $# -lt 2 ]; then
echo "Usage: combine_data.sh [--extra-files 'file1 file2'] <dest-data-dir> <src-data-dir1> <src-data-dir2> ..."
Expand All @@ -28,14 +28,12 @@ fi
dest=$1;
shift;

first_src=$1;

rm -r $dest 2>/dev/null
mkdir -p $dest;

export LC_ALL=C

for dir in $*; do
for dir in "$@"; do
if [ ! -f $dir/utt2spk ]; then
echo "$0: no such file $dir/utt2spk"
exit 1;
Expand All @@ -46,7 +44,7 @@ done
# it is not compulsary for it to exist in src directories, but if it exists in
# even one it should exist in all. We will create the files where necessary
has_utt2uniq=false
for in_dir in $*; do
for in_dir in "$@"; do
if [ -f $in_dir/utt2uniq ]; then
has_utt2uniq=true
break
Expand All @@ -55,7 +53,7 @@ done

if $has_utt2uniq; then
# we are going to create an utt2uniq file in the destdir
for in_dir in $*; do
for in_dir in "$@"; do
if [ ! -f $in_dir/utt2uniq ]; then
# we assume that utt2uniq is a one to one mapping
cat $in_dir/utt2spk | awk '{printf("%s %s\n", $1, $1);}'
Expand All @@ -73,15 +71,15 @@ extra_files=$(echo "$extra_files"|sed -e "s/utt2uniq//g")
# segments are treated similarly to utt2uniq. If it exists in some, but not all
# src directories, then we generate segments where necessary.
has_segments=false
for in_dir in $*; do
for in_dir in "$@"; do
if [ -f $in_dir/segments ]; then
has_segments=true
break
fi
done

if $has_segments; then
for in_dir in $*; do
for in_dir in "$@"; do
if [ ! -f $in_dir/segments ]; then
echo "$0 [info]: will generate missing segments for $in_dir" 1>&2
utils/data/get_segments_for_data.sh $in_dir
Expand All @@ -97,7 +95,7 @@ fi
for file in utt2spk utt2lang utt2dur feats.scp text cmvn.scp reco2file_and_channel wav.scp spk2gender $extra_files; do
exists_somewhere=false
absent_somewhere=false
for d in $*; do
for d in "$@"; do
if [ -f $d/$file ]; then
exists_somewhere=true
else
Expand All @@ -107,7 +105,7 @@ for file in utt2spk utt2lang utt2dur feats.scp text cmvn.scp reco2file_and_chann

if ! $absent_somewhere; then
set -o pipefail
( for f in $*; do cat $f/$file; done ) | sort -k1 > $dest/$file || exit 1;
( for f in "$@"; do cat $f/$file; done ) | sort -k1 > $dest/$file || exit 1;
set +o pipefail
echo "$0: combined $file"
else
Expand Down
5 changes: 2 additions & 3 deletions egs/wsj/s5/utils/convert_slf_parallel.sh
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ word_to_node=false # Words in arcs or nodes? [default:arcs]
echo "$0 $@"

[ -f ./path.sh ] && . ./path.sh
. parse_options.sh || exit 1;
. utils/parse_options.sh || exit 1;

if [ $# -ne 3 ]; then
echo "Usage: $0 [options] <data-dir> <lang-dir|graph-dir> <decode-dir>"
Expand Down Expand Up @@ -42,7 +42,6 @@ done
echo "$0: Converting lattices into '$dir/$dirname'"

# Words in arcs or nodes? [default:nodes]
word_to_link_arg=
$word_to_node && word_to_node_arg="--word-to-node"

nj=$(cat $dir/num_jobs)
Expand All @@ -56,7 +55,7 @@ $cmd $parallel_opts JOB=1:$nj $dir/$dirname/log/lat_convert.JOB.log \
utils/convert_slf.pl $word_to_node_arg - $dir/$dirname/JOB/ || exit 1

# make list of lattices
find -L $PWD/$dir/$dirname -name *.lat.gz > $dir/$dirname/lat_htk.scp || exit 1
find -L $PWD/$dir/$dirname -name "*.lat.gz" > $dir/$dirname/lat_htk.scp || exit 1

# check number of lattices:
nseg=$(cat $data/segments | wc -l)
Expand Down
1 change: 0 additions & 1 deletion egs/wsj/s5/utils/format_lm.sh
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ for f in phones.txt words.txt topo L.fst L_disambig.fst phones/ oov.int oov.txt;
cp -r $lang_dir/$f $out_dir
done

lm_base=$(basename $lm '.gz')
gunzip -c $lm \
| arpa2fst --disambig-symbol=#0 \
--read-symbol-table=$out_dir/words.txt - $out_dir/G.fst
Expand Down
3 changes: 2 additions & 1 deletion egs/wsj/s5/utils/format_lm_sri.sh
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ fi
if [ $# -eq 4 ] ; then
lang_dir=$1
lm=$2
# shellcheck disable=2034
# This unused variable is here for backwards compatibility reasons
lexicon=$3
out_dir=$4
else
Expand Down Expand Up @@ -73,7 +75,6 @@ trap 'rm -rf "$tmpdir"' EXIT
mkdir -p $out_dir
cp -r $lang_dir/* $out_dir || exit 1;

lm_base=$(basename $lm '.gz')
awk '{print $1}' $out_dir/words.txt > $tmpdir/voc || exit 1;

# Change the LM vocabulary to be the intersection of the current LM vocabulary
Expand Down
2 changes: 1 addition & 1 deletion egs/wsj/s5/utils/lang/make_phone_bigram_lang.sh
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
echo "$0 $@" # Print the command line for logging

[ -f ./path.sh ] && . ./path.sh; # source the path.
. parse_options.sh || exit 1;
. utils/parse_options.sh || exit 1;


if [ $# != 3 ]; then
Expand Down
4 changes: 2 additions & 2 deletions egs/wsj/s5/utils/make_absolute.sh
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@
# It turns a pathname into an absolute pathname, including following soft links.
target_file=$1

cd $(dirname $target_file)
cd $(dirname $target_file) || exit 1
target_file=$(basename $target_file)

# Iterate down a (possible) chain of symlinks
while [ -L "$target_file" ]; do
target_file=$(readlink $target_file)
cd $(dirname $target_file)
cd $(dirname $target_file) || exit 1
target_file=$(basename $target_file)
done

Expand Down
8 changes: 5 additions & 3 deletions egs/wsj/s5/utils/mkgraph.sh
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,17 @@
# (this is compiled from this repository using Doxygen,
# the source for this part is in src/doc/graph_recipe_test.dox)

# shellcheck disable=2064

set -o pipefail

tscale=1.0
loopscale=0.1

remove_oov=false

for x in `seq 4`; do
[ "$1" == "--mono" -o "$1" == "--left-biphone" -o "$1" == "--quinphone" ] && shift && \
for _ in `seq 4`; do
[ "$1" == "--mono" ] || [ "$1" == "--left-biphone" ] || [ "$1" == "--quinphone" ] && shift && \
echo "WARNING: the --mono, --left-biphone and --quinphone options are now deprecated and ignored."
[ "$1" == "--remove-oov" ] && remove_oov=true && shift;
[ "$1" == "--transition-scale" ] && tscale=$2 && shift 2;
Expand Down Expand Up @@ -137,7 +139,7 @@ if [[ ! -s $dir/HCLG.fst || $dir/HCLG.fst -ot $dir/HCLGa.fst ]]; then
add-self-loops --self-loop-scale=$loopscale --reorder=true \
$model < $dir/HCLGa.fst | fstconvert --fst_type=const > $dir/HCLG.fst.$$ || exit 1;
mv $dir/HCLG.fst.$$ $dir/HCLG.fst
if [ $tscale == 1.0 -a $loopscale == 1.0 ]; then
if [ $tscale == 1.0 ] && [ $loopscale == 1.0 ]; then
# No point doing this test if transition-scale not 1, as it is bound to fail.
fstisstochastic $dir/HCLG.fst || echo "[info]: final HCLG is not stochastic."
fi
Expand Down
4 changes: 3 additions & 1 deletion egs/wsj/s5/utils/parse_options.sh
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,12 @@ done
###
while true; do
[ -z "${1:-}" ] && break; # break if there are no arguments
# shellcheck disable=2154
case "$1" in
# If the enclosing script is called with --help option, print the help
# message and exit. Scripts should put help messages in $help_message
--help|-h) if [ -z "$help_message" ]; then echo "No help found." 1>&2;
else printf "$help_message\n" 1>&2 ; fi;
else printf "%s\n" "$help_message" 1>&2 ; fi;
exit 0 ;;
--*=*) echo "$0: options to scripts must be of the form --name value, got '$1'"
exit 1 ;;
Expand All @@ -63,6 +64,7 @@ while true; do
# The test [ -z ${foo_bar+xxx} ] will return true if the variable foo_bar
# is undefined. We then have to wrap this test inside "eval" because
# foo_bar is itself inside a variable ($name).
# shellcheck disable=2016
eval '[ -z "${'$name'+xxx}" ]' && echo "$0: invalid option $1" 1>&2 && exit 1;

oldval="`eval echo \\$$name`";
Expand Down
2 changes: 1 addition & 1 deletion egs/wsj/s5/utils/prepare_online_nnet_dist_build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ ivec_extractor_files="final.dubm final.ie final.mat global_cmvn.stats online_cmv

echo "$0 $@" # Print the command line for logging
[ -f path.sh ] && . ./path.sh;
. parse_options.sh || exit 1;
. utils/parse_options.sh || exit 1;

if [ $# -ne 3 ]; then
echo "Usage: $0 <lang-dir> <model-dir> <output-tgz>"
Expand Down
2 changes: 1 addition & 1 deletion egs/wsj/s5/utils/remove_data_links.sh
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ if [ $# == 0 ]; then
echo " With --dry-run, just prints what it would do."
fi

for dir in $*; do
for dir in "$@"; do
if [ ! -d $dir ]; then
echo "$0: not a directory: $dir"
ret=1
Expand Down
4 changes: 0 additions & 4 deletions egs/wsj/s5/utils/split_data.sh
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,6 @@ else
fi

n=0;
feats=""
wavs=""
utt2spks=""
texts=""

nu=`cat $data/utt2spk | wc -l`
nf=`cat $data/feats.scp 2>/dev/null | wc -l`
Expand Down
5 changes: 3 additions & 2 deletions egs/wsj/s5/utils/subset_data_dir_tr_cv.sh
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,13 @@ uttbase=true; # by default, we choose last 10% utterances for CV

if [ "$1" == "--cv-spk-percent" ]; then
uttbase=false;
spkbase=true;
# Otherwise, do splitting based on speakers, such that the same
# speaker won't be in both train and cross validation sets.
fi

[ -f path.sh ] && . ./path.sh;

. parse_options.sh || exit 1;
. utils/parse_options.sh || exit 1;

if [ $# != 3 ]; then
echo "Usage: $0 [--cv-spk-percent P|--cv-utt-percent P] <srcdir> <traindir> <crossvaldir>"
Expand Down
24 changes: 16 additions & 8 deletions tools/extras/travis_script.sh
Original file line number Diff line number Diff line change
Expand Up @@ -66,16 +66,24 @@ CCC=$(mtoken CXX "$CXX")
#fi

echo "Building tools..." [Time: $(date)]
runvx cd tools
runvx make -j$MAXPAR openfst "$CCC" CXXFLAGS="$CF" \
OPENFST_CONFIGURE="--disable-static --enable-shared --disable-bin --disable-dependency-tracking"
cd ..
# runvx cd tools
# runvx make -j$MAXPAR openfst "$CCC" CXXFLAGS="$CF" \
# OPENFST_CONFIGURE="--disable-static --enable-shared --disable-bin --disable-dependency-tracking"
# runvx cd ..

runvx cd src
runvx touch base/.depend.mk # Fool make depend into skipping the dependency step.
runvx touch .short_version # Make version short, or else ccache will miss everything.
runvx "$CCC" CXXFLAGS="$CF" LDFLAGS="$LDF" ./configure --shared --use-cuda=no "$DPF" --mathlib=OPENBLAS --openblas-root="$XROOT/usr"
runvx make -j$MAXPAR $CI_TARGETS CI_NOLINKBINARIES=1
# runvx touch base/.depend.mk # Fool make depend into skipping the dependency step.
# runvx touch .short_version # Make version short, or else ccache will miss everything.
# runvx "$CCC" CXXFLAGS="$CF" LDFLAGS="$LDF" ./configure --shared --use-cuda=no "$DPF" --mathlib=OPENBLAS --openblas-root="$XROOT/usr"
# runvx make -j$MAXPAR $CI_TARGETS CI_NOLINKBINARIES=1

runvx cd ../egs/wsj/s5
#runvx find ./ -mindepth 1 -name "*.sh" -exec shellcheck --exclude=SC1090,SC2002,SC2006, --shell=bash --external-sources '{}' ';'
runvx find ./utils ./steps -name "*.sh" -exec shellcheck --exclude=SC1090,SC2002,SC2006 --shell=bash --external-sources '{}' ';'
# for bash_file in $(find ../egs -name "*.sh"); do
# runvx shellcheck -s bas $perl_file
# done


# Travis has a 10k line log limit, so use smaller CI_TARGETS when logging.
if [ -r "$CCACHE_LOGFILE" ]; then
Expand Down

0 comments on commit 656ba01

Please sign in to comment.