bobthecow/psysh

PsySH doesn't run on PHP 7.4 after recent update to 0.10.11

Philosoft opened this issue ยท 29 comments

Environment

$ lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description:    Ubuntu 20.04.3 LTS
Release:        20.04
Codename:       focal
$ php -v
PHP 7.4.3 (cli) (built: Oct 25 2021 18:20:54) ( NTS )
Copyright (c) The PHP Group
Zend Engine v3.4.0, Copyright (c) Zend Technologies
    with Zend OPcache v7.4.3, Copyright (c), by Zend Technologies
    with Xdebug v2.9.2, Copyright (c) 2002-2020, by Derick Rethans
$ composer show | grep psy
psy/psysh                        v0.10.11 An interactive shell for modern PHP.

The problem

$ ./vendor/bin/psysh ; echo $?
0

Workaround

It is working if I try to use "ready binary" from release page

$ curl -sL https://github.com/bobthecow/psysh/releases/download/v0.10.11/psysh-v0.10.11.tar.gz | tar xzf - && ./psysh
Using local PsySH version at ~/.config/composer
Psy Shell v0.10.11 (PHP 7.4.3 โ€” cli) by Justin Hileman
>>>

Can you try running it with -vvvv verbosity to see if it gives any additional info?

Also, try:

echo "\\psy\\info()" | ./vendor/bin/psysh

Thanks for your fast reply ๐Ÿ™ . I get strangest of results:

$ psysh -vvvv ; echo $?
0
$
$ echo "\\psy\\info()" | ./vendor/bin/psysh
$ 

Mini investigation shows, that vendor/bin/psysh is executed "correctly" without any errors, reaches this part

    if (function_exists('stream_wrapper_register') && stream_wrapper_register('composer-bin-proxy', 'Composer\BinProxyWrapper')) {
        include("composer-bin-proxy://" . $binPath);
        exit(0);
    }

successfully includes file and as successfully exists.

Also directly running the file is working ๐Ÿคฏ

$ php vendor/psy/psysh/bin/psysh
Psy Shell v0.10.11 (PHP 7.4.3 โ€” cli) by Justin Hileman
>>>

Ohhhh that's interesting. Set 'usePcntl' => false in your PsySH config? I bet that'll fix it.

Like this?

$ cat ~/.config/psysh/config.php
<?php

return [
    'usePcntl' => false,
];

no luck

$ cat ~/.config/psysh/config.php ; psysh ; echo "PsySH exit code: $?"
<?php

return [
    'verbosity' => \Psy\Configuration::VERBOSITY_DEBUG
    'usePcntl' => false,
];
PsySH exit code: 0

Reproduction

preparation

$ ls
74.dockerfile  80.dockerfile
$ cat 74.dockerfile
FROM php:7.4.3-cli-alpine
COPY --from=composer:2.1.12 /usr/bin/composer /usr/bin/composer
RUN composer require --prefer-dist psy/psysh
$ cat 80.dockerfile
FROM php:8.0.11-cli-alpine
COPY --from=composer:2.1.12 /usr/bin/composer /usr/bin/composer
RUN composer require --prefer-dist psy/psysh
$ docker build -t test74 . -f 74.dockerfile
# skip build log
$ docker build -t test80 -f 80.dockerfile
# skip build log

Test

PHP 7.4

$ docker run --rm -it test74 /vendor/bin/psysh
$
$ docker run --rm -it test74 /vendor/psy/psysh/bin/psysh
Psy Shell v0.10.11 (PHP 7.4.3 โ€” cli) by Justin Hileman
>>> exit();
Exit:  Goodbye

PHP 8.0

$ docker run --rm -it test80 /vendor/bin/psysh
$
$ docker run --rm -it test80 /vendor/psy/psysh/bin/psysh
Psy Shell v0.10.11 (PHP 8.0.11 โ€” cli) by Justin Hileman
>>> exit();
Exit:  Goodbye

Maybe I am doing something completely wrong, coz versions 0.10.5 and 0.10.9 do not work as expected in docker (or just in local env)

Ok. sorry to bother you. There is some kind of problem on my end. In virtual machine with manjaro and PHP 8.0.12 everything is working fine both natively and in docker. I suggest to close this issue until I find something or more ppl will report similar problems

It may be an issue with a specific version of composer?

maybe diff php -i to see if there's a suspicious setting?

hah. @bobthecow you are right. it IS composer. <= 2.1.9 - works. 2.1.12 - doesn't. Same in docker. Doesn't work with composer 2.1.10 and above (up to current - 2.1.12)

Wanna open an issue with them? :)

Already searching if they have something similar in recent issues))

Did you run composer on the host and execute the php in the container?

Is optimized autoload dumping enabled, and is the host PHP version different to the container's?

Did you run composer on the host and execute the php in the container?

Originally - host. Later for "checking purposes" in docker too.

Is optimized autoload dumping enabled

Doesn't matter - result is the same with and without --optimize-autoload option

and is the host PHP version different to the container's?

Doesn't matter. I checked the same version (7.4.3), latest stable in 7.4 branch (7.4.26) and 8.0.13

I had a look at this and this is an issue that should ideally be fixed in PsySH because the issue-triggering change on Composer's end is only going to get worse with the 2.2 release.

The change we did was that a few releases ago we started building PHP proxies instead of bash proxies for PHP composer-binaries, that allows people to use custom versions of php to call like php7.4 vendor/bin/psysh if they don't want to run with the default PHP. That requires that the bin file is either a symlink (not very cross-platform sadly) or a php file (our current solution). In 2.1 at some point we started using PHP proxies in some envs, but on 2.2 we will always use them as they fix other issues with path repos.

So the problem on the PsySH end is that

psysh/bin/psysh

Lines 122 to 126 in 3801753

if (Psy\Shell::isIncluded($trace)) {
unset($trace);
return;
}
detects it is being included and not run directly and it bails out.

Namely $trace is:

array(1) {
  [0] =>
  array(3) {
    'file' =>
    string(40) "/var/www/test/vendor/bin/psysh"
    'line' =>
    int(98)
    'function' =>
    string(7) "include"
  }
}

I am not sure why you have this code in there, but it'd be great if you can exclude it if (preg_match('{[\\\\/]psysh$}', $trace[0]['file']) at least.

Alternatively from Composer 2.2 on you'll have as a bonus $_composer_autoload_path defined as a global variable by the proxy, so you can detect you're run by a composer proxy, and you can simply include that if it's set instead of the whole 100 lines of autoload-detection you got there :)

See composer/composer#10137 for related changes upcoming in Composer 2.2.

Thanks @Seldaek. That explains why I was unable to reproduce. How do I get the PHP proxies on the current version of Composer for testing?

Unfortunately the whole point of checking the trace and returning is for when psysh is included like this. It lets people import the phar or a globally composer-installed psysh bin as a library, without having to add a local composer installed version. Specifically, this was a fix for #48

I'm open to other ways of detecting when it's running like this, and happy to poke around if you can point me to a way to get PHP proxies for testing :)

You can set this up with composer config bin-compat full then rm the proxy file and run composer install to restore it. You should then get a php proxy.

Your proxy leaks a $binPath global :)

But! If we could make the presence of that global an officially supported thing, and call it something like $_composer_bin_path, I could use that to keep psysh from breaking.

Changing the isIncluded check to this solves the issue, and lets PsySH work both as an include and a binary.

    /**
     * Check whether the first thing in a backtrace is an include call.
     *
     * This is used by the psysh bin to decide whether to start a shell on boot,
     * or to simply autoload the library.
     */
    public static function isIncluded(array $trace)
    {
        $isIncluded = isset($trace[0]['function']) &&
          \in_array($trace[0]['function'], ['require', 'include', 'require_once', 'include_once']);

        // Detect Composer PHP bin proxies.
        if ($isIncluded && \array_key_exists('binPath', $GLOBALS)) {
            // If we're in a bin proxy, we'll *always* see one include, but we
            // care if we see a second immediately after that.
            return isset($trace[1]['function']) &&
                \in_array($trace[1]['function'], ['require', 'include', 'require_once', 'include_once']);
        }

        return $isIncluded;
    }

It is a bit fragile, though, because it depends on the current implementation not something more stable.

As I mentioned above Composer 2.2 will introduce $_composer_autoload_path as global, and that is defined in our public API so I'd definitely prefer if you rely on that. You can try it by using composer self-update --snapshot. We'll hopefully have a release ready soon so it'd be good if PsySH is ready too :) As workaround @Philosoft I'd recommend using Composer's snapshot builds too for now once the fix is in place here.

And $binPath I guess we should get rid of, it adds no value and it's best if we don't leak an additional variable. Edit: composer/composer@8a36c88

$_composer_autoload_path will exist no matter how autoload happens, right? it wouldn't tell us anything specific about being used through a bin proxy?

I just landed a fix for this issue. Confirmed that everything works when running either a local dependency, or globally installed psysh, as either a binary or an include.

$_composer_autoload_path will exist no matter how autoload happens, right? it wouldn't tell us anything specific about being used through a bin proxy?

No that is not the case. The autoloader does not set this variable, only the bin proxy does, so it's really only available to scripts defined in the "bin" key of composer.json and called via the proxy.

Fix looks good to me though ๐Ÿ‘๐Ÿป

@Seldaek awesome. Thanks for your help!

I'm pulling this fix into v0.10.x as well, and will cut a release as soon as that branch is green.

The fix has landed in v0.10.12. Let me know if you have any further issues, @Philosoft!

@bobthecow ๐Ÿ‘ ๐ŸŽ‰ nope. everything works as expected after upgrading to 0.10.12. Thank you very much