Ironholds / rgeolocate

Generalised IP geolocation through R
https://cran.r-project.org/web/packages/rgeolocate/index.html
Other
66 stars 22 forks source link

gcc 7 toolchain (macos anyway) #41

Closed hrbrmstr closed 7 years ago

hrbrmstr commented 7 years ago

it's not building with gcc 7 toolchain but still does with old one. CRAN does not have a binary for 3.4 for macOS and that's likely due to the gcc 7 thing. i'll hit you up at work abt it.

wrathematics commented 7 years ago

Builds for me in gcc 7 on Linux. The CRAN check error is clang related.

I fixed some of the compiler warnings in the latest gcc and clang. I do not consider them real warnings; they were recently added to clang because of an error specific to MS compilers, and gcc has apparently decided to follow suit. As much as I may disagree with their reasoning, it's not a hard fix. However, I am unable to open pull requests at the moment. I get a strange error every time: gh

The error looks like something wrong with github.

gcc 7 has a few additional compiler warnings that actually are serious. The maxmind folks don't check fread() return values. You can add some boilerplate to suppress the warnings, but I don't see a simple path to error handling. I would recommend reporting this to the maintainers (or ignoring it I guess).

oschwald commented 7 years ago

AFAIK, we don't use fread(); it is all mmap based. Do you have a link to the build log?

wrathematics commented 7 years ago

Oh you're right! My mistake. It's in another included library.

hrbrmstr commented 7 years ago

I'll post the macOS build issues in the AM. in the middle of work-work at the moment (thx to WannCry)

Ironholds commented 7 years ago

I believe https://github.com/chrislim2888/IP2Location-C-Library would be the other included lib?

Ironholds commented 7 years ago

This last patch should've fixed it, and I'll pass the changes over to the ip2location people too