bobthecow / psysh

A REPL for PHP
https://psysh.org
MIT License
9.71k stars 308 forks source link

Strange output inside docker container #778

Closed iwex closed 6 months ago

iwex commented 6 months ago

If I run psysh inside the docker alpine container I have strange output when evaluating commands. For example, if I write $x = 1: CleanShot 2024-03-16 at 14 48 32 Also, it removes my input line with $x = 1

bobthecow commented 6 months ago

Weird. Can you share a dockerfile that'll reproduce this for me?

iwex commented 6 months ago

Sure

FROM php:8.3.2-fpm-alpine3.19

RUN apk --update add \
            linux-headers \
            git \
            unzip \
            zip \
            zlib-dev \
            nano \
            autoconf \
            gcc \
            make \
            g++ \
            shadow \
            openssl \
            freetype \
            libpng \
            libjpeg-turbo \
            freetype-dev \
            libpng-dev \
            libjpeg-turbo-dev \
            libwebp-dev \
            libwebp \
            tzdata \
            libzip-dev \
            icu \
            icu-dev \
            oniguruma-dev \
            libpq-dev \
            gmp-dev \
            gmp \
            wget \
    && docker-php-ext-configure gd \
           --with-freetype \
           --with-jpeg \
           --with-webp \
    && docker-php-ext-install gd pdo_mysql zip pcntl intl mbstring opcache bcmath exif \
    && pecl install apcu redis \
    && docker-php-ext-enable apcu redis \
    && apk del zlib-dev autoconf gcc make g++ make freetype-dev libpng-dev libjpeg-turbo-dev libwebp-dev icu-dev oniguruma-dev libpq-dev gmp-dev \
    && rm -rf /var/cache/apk/*

RUN groupadd -r web \
    && useradd -u 1000 --no-log-init -r -m -g web web \
    && mkdir /app \
    && chown web:web /app

COPY .docker/app/www.conf /usr/local/etc/php-fpm.d/www.conf
COPY .docker/app/custom.ini /usr/local/etc/php/conf.d/

ENV SUPERCRONIC_URL=https://github.com/aptible/supercronic/releases/download/v0.2.29/supercronic-linux-amd64 \
    SUPERCRONIC=supercronic-linux-amd64 \
    SUPERCRONIC_SHA1SUM=cd48d45c4b10f3f0bfdd3a57d054cd05ac96812b

RUN curl -fsSLO "$SUPERCRONIC_URL" \
 && echo "${SUPERCRONIC_SHA1SUM}  ${SUPERCRONIC}" | sha1sum -c - \
 && chmod +x "$SUPERCRONIC" \
 && mv "$SUPERCRONIC" "/usr/local/bin/${SUPERCRONIC}" \
 && ln -s "/usr/local/bin/${SUPERCRONIC}" /usr/local/bin/supercronic

RUN wget -O - https://github.com/jwilder/dockerize/releases/download/v0.7.0/dockerize-linux-amd64-v0.7.0.tar.gz | tar xzf - -C /usr/local/bin

COPY --from=composer:2.7.1 /usr/bin/composer /usr/bin/composer

ENV COMPOSER_CACHE_DIR=/dev/null

USER web

WORKDIR /app

COPY --chown=web:web composer.json /app/composer.json
COPY --chown=web:web composer.lock /app/composer.lock

RUN /usr/bin/composer install --no-dev --no-autoloader --no-plugins --ignore-platform-reqs --no-scripts --no-progress --no-suggest

COPY --chown=web:web . /app

RUN /usr/bin/composer dump-autoload

RUN php artisan icon:cache \
    && php artisan config:cache \
    && php artisan event:cache \
    && php artisan filament:cache-components \
    && php artisan route:cache \
    && php artisan view:cache
iwex commented 6 months ago

@bobthecow I made some tests - this Dockerfile is enough for tests:

FROM php:8.3.2-fpm-alpine3.19

RUN apk --update add \
            linux-headers \
            git \
            unzip \
            zip \
            zlib-dev \
            nano \
            autoconf \
            gcc \
            make \
            g++ \
            shadow \
            openssl \
            freetype \
            libpng \
            libjpeg-turbo \
            freetype-dev \
            libpng-dev \
            libjpeg-turbo-dev \
            libwebp-dev \
            libwebp \
            tzdata \
            libzip-dev \
            icu \
            icu-dev \
            oniguruma-dev \
            libpq-dev \
            gmp-dev \
            gmp \
            wget \
    && docker-php-ext-configure gd \
           --with-freetype \
           --with-jpeg \
           --with-webp \
    && docker-php-ext-install gd pdo_mysql zip pcntl intl mbstring opcache bcmath exif \
    && pecl install apcu redis \
    && docker-php-ext-enable apcu redis \
    && apk del zlib-dev autoconf gcc make g++ make freetype-dev libpng-dev libjpeg-turbo-dev libwebp-dev icu-dev oniguruma-dev libpq-dev gmp-dev \
    && rm -rf /var/cache/apk/*

WORKDIR /app

RUN wget https://psysh.org/psysh
RUN chmod +x psysh
iwex commented 6 months ago

@bobthecow After some tests I've found that docker-php-ext-install pcntl is causing the problem

iwex commented 6 months ago

Also if use usePcntl = false it fixes situation

iwex commented 6 months ago

That format from my screenshot is very similar to less output https://github.com/bobthecow/psysh/issues/721

if I use this config I have this error

return [
  'usePcntl' => false,
  'pager' => 'less',
];

if without pager - everything is ok

return [
  'usePcntl' => false
];

I thing less in apline has some different implementation. here is quick info about less from alpine:

/app # less --help
BusyBox v1.36.1 (2023-11-07 18:53:09 UTC) multi-call binary.

Usage: less [-EFIMmNSRh~] [FILE]...

View FILE (or stdin) one screenful at a time

    -E  Quit once the end of a file is reached
    -F  Quit if entire file fits on first screen
    -I  Ignore case in all searches
    -M,-m   Display status line with line numbers
        and percentage through the file
    -N  Prefix line number to each line
    -S  Truncate long lines
    -R  Remove color escape codes in input
    -~  Suppress ~s displayed past EOF
bobthecow commented 6 months ago

good catch. the problem might be that this flavor of less doesn't honor the X param:

       -X or --no-init
              Disables sending the termcap initialization and deinitialization strings to the terminal.
              This is sometimes desirable if the deinitialization string does something unnecessary,
              like clearing the screen.

… but there's definitely something up with this less.

From my testing, disabling forking (via usePcntl or not adding the pcntl extension) fixes it, but only because it disables the default pager. If I explicitly set less as the pager, the output is bad with or without pcntl. So I think that's a red herring.

iwex commented 6 months ago

@bobthecow yeah, completely agree, but what to do?)

bobthecow commented 6 months ago

… actually that's intended behavior:

     * If no Pager has been explicitly provided, and Pcntl is available, this
     * will default to `cli.pager` ini value, falling back to `which less`.

So if I explicitly set less as the pager it respects that (and breaks), but PsySH will only default to that when pcntl is enabled.

Maybe the right fix here is to try to detect "good" less and only auto-enable it when it's not bad? Or maybe we just disable output paging by default and let people opt into it instead.

iwex commented 6 months ago

As I see - users can manually install correct less via apk add less I think the easiest way is just to add this info to the docs.

... or check if less is symlinked to /bin/busybox

bobthecow commented 6 months ago

The default less check happens once, lazily, the first time it's needed, so i'm not super worried about making it a touch more complicated.

I'll push a change to see if less is a symlink with busybox in the path. Should have minimal false positives, but solve for this (probably not uncommon) case without making people head to docs to figure out what's up.

… and if it gets a false positive, the worst that can happen is that output paging is disabled, so i'm not super worried ¯\_(ツ)_/¯

bobthecow commented 6 months ago

K. I manually confirmed the fix pushed in 9ebb102 as working without disabling pcntl or installing a better less. If you install via composer with @dev you can confirm on your end as well.

If we're happy with the fix, I'll cut a point release and get this shipped :)

iwex commented 6 months ago

I also created a pull request 😅 https://github.com/bobthecow/psysh/pull/779

But okay, will discard it. Thanks!

bobthecow commented 6 months ago

Ha! You're too quick :)