Torann / laravel-geoip

Determine the geographical location of website visitors based on their IP addresses.
http://lyften.com/projects/laravel-geoip
BSD 2-Clause "Simplified" License
2.1k stars 373 forks source link

Remove faulty mkdir call #178

Closed dwightwatson closed 4 years ago

dwightwatson commented 4 years ago

This reverts a change that was made in #172

Quoting my comment from that PR:

This update has broken MaxMindDatabase for me.

[2020-03-09 23:23:40] testing.ERROR: mkdir(): File exists {"exception":"[object] (ErrorException(code: 0): mkdir(): File exists at /Users/Dwight/Sites/my-app/vendor/torann/geoip/src/Services/MaxMindDatabase.php:29) This is with the default configuration. Out of the box the path is storage/app/geoip.mmdb, your call to str_replace turns this into storage/app and then tries to create that directory, which already exists.

This PR should be reverted, and made again if necessary with a proper explanation of the error that is being seen and relevant tests to ensure it isn't breaking existing functionality.

The change means the latest release has started breaking test suites. I don't believe the original PR describes well enough what it was trying to fix, so rather than attempting to resolve that issue I suggest we revert it and ship a patch release.

jessarcher commented 4 years ago

I've just run into this as well. It seems to break clean installs as well as tests - basically any time the geoip.mmdb file doesn't exist, but the parent directory does.

My temporary workaround has been to add another subdirectory to the database path in the configuration so that mkdir has something to create.

Torann commented 4 years ago

Fixed with release 1.2.1