1

I am creating a configure file for an R package. So far, the R/C++ parts of the code are ok, but when I run devtools::check() to check for my code integrity/quality, I get a note (i.e., a soft warning) about my configure file that says possible bashism in configure line XX ('command' with option other than -p)

This is the part of configure that R doesn't like:

# Extra checks on MacOS for SSL support in libpq
# command -v is probably fine: https://stackoverflow.com/a/677212/946850
if [ `uname` = "Darwin" ] && [ `command -v pkg-config` ]; then
  if pkg-config --atleast-version=12 libpq; then
    case "`pkg-config --libs --static libpq`" in
    *crypto*)
      echo "Local libpq has SSL support"
      ;;
    *)
      echo "Local libpq does not have SSL support"
      FORCE_AUTOBREW=1
      ;;
    esac
  else
    FORCE_AUTOBREW=1
  fi
fi

Should I ignore that or should I write that part in a general way? If the right way is the 2nd way, which would be a good practice to write these kind of configurations?

Sorry if this is a noob question. I am a statistician trying to learn something that is not what I readily available in R/SPSS.

pachadotdev
  • 3,345
  • 6
  • 33
  • 60
  • 4
    Add a POSIX shell shebang, then use shellcheck as your current script contains errors and deprecated syntax. ShellCheck will tell you exactly what and link you to relevant documentation to help you fix it. – Léa Gris Feb 08 '23 at 20:01
  • 1
    I don't see what *I* think of as bashisms there, but good idea to let a machine debug it for you. Let us know what you find. Good luck. – shellter Feb 08 '23 at 20:03
  • Thanks a lot, at least now know what to find, here is one relevant answer https://stackoverflow.com/a/53116875/3720258. Another relevant search shows that a POSIX compatible solution is -v, which I added but the check doesn't like it https://stackoverflow.com/a/677212/3720258 – pachadotdev Feb 08 '23 at 20:04
  • 1
    `command -v` is not a bashism. – Charles Duffy Feb 08 '23 at 20:06
  • 1
    See the relevant POSIX spec at https://pubs.opengroup.org/onlinepubs/9699919799/utilities/command.html#tag_20_22 – Charles Duffy Feb 08 '23 at 20:06
  • 1
    now, where there _is_ an obvious bug is ```[ `uname` = "Darwin" ]```; that should be ```[ "`uname`" = Darwin ]``` -- quotes are needed around the command substitution, they _aren't_ needed around the constant string that contains no potential syntax. – Charles Duffy Feb 08 '23 at 20:07
  • Thanks! Bash is zero obvious to me, I write some scripts and if those work, I consider those to be good, but I am not an artist of bash. – pachadotdev Feb 08 '23 at 20:10
  • 2
    ...I wonder if the concern is compatibility with pre-POSIX shells. `command -v` is guaranteed in any POSIX-y shell (which is to say, any shell compliant with early-90s specifications), but configure scripts often try to be compatible with 1970s-era Bourne (aka "Heirloom sh"). – Charles Duffy Feb 08 '23 at 20:10
  • ```if [ "`uname`" = Darwin ] && [ `command -v pkg-config` ]; then``` also says "possible bashism" – pachadotdev Feb 08 '23 at 20:14
  • 1
    ...checking, `command -v` is definitely there in the 1994 issue of POSIX.2; I haven't found a copy of the earlier versions of the spec to see if it's there in the original 1992 release. – Charles Duffy Feb 08 '23 at 20:14
  • 2
    BTW, you should probably use quotes in ```[ "`command -v pkg-config`" ]``` for the same reason. It's _unlikely_ that the difference will matter, but if your PATH contains an entry with a directory with a space in it, or your IFS is set to `/`, bad things would happen without those quotes. Won't help with the warning, though -- afaik that's either a bug or excessive stringency (checking for compatibility with shells that were obsolete before a lot of folks here were born). – Charles Duffy Feb 08 '23 at 20:17
  • 2
    (BTW, using backticks for command substitution is _itself_ obsolete; `$(...)` is another feature that's been standardized since POSIX.2 was first published). – Charles Duffy Feb 08 '23 at 20:24
  • 4
    Or, just use `command -v pkg-config > /dev/null` instead of capture its output to test with `[`. – chepner Feb 08 '23 at 20:27

2 Answers2

3

The check is either wrong, or checking for compatibility with pre-POSIX Bourne shell; the POSIX.2 standard requires command -v to work and has since at least 1994 if not the initial publication in 1992; there's no genuine bashism here.

Charles Duffy
  • 280,126
  • 43
  • 390
  • 441
2

what works with devtools::check() and returns 0 notes is

if [ "`uname`" = Darwin ] && [ "`command -v pkg-config`" ]; then

instead of

if [ `uname` = "Darwin" ] && [ `command -v pkg-config` ]; then

this solution is an "average" of all the comments in this thread

pachadotdev
  • 3,345
  • 6
  • 33
  • 60
  • 3
    This is definitely more reliable code, but it's surprising that it suppresses a warning about bashisms; bash has the same bugs when executing the inadequately-quoted version as-currently-written. (It also has an _extended_ test operator, `[[`, which doesn't require the quotes for correct operation, but using that actually _would_ be a bashism; `[` is for this immediate purposes at feature parity either way) – Charles Duffy Feb 08 '23 at 22:25