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

Does not play well with PHP Scoper #194

Closed
andrewjmead opened this issue Aug 3, 2022 · 5 comments · Fixed by #195
Closed

Does not play well with PHP Scoper #194

andrewjmead opened this issue Aug 3, 2022 · 5 comments · Fixed by #195

Comments

@andrewjmead
Copy link

I'm using this library in a WordPress plugin that uses PHP Scoper to scope all dependencies.

I noticed that this library does not support PHP Scoper out of the box due to its use of string class names as seen here, here, and here.

I was able to patch this in PHP Scoper using the following patcher, but it would be awesome to see built-in support. Keep in mind this just patches the two instances in Reader.php and doesn't attempt to patch the instance in Client.php:

    'patchers' => [
        static function (string $filePath, string $prefix, string $contents): string {
            $parts = explode('vendor', $filePath);

            // Fix an issue with geoip2 where the reader has string class names
            if ($parts[1] === '/geoip2/geoip2/src/Database/Reader.php') {
                return preg_replace(
                    '%\$class = \'GeoIp2%',
                    '$class = \'' . $prefix . '\\\\GeoIp2',
                    $contents
                );
            }

            return $contents;
        },
    ],

I was encourage to open a separate issue by @oschwald in #170. Thanks!

😁

@oschwald
Copy link
Member

oschwald commented Aug 4, 2022

Would you be willing to test #195 to confirm that it resolves the issue for you?

@andrewjmead
Copy link
Author

Yup. I'll test it tomorrow 👍

@andrewjmead
Copy link
Author

I just installed the branch using composer require geoip2/geoip2:dev-greg/no-stringy-class-names and it fixed the issue right away!

@oschwald
Copy link
Member

oschwald commented Aug 5, 2022

Great! Thanks for testing!

@andrewjmead
Copy link
Author

Thanks to you for addressing it!

bearer-pipeline-test pushed a commit to BearerPipelineTest/GeoIP2-php that referenced this issue Aug 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants