Skip to content

Conversation

@devinamatthews
Copy link
Member

Details:

  • Having a space in the source directory, build directory, or install directory leads to major problems, both due to insufficient quoting in several bash scripts, but also due to fundamental limitations in GNU make.
  • Quoting problems in scripts have been fixed.
  • For the Makefile, no true solution is possible. Instead, symlink the source directory (DIST_PATH) as .dist_path and install directories (e.q. INSTALL_LIBDIR as .install_libdir). New helper rules ensure these symlinks as well as the underlying directories exist when needed.

Details:
- Having a space in the source directory, build directory, or install
  directory leads to major problems, both due to insufficient quoting in
  several bash scripts, but also due to fundamental limitations in GNU
  make.
- Quoting problems in scripts have been fixed.
- For the Makefile, no true solution is possible. Instead, symlink the
  source directory (DIST_PATH) as .dist_path and install directories
  (e.q. INSTALL_LIBDIR as .install_libdir). New helper rules ensure
  these symlinks as well as the underlying directories exist when
  needed.

# Continue recursively on the contents of cur_e_dir.
mirror_tree "$(ls $cur_e_dir)"
mirror_tree "$(ls "$cur_e_dir")"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$cur_n_dir needs quoting above. [[ ]] Bash tests are safer than [ ] POSIX test command since they do not split words and don't need double-quotes. So you should use:

if [[ ! -d $cur_n_dir ]]; then
    mkdir "$cur_n_dir"
fi

However,

mkdir -p "$cur_n_dir"

is simpler.

I recommend using shellcheck to test all shell scripts.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You would need to change the shebang to #!/bin/bash instead of #!/bin/sh if you want to use [[ ]] or local.

However, if you remove local and use [ ] or test instead of [[ ]] and double-quote arguments to prevent word splitting, that will also work.


# Continue recursively on the contents of cur_e_dir.
mirror_tree "$(ls $cur_e_dir)"
mirror_tree "$(ls "$cur_e_dir")"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$ shellcheck -o all -e 2250,2312 mirror-tree.sh

In mirror-tree.sh line 39:
        local script_name
        ^---------------^ SC3043 (warning): In POSIX sh, 'local' is undefined.


In mirror-tree.sh line 46:
        echo " "${script_name}
                ^------------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
        echo " ""${script_name}"


In mirror-tree.sh line 72:
                case $opt in
                ^-- SC2249 (info): Consider adding a default *) case, even if it just exits with error.


In mirror-tree.sh line 75:
                             exit 1
                             ^----^ SC2317 (info): Command appears to be unreachable. Check usage (or ignore if invoked indirectly).


In mirror-tree.sh line 78:
        shift $(($OPTIND - 1))
                 ^-----^ SC2004 (style): $/${} is unnecessary on arithmetic variables.


In mirror-tree.sh line 149:
                        if [ ! -d $cur_n_dir ]; then
                                  ^--------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
                        if [ ! -d "$cur_n_dir" ]; then


In mirror-tree.sh line 150:
                                mkdir $cur_n_dir
                                      ^--------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
                                mkdir "$cur_n_dir"

For more information:

https://www.shellcheck.net/wiki/SC3043 -- In POSIX sh, 'local' is undefined.
https://www.shellcheck.net/wiki/SC2086 -- Double quote to prevent globbing ...
https://www.shellcheck.net/wiki/SC2249 -- Consider adding a default *) case...

create_makefile_fragment FRAME "${plugin_dir}/." "obj/${config_name} false"
create_makefile_fragment CONFIG "${config_dirpath}" "obj/${config_name}/config"
create_makefile_fragment KERNELS "${kernels_dirpath}" "obj/${config_name}/kernels"
create_makefile_fragment REFKERN "${refkern_dirpath}" "obj/${config_name}/ref_kernels"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$ shellcheck -o all -e 2248 ./configure

In ./configure line 2451:
                        ${tool} ${opt} > /dev/null 2>&1
                                ^----^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
                        ${tool} "${opt}" > /dev/null 2>&1


In ./configure line 2478:
build_and_check_configurations()
^-- SC2120 (warning): build_and_check_configurations references arguments, but none are ever passed.


In ./configure line 3578:
        build_and_check_configurations
        ^----------------------------^ SC2119 (info): Use build_and_check_configurations "$@" if function's $1 should mean script's $1.


In ./configure line 4925:
        echo "$(grep "^ *$1 *:=" ${sharedir}/blis/config.mk | sed 's/'$1' *:= *//')"
             ^-- SC2005 (style): Useless echo? Instead of 'echo $(cmd)', just use 'cmd'.
                                 ^---------^ SC2086 (info): Double quote to prevent globbing and word splitting.
                                                                      ^-- SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
        echo "$(grep "^ *$1 *:=" "${sharedir}"/blis/config.mk | sed 's/'"$1"' *:= *//')"


In ./configure line 5265:
                plugin_h=$(ls ${plugin_dir}/bli_plugin_*.h 2>/dev/null)
                              ^-----------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
                plugin_h=$(ls "${plugin_dir}"/bli_plugin_*.h 2>/dev/null)


In ./configure line 5266:
                if [ -z ${plugin_h} ]; then
                        ^---------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
                if [ -z "${plugin_h}" ]; then


In ./configure line 5269:
                        plugin_name=$(echo ${plugin_h} | sed -e 's/.*bli_plugin_//' -e 's/\.h//')
                                           ^---------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
                        plugin_name=$(echo "${plugin_h}" | sed -e 's/.*bli_plugin_//' -e 's/\.h//')


In ./configure line 5399:
                        cp ${sharedir}/blis/config_registry config_registry
                           ^---------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
                        cp "${sharedir}"/blis/config_registry config_registry


In ./configure line 5411:
                                cp ${sharedir}/blis/plugin/bli_plugin_register.c bli_plugin_register.c
                                   ^---------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
                                cp "${sharedir}"/blis/plugin/bli_plugin_register.c bli_plugin_register.c


In ./configure line 5413:
                                strip_examples ${sharedir}/blis/plugin/bli_plugin_register.c bli_plugin_register.c
                                               ^---------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
                                strip_examples "${sharedir}"/blis/plugin/bli_plugin_register.c bli_plugin_register.c


In ./configure line 5421:
                if [ -e bli_plugin_${plugin_name}.h ] && [ ${force_flag} == '0' ]; then
                                   ^------------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
                if [ -e bli_plugin_"${plugin_name}".h ] && [ ${force_flag} == '0' ]; then


In ./configure line 5425:
                                generate_config_file ${sharedir}/blis/plugin/bli_plugin.h.in bli_plugin_${plugin_name}.h
                                                     ^---------^ SC2086 (info): Double quote to prevent globbing and word splitting.
                                                                                                        ^------------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
                                generate_config_file "${sharedir}"/blis/plugin/bli_plugin.h.in bli_plugin_"${plugin_name}".h


In ./configure line 5427:
                                generate_config_file ${sharedir}/blis/plugin/bli_plugin.h.in bli_plugin_${plugin_name}_.h
                                                     ^---------^ SC2086 (info): Double quote to prevent globbing and word splitting.
                                                                                                        ^------------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
                                generate_config_file "${sharedir}"/blis/plugin/bli_plugin.h.in bli_plugin_"${plugin_name}"_.h


In ./configure line 5428:
                                strip_examples bli_plugin_${plugin_name}_.h bli_plugin_${plugin_name}.h
                                                          ^------------^ SC2086 (info): Double quote to prevent globbing and word splitting.
                                                                                       ^------------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
                                strip_examples bli_plugin_"${plugin_name}"_.h bli_plugin_"${plugin_name}".h


In ./configure line 5429:
                                rm bli_plugin_${plugin_name}_.h
                                              ^------------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
                                rm bli_plugin_"${plugin_name}"_.h


In ./configure line 5444:
                        if [ ! -e ref_kernels/${file} ] || [ ${force_flag} == '1' ]; then
                                              ^-----^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
                        if [ ! -e ref_kernels/"${file}" ] || [ ${force_flag} == '1' ]; then


In ./configure line 5447:
                                        cp ${sharedir}/blis/plugin/${file} ref_kernels/${file}
                                           ^---------^ SC2086 (info): Double quote to prevent globbing and word splitting.
                                                                   ^-----^ SC2086 (info): Double quote to prevent globbing and word splitting.
                                                                                       ^-----^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
                                        cp "${sharedir}"/blis/plugin/"${file}" ref_kernels/"${file}"


In ./configure line 5449:
                                        strip_examples ${sharedir}/blis/plugin/${file} ref_kernels/${file}
                                                       ^---------^ SC2086 (info): Double quote to prevent globbing and word splitting.
                                                                               ^-----^ SC2086 (info): Double quote to prevent globbing and word splitting.
                                                                                                   ^-----^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
                                        strip_examples "${sharedir}"/blis/plugin/"${file}" ref_kernels/"${file}"


In ./configure line 5451:
                                perl -pi -e "s|\@PLUGIN_HEADER\@|${plugin_h}|;" -e "s|\@plugin_name\@|${plugin_name}|;" ref_kernels/${file}
                                                                                                                                    ^-----^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
                                perl -pi -e "s|\@PLUGIN_HEADER\@|${plugin_h}|;" -e "s|\@plugin_name\@|${plugin_name}|;" ref_kernels/"${file}"


In ./configure line 5469:
                        if [ -e config/${config}/make_defs.mk ] && [ ${force_flag} == '0' ]; then
                                       ^-------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
                        if [ -e config/"${config}"/make_defs.mk ] && [ ${force_flag} == '0' ]; then


In ./configure line 5472:
                                mkdir -p config/${config}
                                                ^-------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
                                mkdir -p config/"${config}"


In ./configure line 5473:
                                cp ${sharedir}/blis/config/${config}/make_defs.mk config/${config}
                                   ^---------^ SC2086 (info): Double quote to prevent globbing and word splitting.
                                                           ^-------^ SC2086 (info): Double quote to prevent globbing and word splitting.
                                                                                         ^-------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
                                cp "${sharedir}"/blis/config/"${config}"/make_defs.mk config/"${config}"


In ./configure line 5479:
                        if [ ! -e config/${config}/bli_plugin_init_${config}.c ] || [ ${force_flag} == '1' ]; then
                                         ^-------^ SC2086 (info): Double quote to prevent globbing and word splitting.
                                                                   ^-------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
                        if [ ! -e config/"${config}"/bli_plugin_init_"${config}".c ] || [ ${force_flag} == '1' ]; then


In ./configure line 5480:
                                if [ ${config} != zen3 ] || [ ${examples_flag} == '0' ]; then
                                     ^-------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
                                if [ "${config}" != zen3 ] || [ ${examples_flag} == '0' ]; then


In ./configure line 5481:
                                        strip_examples ${sharedir}/blis/plugin/bli_plugin_init_zen3.c config/${config}/bli_plugin_init_${config}.c
                                                       ^---------^ SC2086 (info): Double quote to prevent globbing and word splitting.
                                                                                                             ^-------^ SC2086 (info): Double quote to prevent globbing and word splitting.
                                                                                                                                       ^-------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
                                        strip_examples "${sharedir}"/blis/plugin/bli_plugin_init_zen3.c config/"${config}"/bli_plugin_init_"${config}".c


In ./configure line 5482:
                                        perl -pi -e "s/zen3/${config}/g" config/${config}/bli_plugin_init_${config}.c
                                                                                ^-------^ SC2086 (info): Double quote to prevent globbing and word splitting.
                                                                                                          ^-------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
                                        perl -pi -e "s/zen3/${config}/g" config/"${config}"/bli_plugin_init_"${config}".c


In ./configure line 5484:
                                        cp ${sharedir}/blis/plugin/bli_plugin_init_zen3.c config/${config}/bli_plugin_init_${config}.c
                                           ^---------^ SC2086 (info): Double quote to prevent globbing and word splitting.
                                                                                                 ^-------^ SC2086 (info): Double quote to prevent globbing and word splitting.
                                                                                                                           ^-------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
                                        cp "${sharedir}"/blis/plugin/bli_plugin_init_zen3.c config/"${config}"/bli_plugin_init_"${config}".c


In ./configure line 5486:
                                perl -pi -e "s|\@PLUGIN_HEADER\@|${plugin_h}|;" -e "s|\@plugin_name\@|${plugin_name}|;" config/${config}/bli_plugin_init_${config}.c
                                                                                                                               ^-------^ SC2086 (info): Double quote to prevent globbing and word splitting.
                                                                                                                                                         ^-------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
                                perl -pi -e "s|\@PLUGIN_HEADER\@|${plugin_h}|;" -e "s|\@plugin_name\@|${plugin_name}|;" config/"${config}"/bli_plugin_init_"${config}".c


In ./configure line 5489:
                        if [ ! -e config/${config}/bli_kernel_defs_${config}.h ] || [ ${force_flag} == '1' ]; then
                                         ^-------^ SC2086 (info): Double quote to prevent globbing and word splitting.
                                                                   ^-------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
                        if [ ! -e config/"${config}"/bli_kernel_defs_"${config}".h ] || [ ${force_flag} == '1' ]; then


In ./configure line 5490:
                                cp ${sharedir}/blis/config/${config}/bli_kernel_defs_${config}.h config/${config}/bli_kernel_defs_${config}.h
                                   ^---------^ SC2086 (info): Double quote to prevent globbing and word splitting.
                                                           ^-------^ SC2086 (info): Double quote to prevent globbing and word splitting.
                                                                                     ^-------^ SC2086 (info): Double quote to prevent globbing and word splitting.
                                                                                                        ^-------^ SC2086 (info): Double quote to prevent globbing and word splitting.
                                                                                                                                  ^-------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
                                cp "${sharedir}"/blis/config/"${config}"/bli_kernel_defs_"${config}".h config/"${config}"/bli_kernel_defs_"${config}".h


In ./configure line 5501:
                        if [ -e kernels/${kernels} ] && [ ${force_flag} == '0' ]; then
                                        ^--------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
                        if [ -e kernels/"${kernels}" ] && [ ${force_flag} == '0' ]; then


In ./configure line 5504:
                                mkdir -p kernels/${kernels}
                                                 ^--------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
                                mkdir -p kernels/"${kernels}"


In ./configure line 5511:
                                cp ${sharedir}/blis/plugin/my_kernel_1_zen3.c kernels/zen3
                                   ^---------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
                                cp "${sharedir}"/blis/plugin/my_kernel_1_zen3.c kernels/zen3


In ./configure line 5533:
                        cp ${sharedir}/blis/plugin/Makefile Makefile
                           ^---------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
                        cp "${sharedir}"/blis/plugin/Makefile Makefile


In ./configure line 5549:
                build_and_check_configurations
                ^----------------------------^ SC2119 (info): Use build_and_check_configurations "$@" if function's $1 should mean script's $1.


In ./configure line 5551:
                generate_config_file ${sharedir}/blis/plugin/config.mk.in config.mk
                                     ^---------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
                generate_config_file "${sharedir}"/blis/plugin/config.mk.in config.mk

For more information:

https://www.shellcheck.net/wiki/SC2120 -- build_and_check_configurations re...
https://www.shellcheck.net/wiki/SC2086 -- Double quote to prevent globbing ...
https://www.shellcheck.net/wiki/SC2119 -- Use build_and_check_configuration...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can make a run over the files and make additional recommended changes in another branch.

* Suggested fixes for shell quoting fix

* fix .install_incdir
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.

3 participants