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

Deprecation warnings running php v8? #258

Closed
moviebrain opened this issue May 4, 2024 · 11 comments
Closed

Deprecation warnings running php v8? #258

moviebrain opened this issue May 4, 2024 · 11 comments

Comments

@moviebrain
Copy link

Would there be any pushback if I attempted a PR to resolve some of the deprecation warnings I see running on php v8? I don't know how quickly php dev moves, but it is my understanding a good number of these will become hard breaks in php v9?

For the latter trim() warning (the very last appears to be the code checking if a mp3 tag exists?) , it looks like a null coalescing assignment operator will work for > php7, but I don't want to try and make a PR that is breaking for anyone, or if it's not really wanted by anyone. I am not really certain what the targeted runtime is, and I don't want to try this and break stuff without knowing any better.

Maybe it'd just be QoL and I'm the only one who gets that squiggle feeling when the terminal pukes out pages of warnings?

PHP Deprecated: Return type of Sandreas\Time\TimeUnit::jsonSerialize() should either be compatible with JsonSerializable::jsonSerialize(): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in phar:///opt/homebrew/bin/m4b-tool/vendor/sandreas/php-time/src/Sandreas/Time/TimeUnit.php on line 241

PHP Deprecated: Return type of M4bTool\Audio\Tag::offsetExists($offset) should either be compatible with ArrayAccess::offsetExists(mixed $offset): bool, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in phar:///opt/homebrew/bin/m4b-tool/src/library/Audio/Tag.php on line 225

PHP Deprecated: Return type of M4bTool\Audio\Tag::offsetGet($offset) should either be compatible with ArrayAccess::offsetGet(mixed $offset): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in phar:///opt/homebrew/bin/m4b-tool/src/library/Audio/Tag.php on line 239

PHP Deprecated: Return type of M4bTool\Audio\Tag::offsetSet($offset, $value) should either be compatible with ArrayAccess::offsetSet(mixed $offset, mixed $value): void, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in phar:///opt/homebrew/bin/m4b-tool/src/library/Audio/Tag.php on line 256

PHP Deprecated: Return type of M4bTool\Audio\Tag::jsonSerialize() should either be compatible with JsonSerializable::jsonSerialize(): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in phar:///opt/homebrew/bin/m4b-tool/src/library/Audio/Tag.php on line 284

PHP Deprecated: trim(): Passing null to parameter #1 ($string) of type string is deprecated in phar:///opt/homebrew/bin/m4b-tool/src/library/Command/AbstractCommand.php on line 482

PHP Deprecated: Implicit conversion from float 5542314.149999999 to int loses precision in phar:///opt/homebrew/bin/m4b-tool/src/library/Parser/SilenceParser.php on line 71

PHP Deprecated: trim(): Passing null to parameter #1 ($string) of type string is deprecated in phar:///opt/homebrew/bin/m4b-tool/src/library/Command/AbstractCommand.php on line 482

PHP Deprecated: trim(): Passing null to parameter #1 ($string) of type string is deprecated in phar:///opt/homebrew/bin/m4b-tool/src/library/Executables/Mp4tags.php on line 95

@JakobTischler
Copy link

I for one would really appreciate fixes for these. I use merge in a chain of commands, which currently can't proceed after the merge because of the error result.

@sandreas
Copy link
Owner

sandreas commented May 5, 2024

Would there be any pushback if I attempted a PR to resolve some of the deprecation warnings I see running on php v8?

No actually this would be great. One minor detail: I'm into this right now (currently analyzing) and I would like to prevent you working on this just to see in a week that there have been updates. I would love to invest more time on this, but since it is started, I will perform my commits. After this you are welcome to do a PR.

I'm totally aware that m4b-tool needs some love again and since there is so much deprecated stuff, this is something that needs to be done as soon as possible.

As a workaround you could either change your php.ini:

error_reporting=E_ALL & ~E_DEPRECATED

or run php with inline ini options:

php -d "error_reporting=E_ALL & ~E_DEPRECATED" m4b-tool.phar #...

I would love to get some feedback on this.

@moviebrain
Copy link
Author

Would there be any pushback if I attempted a PR to resolve some of the deprecation warnings I see running on php v8?

No actually this would be great. One minor detail: I'm into this right now (currently analyzing) and I would like to prevent you working on this just to see in a week that there have been updates. I would love to invest more time on this, but since it is started, I will perform my commits. After this you are welcome to do a PR.

I'm totally aware that m4b-tool needs some love again and since there is so much deprecated stuff, this is something that needs to be done as soon as possible.

As a workaround you could either change your php.ini:

error_reporting=E_ALL & ~E_DEPRECATED

or run php with inline ini options:

php -d "error_reporting=E_ALL & ~E_DEPRECATED" m4b-tool.phar #...

I would love to get some feedback on this.

Excellent. I’ll circle back around to this next weekend when I have some time then, see where it’s at. Thanks for the reply.

sandreas added a commit that referenced this issue May 7, 2024
@sandreas
Copy link
Owner

sandreas commented May 7, 2024

@JakobTischler @moviebrain @tommorris
The marked commit above fixes some of the deprecations, but also alters the error_reporting to no longer contain E_DEPRECATED via:

// disable E_DEPRECATED errors
if(!getenv("M4B_TOOL_FORCE_INI_ERROR_REPORTING")) {
    $errorReportingLevel = error_reporting();
    error_reporting($errorReportingLevel & ~E_DEPRECATED);
}

This basically means it takes the current error_reporting level of the php.ini and removes the E_DEPRECATED flag, as long as you not set the environment variable M4B_TOOL_FORCE_INI_ERROR_REPORTING=1. I hope this will meet your requirements to not have this absolutely useless error messages in your command line and has no side effects (it did not have in my tests).

One deprecation message is caused by a dependency TimeUnit (php-time package), that is also my work. I'll try to update the package asap.

In the next weeks I'll try to make m4b-tool as php8 ready as possible without breaking stuff. If you can't build m4b-tool yourselves, I try to publish a new pre-release in the next days containing these code updates.

@JakobTischler
Copy link

Thank you very much for the help and your work in general.

@moviebrain
Copy link
Author

Thanks @sandreas for all this work. It is greatly appreciated!

@sandreas
Copy link
Owner

@moviebrain @JakobTischler @tommorris
New Pre-Release is out - let me know if it works.

@abhilesh
Copy link

Happy to report that deprecation warnings are gone in the latest Pre-Release version! Thanks a lot!

@sandreas
Copy link
Owner

Happy to report that deprecation warnings are gone in the latest Pre-Release version! Thanks a lot!

Great :-) Thanks for the feedback.

@csandman
Copy link

I have a somewhat related question about this, is it possible to use the alpine package php81 in place of php8? I have a docker project that depends on this project, and I really need to upgrade the version of alpine that my project uses, but the latest version that still includes php8 is v3.16. So I was just curious if 8.1 is a drop in replacement or not before I go down the rabbithold of trying it.

@sandreas
Copy link
Owner

sandreas commented Sep 28, 2024

@csandman
Thanks for pointing out. I think it is time for an upgrade. php 8.1 should work exactly the same. Maybe I'm going to change the base image like this:

  • Switch the base image to php:cli-alpine
  • Remove all manual php installation steps

I did not test it, but the future Dockerfile will be probably look somewhat to this:

FROM sandreas/ffmpeg:5.0.1-3 as ffmpeg
FROM sandreas/tone:v0.1.7 as tone
FROM sandreas/mp4v2:2.1.1 as mp4v2
FROM sandreas/fdkaac:2.0.1 as fdkaac
FROM php:cli-alpine
ENV WORKDIR /mnt/
ENV M4BTOOL_TMP_DIR /tmp/m4b-tool/

RUN echo "---- INSTALL RUNTIME PACKAGES ----" && \
  apk add --no-cache --update --upgrade \
  # mp4v2: required libraries
  libstdc++

COPY --from=ffmpeg /usr/local/bin/ffmpeg /usr/local/bin/
COPY --from=tone /usr/local/bin/tone /usr/local/bin/
COPY --from=mp4v2 /usr/local/bin/mp4* /usr/local/bin/
COPY --from=mp4v2 /usr/local/lib/libmp4v2* /usr/local/lib/
COPY --from=fdkaac /usr/local/bin/fdkaac /usr/local/bin/

ARG M4B_TOOL_DOWNLOAD_LINK="https://github.com/sandreas/m4b-tool/releases/latest/download/m4b-tool.tar.gz"

RUN echo "---- INSTALL M4B-TOOL ----" \
    && if [ ! -f /tmp/m4b-tool.phar ]; then \
            echo "!!! DOWNLOADING ${M4B_TOOL_DOWNLOAD_LINK} !!!" && wget "${M4B_TOOL_DOWNLOAD_LINK}" -O /tmp/m4b-tool.tar.gz && \
            if [ ! -f /tmp/m4b-tool.phar ]; then \
                tar xzf /tmp/m4b-tool.tar.gz -C /tmp/ && rm /tmp/m4b-tool.tar.gz ;\
            fi \
       fi \
    && mv /tmp/m4b-tool.phar /usr/local/bin/m4b-tool \
    && M4B_TOOL_PRE_RELEASE_LINK=$(wget -q -O - https://github.com/sandreas/m4b-tool/releases/tag/latest | grep -o 'M4B_TOOL_DOWNLOAD_LINK=[^ ]*' | head -1 | cut -d '=' -f 2) \
    && echo "!!! DOWNLOADING PRE_RELEASE ${M4B_TOOL_DOWNLOAD_LINK} !!!" && wget "${M4B_TOOL_PRE_RELEASE_LINK}" -O /tmp/m4b-tool.tar.gz \
    && tar xzf /tmp/m4b-tool.tar.gz -C /tmp/ && rm /tmp/m4b-tool.tar.gz \
    && mv /tmp/m4b-tool.phar /usr/local/bin/m4b-tool-pre \
    && chmod +x /usr/local/bin/m4b-tool /usr/local/bin/m4b-tool-pre

WORKDIR ${WORKDIR}
CMD ["list"]
ENTRYPOINT ["m4b-tool-pre"]

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

5 participants