build failure when unmodified uthash.h is installed systemwide

[landry]

The OpenBSD package of postgis is at 3.0.0, it builds and runs fine, except when the uthash header is installed systemwide (we have it in packages), it gets picked up before the bundled deps/uthash/include/uthash.h header, thus the 'hack' done to avoid conflicts with postgresql #define HASH_FUNCTION in postgresql/server/utils/hsearch.h doesnt apply.

For some reason, the order of headers search has postgresql headers and systemwide headers looked last:

cc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -Wno-unused-command-line-argument -O2 -pipe -fPI
C -DPIC -I../liblwgeom -std=gnu99 -I/usr/local/include -fno-math-errno -fno-signed-zeros -I../libpgcommon -I../deps/wagyu -I../deps/uthash/include   -I/usr/local/include -I/usr/local/include -I/usr/local/include -I/usr/local/include/libxml2 -I/usr/local/include  -I/usr/local/include/json-c -I/usr/local/include   -fPIC -DPIC -I. -I./ -I/usr/local/include/postgresql/server -I/usr/local/include/postgresql/internal  -I/usr/local/include -I/usr/local/include/libxml2 -I/usr/local/include   -c -o mvt.o mvt.c                                                                                                           
mvt.c:308:2: error: called object type 'int' is not a function or function pointer                                                                                                                                   
        HASH_FIND(hh, ctx->keys_hash, name, size, kv);                                                                                                                                                               
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~                                                                                                                                                                
/usr/local/include/uthash.h:179:3: note: expanded from macro 'HASH_FIND'                                                                                                                                             
  HASH_VALUE(keyptr, keylen, _hf_hashv);                                         \                                                                                                                                   
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~                                                                                                                                                                              
/usr/local/include/uthash.h:161:11: note: expanded from macro 'HASH_VALUE'                                
  HASH_FCN(keyptr, keylen, hashv);                                               \
  ~~~~~~~~^
mvt.c:321:2: error: called object type 'int' is not a function or function pointer
        HASH_ADD_KEYPTR(hh, ctx->keys_hash, name, size, kv);
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/local/include/uthash.h:428:3: note: expanded from macro 'HASH_ADD_KEYPTR'
  HASH_VALUE(keyptr, keylen_in, _ha_hashv);                                      \
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/local/include/uthash.h:161:11: note: expanded from macro 'HASH_VALUE'
  HASH_FCN(keyptr, keylen, hashv);                                               \
  ~~~~~~~~^

so yes at that point HASH_FUNCTION is indeed an int.

[landry]

After a bit of fiddling, moving $(UTHASH_INCLUDE) right after -I../liblwgeom in the PG_CPPFLAGS line of postgis/Makefile.in solves the build issue when uthash is installed globally. Would this be considered acceptable ?

-PG_CPPFLAGS += -I../liblwgeom @CFLAGS@ -I../libpgcommon $(WAYGU_INCLUDE) $(UTHASH_INCLUDE) @CPPFLAGS@ @PICFLAGS@
+PG_CPPFLAGS += -I../liblwgeom $(UTHASH_INCLUDE) @CFLAGS@ -I../libpgcommon $(WAYGU_INCLUDE) @CPPFLAGS@ @PICFLAGS@

[robe]

I'll try to test this this week and assuming no one has issues with this I'll go ahead and commit.

[strk]

The patch looks fine to me. User-defined flags are good to be listed before externally-defined ones.

[Algunenano]

I don't see anything wrong with the change, but there wouldn't be any issue if you weren't including a PATH inside the CFLAGS and used CPPFLAGS for this instead, right? Are you adding -I/usr/local/include to the CFLAGS in your call to configure or is something broken in our side?

[landry]

Replying to Algunenano:

I don't see anything wrong with the change, but there wouldn't be any issue if you weren't including a PATH inside the CFLAGS and used CPPFLAGS for this instead, right? Are you adding -I/usr/local/include to the CFLAGS in your call to configure or is something broken in our side?

Yes, for iconv.h to be found - and it's also added anyway by pg-config.. but yeah i'll check what happens if i use CPPFLAGS instead of CFLAGS.

[Algunenano]

I'm guessing adding the include to CPPFLAGS fixed this. Feel free to reopen if not.

[landry]

Replying to Algunenano:

I'm guessing adding the include to CPPFLAGS fixed this. Feel free to reopen if not.

sorry for the very late reply, but yes i've just tested with 3.3.0 and using CPPFLAGS instead of CFLAGS in the env allows me to build without modifying PG_CPPFLAGS.