Shell binary not getting detected correctly?

[ecotechie]

Bug Report

• [x] Yes, I reviewed the contribution guidelines.
• [x] Yes, more specifically, I reviewed the guidelines on how to write clear bug reports.

The shell binary '/bin/bash' is not valid.

The wp --info command shows the correct Shell, but wp shell does not seem to find it. Since it's not in /bin/bash

[sergio@samara:/var/www/nerdpress]$ wp --info
OS:     Linux 6.6.32 #1-NixOS SMP PREEMPT_DYNAMIC Sat May 25 14:22:56 UTC 2024 x86_64
Shell:  /run/current-system/sw/bin/bash
PHP binary:     /nix/store/swkmyv7mplz46vlr9jyk7qlp2lxv19z0-php-with-extensions-8.2.19/bin/php
PHP version:    8.2.19
php.ini used:   /nix/store/6g69kphwfkdx4p2qn1fgwm888l15cpz7-wp-cli-2.10.0/etc/php.ini
MySQL binary:   /run/current-system/sw/bin/mysql
MySQL version:  mysql  Ver 15.1 Distrib 10.11.6-MariaDB, for Linux (x86_64) using readline 5.1
SQL modes:      STRICT_TRANS_TABLES,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION
WP-CLI root dir:        phar://wp-cli.phar/vendor/wp-cli/wp-cli
WP-CLI vendor dir:      phar://wp-cli.phar/vendor
WP_CLI phar path:       /var/www/nerdpress
WP-CLI packages dir:
WP-CLI cache dir:       $XDG_CACHE_HOME/wp-cli
WP-CLI global config:
WP-CLI project config:
WP-CLI version: 2.10.0

[sergio@samara:/var/www/nerdpress]$ wp shell
Error: The shell binary '/bin/bash' is not valid. You can override the shell to be used through the WP_CLI_CUSTOM_SHELL environment variable.

[sergio@samara:/var/www/nerdpress]$ php -a
Interactive shell

php > var_dump( is_file( '/run/current-system/sw/bin/bash' ) );
bool(true)
php > var_dump( is_readable( '/run/current-system/sw/bin/bash' ) );
bool(true)
php > exit

Describe how other contributors can replicate this bug

• Have get( 'SHELL' ); return something other than /bin/bash
• Run wp shell

Provide a possible solution

Seems that the wp cli info command gets the shell binary using: getenv( 'SHELL' )

Where wp shell uses getenv( 'WP_CLI_CUSTOM_SHELL' ) to check for a "custom shell", then assumes /bin/bash is it isn't set.

Why not use $shell_binary = getenv( 'SHELL' ); instead of $shell_binary = '/bin/bash';

[ecotechie]

Pretty sure this would make it easier to have the shell indifferent places and helps us not assume that it's always in /bin/bash.

I did set define( 'WP_CLI_CUSTOM_SHELL', '/run/current-system/sw/bin/bash' ); in wp-config.php, tough that could be me #DoingItWrong

[swissspidy]

WP_CLI_CUSTOM_SHELL is an environment variable accessed via getenv. It is not a PHP constant. So you would do this instead:

WP_CLI_CUSTOM_SHELL=/run/current-system/sw/bin/bash wp shell

[ecotechie]

Forgot to mention I tried that too :blush:

Any thoughts on my idea regarding changing the way that wp shell checks the shell env? I'd write a pull request, but I am no good at writing tests...

[swissspidy]

Good question. Maybe @schlessera or @danielbachhuber have some context about why the current default is a hardcoded /bin/bash and not getenv( 'SHELL' ). It sounds like a reasonable enhancement at first glance.

[danielbachhuber]

I don't recall a specific reason off the top of my head. For a more definitive answer, I'd look at the original pull request(s) to see if there was a discussion around it.

[ecotechie]

This is when it got added but I found nothing regarding why it was "hard coded" https://github.com/wp-cli/shell-command/commit/bc1322633260db7cf7179f9e1147d4c9ad563658

I'd be happy to test the code, but can't really write tests (on the to learn list).