Skip to content

Change here-string bashism to POSIX echo #14

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: amd-fftw
Choose a base branch
from
Open

Change here-string bashism to POSIX echo #14

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions configure.ac
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -653,7 +653,7 @@ if test "$have_amd_opt" = yes && test "${enable_debug+set}" != "set" && test "$I
AC_DEFINE(AMD_DYNAMIC_DISPATCHER,1,[Define to enable AMD dynamic dispatcher feature that would build a single portable optimized library which can execute on different x86 CPU architectures.])

SUBSTRGCC='gcc'
if grep -q "$SUBSTRGCC" <<<"$CC"; then
if echo "$CC" | grep -q "$SUBSTRGCC"; then
CFLAGS="$CFLAGS -mno-avx256-split-unaligned-store -mno-avx256-split-unaligned-load -mno-prefer-avx128"
else
AC_MSG_CHECKING([whether we are using clang 14 or later])
Expand Down Expand Up @@ -683,13 +683,13 @@ if test "$have_amd_opt" = yes && test "${enable_debug+set}" != "set" && test "$I
fi
SUBSTRCLANG='clang'
SUBSTRGCC='gcc'
if grep -q "$SUBSTRCLANG" <<<"$CC"; then
if echo "$CC" | grep -q "$SUBSTRCLANG"; then
if [[ -z "${AMD_ARCH}" ]]; then
Copy link

@e-kwsm e-kwsm Sep 2, 2024

Choose a reason for hiding this comment

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

Aren’t [[/]] bashism as well?

Copy link
Author

@jeroen-mostert jeroen-mostert Sep 3, 2024

Choose a reason for hiding this comment

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

Yes, you're quite right. To be fair I spent exactly as much effort as needed to get things to run on a distro that used dash for sh and nothing more, and the clang branch was never hit. If I have copious free time I can check the whole file for bashisms (shellcheck does not pick up on this, for some reason).

Copy link

@e-kwsm e-kwsm Sep 4, 2024

Choose a reason for hiding this comment

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

Hopefully these bashism stuffs can be found in Ubuntu GitHub Actions once implemented.

CFLAGS="$CFLAGS -mavx2 -mfma"
else
CFLAGS="$CFLAGS -march=$AMD_ARCH -mavx2 -mfma"
fi
elif grep -q "$SUBSTRGCC" <<<"$CC"; then
elif echo "$CC" | grep -q "$SUBSTRGCC"; then
GCCVERSION=$(expr `gcc -dumpversion | cut -f1 -d.`)
case "$AMDZENFAMILY" in
"23")
Expand Down