Confirm less is link before using it as such

[piurafunk]

The PHP docs of readlink() indicate that if the path provided is not a link, it fails.

Note: The function fails if path is not a symlink, except on Windows, where the normalized path will be returned.

I am seeing this in Docker with PHP FPM image, with Alpine 3.20.3, PHP 8.2.23:

65aefc8b7840:/var/www/html$ php artisan tinker

   ErrorException 

  readlink(): Invalid argument

  at vendor/psy/psysh/src/Configuration.php:1266
    1262▕                 // n.b. The busybox less implementation is a bit broken, so
    1263▕                 // let's not use it by default.
    1264▕                 //
    1265▕                 // See https://github.com/bobthecow/psysh/issues/778
  ➜ 1266▕                 $link = @\readlink($less);
    1267▕                 if ($link !== false && \strpos($link, 'busybox') !== false) {
    1268▕                     return false;
    1269▕                 }
    1270▕ 

      +16 vendor frames 
  17  artisan:31
      Illuminate\Foundation\Console\Kernel::handle(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))

[piurafunk]

Just discovered that the reason less is not a symlink in my environment is because we install it specifically in our Dockerfile:

RUN apk add --no-cache \
    ...
    less \
    ...
    ;

which then clobbers the symlink:

65aefc8b7840:/var/www/html$ ls -lah /usr/bin/less
-rwxr-xr-x    1 root     root      167.9K Apr 17 02:57 /usr/bin/less

Would still love this to be merged, though; it'll help stabilize in different environments.

[bobthecow]

that @ is meant to suppress the error. something is overeagerly changing them into ErrorExceptions :)

[piurafunk]

Yes, I imagine that is Laravel. In combination with PHP 8, the @ symbol reports it's error level, see docs. Which causes Laravel to throw a new ErrorException.

It seems like defensive programming to first check if it is a link before treating it as such. Do you agree with that?

[bobthecow]

yeah, it's reasonable to check first.

if we're going to do that, we should wrap this whole thing in a conditional block, rather than using a ternary to fall back to the actual less path. this line of code is trying to say "if less is a symlink, and it includes busybox in the actual path, then…"

[piurafunk]

I've updated that in 0c59c2d.

[bobthecow]

Thanks!

[piurafunk]

@bobthecow How soon/frequently do you do releases? I'd love to upgrade to get this change in our app.

[bobthecow]

There's no set cadence, but it's probably about time. I'll get this released in the next few days.