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

PHP - bcmath extension is required, but not typically installed by default #4465

Closed
dwsupplee opened this issue Apr 2, 2018 · 10 comments
Closed
Assignees

Comments

@dwsupplee
Copy link
Contributor

dwsupplee commented Apr 2, 2018

Certain bundles come with the bcmath extension, but it is not always included by default. It would be great if we either forced a requirement on this extension in the composer.json, or ideally had a fallback for the case where it is not installed.

An error is triggered with calls to Google\Protobuf\Internal\Message::mergeFromJsonString() (due to bccomp calls), and when using the protobuf C extension a segfault is triggered - making tracing the issue extremely difficult.

This was surfaced from a user reported issue at googleapis/google-cloud-php#950.

@TeBoring
Copy link
Contributor

TeBoring commented Apr 2, 2018

I'll add a requirement for bcmath

@TeBoring
Copy link
Contributor

TeBoring commented Apr 3, 2018

Using c extension should not meet this problem.

@TeBoring
Copy link
Contributor

TeBoring commented Apr 3, 2018

I am thinking whether reporting an error will be more reasonable. Some users are using c extension and others may not use mergeFromJsonString. For these uses, bcmath is not needed.

@dwsupplee
Copy link
Contributor Author

@TeBoring that sounds fair and would be better than just a hard to trace segfault.

@dwsupplee
Copy link
Contributor Author

@TeBoring friendly ping :) - is this something still on the radar? This issue can be confusing to our users.

@hackzilla
Copy link

I've just run into this problem with bcmath extension not being installed.

Call to undefined function Google\Protobuf\Internal\bccomp()" {"exception":"[object] (Error(code: 0): Call to undefined function Google\\Protobuf\\Internal\\bccomp() at .../vendor/google/protobuf/src/Google/Protobuf/Internal/Message.php:1000

I can see that it was a suggestion but it feels like the extension is required.

composer info google/protobuf
name     : google/protobuf
descrip. : proto library for PHP
keywords : proto
versions : * v3.17.3
type     : library
license  : BSD 3-Clause "New" or "Revised" License (BSD-3-Clause) (OSI approved) https://spdx.org/licenses/BSD-3-Clause.html#licenseText
homepage : https://developers.google.com/protocol-buffers/
source   : [git] https://github.com/protocolbuffers/protobuf-php.git ae9282cf11dd2933b7e71a611f9590f07d53d3f3
dist     : [zip] https://api.github.com/repos/protocolbuffers/protobuf-php/zipball/ae9282cf11dd2933b7e71a611f9590f07d53d3f3 ae9282cf11dd2933b7e71a611f9590f07d53d3f3
path     : /var/www/ukwm-feeds.ukwm.co.uk/vendor/google/protobuf
names    : google/protobuf

@alicejli
Copy link

I also ran into this issue when testing out the Running Tests section of using the PHP client library generator (which I see referenced in googleapis/google-cloud-php#1812): Specifically, the unit tests failed with the error:

1) Google\Generator\Tests\Unit\ProtoTests\ProtoTest::testGrpcServiceConfig
Error: Call to undefined function Google\Protobuf\Internal\bccomp()

I've updated the documentation there to include a note about this, but curious if there's been any reconsideration of whether we can just include the BCMath extension by default.

@cdcline
Copy link

cdcline commented Apr 24, 2022

This is a pretty annoying issue if you're trying to setup secrets on the google cloud. The docs don't mention any of this.

@fowles fowles assigned haberman and unassigned TeBoring May 6, 2022
@keifgwinn
Copy link

We just ran into this using google secrets in google run

@deannagarcia
Copy link
Member

Documented in #10490

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants