Skip to content
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.

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

Shell binary not getting detected correctly? #66

Open
2 tasks done
ecotechie opened this issue Jul 10, 2024 · 6 comments
Open
2 tasks done

Shell binary not getting detected correctly? #66

ecotechie opened this issue Jul 10, 2024 · 6 comments

Comments

@ecotechie
Copy link

ecotechie commented Jul 10, 2024

Bug Report

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
Copy link
Author

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
Copy link
Member

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
Copy link
Author

Forgot to mention I tried that too 😊

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
Copy link
Member

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
Copy link
Member

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
Copy link
Author

This is when it got added but I found nothing regarding why it was "hard coded"
bc13226

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

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

No branches or pull requests

3 participants