Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#4528 closed defect (fixed)

--with-jsondir is broken in 3.0.0

Reported by: jabakobob Owned by: pramsey
Priority: medium Milestone: PostGIS 3.0.0
Component: postgis Version:
Keywords: Cc:

Description

Building with —with-jsondir no longer works in PostGIS 3.0.0beta1. It does work with PostGIS 2.5.3.

Here's the relevant snippet from config.log:

configure:17345: checking for json_object_get in -ljson-c
configure:17370: gcc -o conftest -std=gnu99  -mmacosx-version-min=10.12 -fno-math-errno -fno-signed-zeros   conftest.c -ljson-c   >&5
ld: library not found for -ljson-c

Looking at a diff of configure.ac, it seems that the problem is that the following line is missing in version 3.0.0:

JSON_LDFLAGS="-L$JSONDIR/lib"

Additionally, it seems that the HAVE_LIBJSON_C variable is no longer necessary and should probably be removed.

Change History (7)

comment:1 by algunenano, 5 years ago

Resolution: fixed
Status: newclosed

In 17877:

Fix —with-jsondir ldflags

Fixes #4528

comment:2 by Algunenano, 5 years ago

Thanks for the report. Now it should be working as postgis 2.5

comment:3 by jabakobob, 5 years ago

Still doesn't work. Looks like AC_CHECK_LIB fails because $LIBS does not include -L$JSONDIR/lib

I took inspiration from configure.ac from 2.5.3, and came up with the following (which works on my machine):

--- a/configure.ac
+++ b/configure.ac
@@ -929,19 +929,23 @@ if test "$CHECK_JSON" != "no"; then
             else
                     AC_MSG_RESULT([Using user-specified json-c directory: $JSONDIR])
 
+                    JSON_LDFLAGS="-L$JSONDIR/lib"
+                    LIBS_SAVE="$LIBS"
+                    LIBS="$JSON_LDFLAGS"
                     AC_CHECK_FILE("$JSONDIR/include/json-c/json.h",
                                 [
                                     JSON_CPPFLAGS="-I$JSONDIR/include/json-c"
-                                    AC_CHECK_LIB([json-c], [json_object_get], [HAVE_JSON_C=yes; JSON_LDFLAGS="-L$JSONDIR/lib -ljson-c"])
+                                    AC_CHECK_LIB([json-c], [json_object_get], [HAVE_JSON_C=yes; JSON_LDFLAGS="${JSON_LDFLAGS} -ljson-c"])
                                 ],
                                 [
                                     AC_CHECK_FILE("$JSONDIR/include/json/json.h",
                                     [
                                         JSON_CPPFLAGS="-I$JSONDIR/include/json"
-                                        AC_CHECK_LIB([json], [json_object_get], [HAVE_JSON=yes; JSON_LDFLAGS="-L$JSONDIR/lib -ljson"])
+                                        AC_CHECK_LIB([json], [json_object_get], [HAVE_JSON=yes; JSON_LDFLAGS="${JSON_LDFLAGS} -ljson"])
                                     ],
                                     [AC_MSG_ERROR([Could not find header: json.h])])
                                 ])
+                    LIBS="$LIBS_SAVE"
             fi
     elif test ! -z "$PKG_CONFIG"; then
        PKG_CHECK_MODULES([JSONC], [json-c], [

comment:4 by Algunenano, 5 years ago

You are right, I was doing tests with the default installation (in /usr/lib) so it was working anyway.

Thanks for the patch. I'm pushing it now.

comment:5 by algunenano, 5 years ago

In 17878:

Fix lib detection with "—with-jsondir" flag

Patch by jabakobob
References #4528

comment:6 by jabakobob, 5 years ago

What about the HAVE_LIBJSON_C flag? As far as I understand it's not needed any more.

I would suggest the following patch below to remove it. Am I missing something?

diff --git a/configure.ac b/configure.ac
index 9af7282e4..78ac17ec3 100644
--- a/configure.ac
+++ b/configure.ac
@@ -910,7 +910,6 @@ dnl ===========================================================================
 
 CHECK_JSON=yes
 HAVE_JSON=no
-HAVE_JSON_C=no
 
 AC_ARG_WITH([json],
 	[AS_HELP_STRING([--without-json], [build without json-c support])],
@@ -935,7 +934,7 @@ if test "$CHECK_JSON" != "no"; then
                     AC_CHECK_FILE("$JSONDIR/include/json-c/json.h",
                                 [
                                     JSON_CPPFLAGS="-I$JSONDIR/include/json-c"
-                                    AC_CHECK_LIB([json-c], [json_object_get], [HAVE_JSON_C=yes; JSON_LDFLAGS="${JSON_LDFLAGS} -ljson-c"])
+                                    AC_CHECK_LIB([json-c], [json_object_get], [HAVE_JSON=yes; JSON_LDFLAGS="${JSON_LDFLAGS} -ljson-c"])
                                 ],
                                 [
                                     AC_CHECK_FILE("$JSONDIR/include/json/json.h",
@@ -949,7 +948,7 @@ if test "$CHECK_JSON" != "no"; then
             fi
     elif test ! -z "$PKG_CONFIG"; then
 	PKG_CHECK_MODULES([JSONC], [json-c], [
-                HAVE_JSON_C=yes
+                HAVE_JSON=yes
                 JSON_CPPFLAGS="$JSONC_CFLAGS"
                 JSON_LDFLAGS="$JSONC_LIBS"
             ], [])
@@ -958,16 +957,10 @@ if test "$CHECK_JSON" != "no"; then
     if test "$HAVE_JSON" = "yes"; then
             AC_DEFINE([HAVE_LIBJSON], 1, [Define to 1 if libjson is present])
     fi
-    if test "$HAVE_JSON_C" = "yes"; then
-            AC_DEFINE([HAVE_LIBJSON], 1, [Define to 1 if libjson is present])
-            AC_DEFINE([HAVE_LIBJSON_C], 1, [Define to 1 if libjson resides in a json-c subdir])
-            HAVE_JSON=yes
-    fi
 
     AC_SUBST([JSON_CPPFLAGS])
     AC_SUBST([JSON_LDFLAGS])
     AC_SUBST([HAVE_JSON])
-    AC_SUBST([HAVE_LIBJSON_C])
 
 fi
 
diff --git a/liblwgeom/lwin_geojson.c b/liblwgeom/lwin_geojson.c
index 30a5564e9..415e01134 100644
--- a/liblwgeom/lwin_geojson.c
+++ b/liblwgeom/lwin_geojson.c
@@ -28,7 +28,7 @@
 #include "lwgeom_log.h"
 #include "../postgis_config.h"
 
-#if defined(HAVE_LIBJSON) || defined(HAVE_LIBJSON_C) /* --{ */
+#if defined(HAVE_LIBJSON) /* --{ */
 
 #define JSON_C_VERSION_013 (13 << 8)
 
@@ -403,7 +403,7 @@ parse_geojson(json_object *geojson, int *hasz)
 	return NULL; /* Never reach */
 }
 
-#endif /* HAVE_LIBJSON or HAVE_LIBJSON_C --} */
+#endif /* HAVE_LIBJSON --} */
 
 LWGEOM *
 lwgeom_from_geojson(const char *geojson, char **srs)
diff --git a/postgis/lwgeom_in_geojson.c b/postgis/lwgeom_in_geojson.c
index 9d8bd7750..08b52a7e2 100644
--- a/postgis/lwgeom_in_geojson.c
+++ b/postgis/lwgeom_in_geojson.c
@@ -32,7 +32,7 @@
 #include "liblwgeom.h"
 #include "lwgeom_export.h"
 
-#if defined(HAVE_LIBJSON) || defined(HAVE_LIBJSON_C)
+#if defined(HAVE_LIBJSON)
 
 #include <json.h>
 
diff --git a/postgis_config.h.in b/postgis_config.h.in
index 57afbecc0..abb6f3f02 100644
--- a/postgis_config.h.in
+++ b/postgis_config.h.in
@@ -53,9 +53,6 @@
 /* Define to 1 if libjson is present */
 #undef HAVE_LIBJSON
 
-/* Define to 1 if libjson resides in json-c subdir */
-#undef HAVE_LIBJSON_C
-
 /* Define to 1 if you have the `pq' library (-lpq). */
 #undef HAVE_LIBPQ

comment:7 by algunenano, 5 years ago

In 17891:

Remove HAVE_LIBJSON_C

Patch by jabakobob
References #4528

Note: See TracTickets for help on using tickets.