maxmind / libmaxminddb

C library for the MaxMind DB file format
https://maxmind.github.io/libmaxminddb/
Apache License 2.0
912 stars 239 forks source link

Make PACKAGE_VERSION a private macro #308

Closed bsergean closed 1 year ago

bsergean commented 1 year ago

This macro won't be inherited from the command line, and only used internally inside libmaxmindb.

This is causing a warning that gets transformed in an error for us unfortunately as thrift also has an internal PACKAGE_VERSION macro, and both conflict as soon as we include thrift.h.

oschwald commented 1 year ago

The CMake build is failing with:

[  6%] Building C object bin/CMakeFiles/mmdblookup.dir/mmdblookup.c.o
/home/runner/work/libmaxminddb/libmaxminddb/bin/mmdblookup.c: In function ‘get_options’:
/home/runner/work/libmaxminddb/libmaxminddb/bin/mmdblookup.c:2[9](https://github.com/maxmind/libmaxminddb/actions/runs/3291830545/jobs/5426470378#step:4:10)4:59: error: ‘PACKAGE_VERSION’ undeclared (first use in this function)
  294 |         fprintf(stdout, "\n  %s version %s\n\n", program, PACKAGE_VERSION);
      |                                                           ^~~~~~~~~~~~~~~
/home/runner/work/libmaxminddb/libmaxminddb/bin/mmdblookup.c:294:59: note: each undeclared identifier is reported only once for each function it appears in
bsergean commented 1 year ago

Yes I need to try fixing this.

I see that PACKAGE_VERSION is also defined in the header, so including it in mmdblookup.c would fix it, but apparently not.

On Oct 20, 2022, at 11:51 AM, Gregory Oschwald @.***> wrote:

The CMake build is failing with:

[ 6%] Building C object bin/CMakeFiles/mmdblookup.dir/mmdblookup.c.o /home/runner/work/libmaxminddb/libmaxminddb/bin/mmdblookup.c: In function ‘get_options’: /home/runner/work/libmaxminddb/libmaxminddb/bin/mmdblookup.c:294:59: error: ‘PACKAGE_VERSION’ undeclared (first use in this function) 294 | fprintf(stdout, "\n %s version %s\n\n", program, PACKAGE_VERSION); | ^~~~~~~ /home/runner/work/libmaxminddb/libmaxminddb/bin/mmdblookup.c:294:59: note: each undeclared identifier is reported only once for each function it appears in — Reply to this email directly, view it on GitHub https://github.com/maxmind/libmaxminddb/pull/308#issuecomment-1285995055, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC2O6UIF3AQP5KJ3SS6YXJDWEGIBZANCNFSM6AAAAAARKMVPEQ. You are receiving this because you authored the thread.

bsergean commented 1 year ago

@oschwald I think the CMake build does pass now. It's not super elegant but I repeated the definition as a private one for the test program and the unittest.

bsergean commented 1 year ago

@oschwald if you have another idea for the fix please tell me. I searched in the configure/autoconf file for a PACKAGE_VERSION but couldn't find anything similar there.

oschwald commented 1 year ago

I believe PACKAGE_VERSION gets generated by AC_INIT in the Autotools build. The PACKAGE_VERSION in maxmind.h appears to only be set for _WIN32, which may be why it didn't work for you. I think it was used for the MSBuild build before we supported CMake.

In terms of how to fix this, does #310 meet your needs? I think that might be a bit cleaner, although I am no CMake expert.

bsergean commented 1 year ago

Sure that works too, for us what matter is that the macro is kept private to maxmindb.

bsergean commented 1 year ago

Closing in favor of #310