Skip to content

Conversation

whitslack
Copy link
Contributor

Configure scripts are supposed to adhere to the POSIX shell language. The POSIX test builtin does not implement an == operator. Bash does, but not all systems have Bash installed as /bin/sh. In particular, many systems use the lighter-weight Dash as the default POSIX shell. Dash emits the following error when running configure:

./configure: 39065: test: xno: unexpected operator

This PR removes the Bashism and restores correct operation with POSIX-compliant shells like Dash.

@hebasto
Copy link
Member

hebasto commented Nov 19, 2021

Concept ACK.

It seems the proper way to achieve configure script portability is to use AS_IF macro. But I'm fine with the suggested solution as well.

@maflcko
Copy link
Member

maflcko commented Nov 20, 2021

Is there a way to turn the warning into an error? We use dash in the CI, but no one is going to read the logs to find those errors.

See https://cirrus-ci.com/task/5279449093505024?logs=ci#L1704

Options used to compile and link:
  external signer = yes
  multiprocess    = no
  with experimental syscall sandbox support = no
  with libs       = yes
  with wallet     = yes
    with sqlite   = yes
    with bdb      = yes
  with gui / qt   = yes
    with qr       = yes
  with zmq        = yes
./configure: 35955: test: xno: unexpected operator
  with test       = not building test_bitcoin because fuzzing is enabled
    with fuzz     = no
...

@whitslack
Copy link
Contributor Author

Is there a way to turn the warning into an error?

On my Gentoo system, because this particular mistake is so common, I have a post-configure check on all builds:

if fgrep -q -e ': unexpected operator' -e 'configure: line' "${T}/build.log" ; then
        eerror 'Package exhibits symptoms of assuming /bin/sh is Bash!'
        eerror 'Maybe setting CONFIG_SHELL=/bin/bash will work around it.'
        die
fi

It's a brittle check, but it's helped me catch and fix at least a dozen Bashisms in various packages.

@katesalazar
Copy link
Contributor

ACK cf72925.

@laanwj
Copy link
Member

laanwj commented Nov 22, 2021

Code review ACK cf72925

Obvious agree that it would be good if these kind of errors were actually fatal errors.

@maflcko maflcko merged commit ee7e061 into bitcoin:master Nov 22, 2021
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Nov 22, 2021
cf72925 configure.ac: remove Bashism (Matt Whitlock)

Pull request description:

  Configure scripts are supposed to adhere to the POSIX shell language. The POSIX `test` builtin does not implement an `==` operator. Bash does, but not all systems have Bash installed as `/bin/sh`. In particular, many systems use the lighter-weight Dash as the default POSIX shell. Dash emits the following error when running `configure`:

  ```
  ./configure: 39065: test: xno: unexpected operator
  ```

  This PR removes the Bashism and restores correct operation with POSIX-compliant shells like Dash.

ACKs for top commit:
  katesalazar:
    ACK cf72925.
  laanwj:
    Code review ACK cf72925

Tree-SHA512: 578c873fba52e0472baed9e024bddcf58a0e088600bd5854f3011f1f8d135773ad923bb16baefc960d17ecedee9cc980b36aaa70fb32eb9bc7de93f7fe60541d
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Nov 23, 2021
cf72925 configure.ac: remove Bashism (Matt Whitlock)

Pull request description:

  Configure scripts are supposed to adhere to the POSIX shell language. The POSIX `test` builtin does not implement an `==` operator. Bash does, but not all systems have Bash installed as `/bin/sh`. In particular, many systems use the lighter-weight Dash as the default POSIX shell. Dash emits the following error when running `configure`:

  ```
  ./configure: 39065: test: xno: unexpected operator
  ```

  This PR removes the Bashism and restores correct operation with POSIX-compliant shells like Dash.

ACKs for top commit:
  katesalazar:
    ACK cf72925.
  laanwj:
    Code review ACK cf72925

Tree-SHA512: 578c873fba52e0472baed9e024bddcf58a0e088600bd5854f3011f1f8d135773ad923bb16baefc960d17ecedee9cc980b36aaa70fb32eb9bc7de93f7fe60541d
@whitslack whitslack deleted the fix-configure-bashism branch January 14, 2022 08:38
@bitcoin bitcoin locked and limited conversation to collaborators Jan 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants