Patchwork New package: cJSON (v2)

login
register
mail settings
Submitter Danomi Manchego
Date June 5, 2012, 12:38 a.m.
Message ID <1338856703-4082-1-git-send-email-danomimanchego123@gmail.com>
Download mbox | patch
Permalink /patch/162886/
State Changes Requested
Headers show

Comments

Danomi Manchego - June 5, 2012, 12:38 a.m.
Signed-off-by: Danomi Manchego <danomimanchego123@gmail.com>
---
 package/Config.in       |    1 +
 package/cjson/Config.in |    7 +++++++
 package/cjson/cjson.mk  |   41 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 49 insertions(+)
 create mode 100644 package/cjson/Config.in
 create mode 100644 package/cjson/cjson.mk
Peter Korsgaard - June 11, 2012, 10:08 a.m.
>>>>> "Danomi" == Danomi Manchego <danomimanchego123@gmail.com> writes:

 Danomi> Signed-off-by: Danomi Manchego <danomimanchego123@gmail.com>
 Danomi> ---
 Danomi>  package/Config.in       |    1 +
 Danomi>  package/cjson/Config.in |    7 +++++++
 Danomi>  package/cjson/cjson.mk  |   41 +++++++++++++++++++++++++++++++++++++++++

 Danomi> +++ b/package/cjson/cjson.mk
 Danomi> @@ -0,0 +1,41 @@
 Danomi> +#############################################################
 Danomi> +#
 Danomi> +# cjson
 Danomi> +#
 Danomi> +#############################################################
 Danomi> +CJSON_VERSION         = undefined
 Danomi> +CJSON_SOURCE          = cJSONFiles.zip
 Danomi> +CJSON_SITE            = http://$(BR2_SOURCEFORGE_MIRROR).dl.sourceforge.net/project/cjson/
 Danomi> +CJSON_INSTALL_STAGING = YES
 Danomi> +
 Danomi> +define CJSON_EXTRACT_CMDS
 Danomi> +	unzip -d $(@D) $(DL_DIR)/$(CJSON_SOURCE)
 Danomi> +endef
 Danomi> +
 Danomi> +define CJSON_BUILD_CMDS
 Danomi> +	cd $(@D)/cJSON && $(TARGET_CC) -Wall -O2 -shared -fpic cJSON.c -o libcJSON.so

I'm not sure it makes sense to build a .so of something as simple (and
unversioned) as this. How about just doing a static .a like ezxml? You
also shouldn't hardcode compiler flags like -Wall / -O2 but use
$(TARGET_CFLAGS) instead - E.G.

cd $(@D)/cJSON && $(TARGET_CC) $(TARGET_CFLAGS) -o cJSON.o cJSON.c
cd $(@D)/cJSON && $(TARGET_AR) rcs libcJSON.a cJSON.o



 Danomi> +endef
 Danomi> +
 Danomi> +define CJSON_INSTALL_STAGING_CMDS
 Danomi> +	$(INSTALL) -D $(@D)/cJSON/cJSON.h $(STAGING_DIR)/usr/include/cJSON.h
 Danomi> +	$(INSTALL) -D $(@D)/cJSON/libcJSON.so $(STAGING_DIR)/usr/lib/libcJSON.so
 Danomi> +endef
 Danomi> +
 Danomi> +define CJSON_INSTALL_TARGET_CMDS
 Danomi> +	$(INSTALL) -D $(@D)/cJSON/libcJSON.so $(TARGET_DIR)/usr/lib/libcJSON.so

You should also install the header file into TARGET_DIR. It will get
removed by target-finalize if it isn't needed.

Care to fix and resend?
Danomi Manchego - June 12, 2012, 12:03 a.m.
Peter,

> I'm not sure it makes sense to build a .so of something as simple (and
> unversioned) as this. How about just doing a static .a like ezxml?

We use cJSON in about half a dozen apps in our project, so a .so seemed
like the way to go.  If you feel strongly about this, would you be open to
having an option to make a shared object?  (Maybe something like the
logic in zlib.mk or qt.mk.)

> You also shouldn't hardcode compiler flags like -Wall / -O2 but use
> $(TARGET_CFLAGS) instead - E.G.

Understood, will adjust.

> You should also install the header file into TARGET_DIR. It will get
> removed by target-finalize if it isn't needed.

Ah, I didn't know that.  Nice!

> Care to fix and resend?

Certainly - after hearing your feedback on the .so/.a question.

Danomi -
Peter Korsgaard - June 12, 2012, 8:08 a.m.
>>>>> "Danomi" == Danomi Manchego <danomimanchego123@gmail.com> writes:

 Danomi> Peter,
 >> I'm not sure it makes sense to build a .so of something as simple (and
 >> unversioned) as this. How about just doing a static .a like ezxml? 

 Danomi> We use cJSON in about half a dozen apps in our project, so a
 Danomi> .so seemed like the way to go.  If you feel strongly about
 Danomi> this, would you be open to having an option to make a shared
 Danomi> object?  (Maybe something like the logic in zlib.mk or qt.mk.)

cJSON is quite small, so the overhead of dynamically loading it is
probably in the same ballpark:

size cJSON.o
   text	   data	    bss	    dec	    hex	filename
   7047	     16	      8	   7071	   1b9f	cJSON.o

But ok, I don't feel strongly about it. If you prefer .so, then keep it
like that.

Patch

diff --git a/package/Config.in b/package/Config.in
index ca8fc96..153b3df 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -462,6 +462,7 @@  source "package/slang/Config.in"
 endmenu
 
 menu "JSON/XML"
+source "package/cjson/Config.in"
 source "package/expat/Config.in"
 source "package/ezxml/Config.in"
 source "package/json-c/Config.in"
diff --git a/package/cjson/Config.in b/package/cjson/Config.in
new file mode 100644
index 0000000..ff90074
--- /dev/null
+++ b/package/cjson/Config.in
@@ -0,0 +1,7 @@ 
+config BR2_PACKAGE_CJSON
+	bool "cJSON"
+	help
+	  An ultra-lightweight, portable, single-file, simple-as-can-be ANSI-C
+	  compliant JSON parser, under MIT license.
+
+	  http://cjson.sourceforge.net/
diff --git a/package/cjson/cjson.mk b/package/cjson/cjson.mk
new file mode 100644
index 0000000..cc1853d
--- /dev/null
+++ b/package/cjson/cjson.mk
@@ -0,0 +1,41 @@ 
+#############################################################
+#
+# cjson
+#
+#############################################################
+CJSON_VERSION         = undefined
+CJSON_SOURCE          = cJSONFiles.zip
+CJSON_SITE            = http://$(BR2_SOURCEFORGE_MIRROR).dl.sourceforge.net/project/cjson/
+CJSON_INSTALL_STAGING = YES
+
+define CJSON_EXTRACT_CMDS
+	unzip -d $(@D) $(DL_DIR)/$(CJSON_SOURCE)
+endef
+
+define CJSON_BUILD_CMDS
+	cd $(@D)/cJSON && $(TARGET_CC) -Wall -O2 -shared -fpic cJSON.c -o libcJSON.so
+endef
+
+define CJSON_INSTALL_STAGING_CMDS
+	$(INSTALL) -D $(@D)/cJSON/cJSON.h $(STAGING_DIR)/usr/include/cJSON.h
+	$(INSTALL) -D $(@D)/cJSON/libcJSON.so $(STAGING_DIR)/usr/lib/libcJSON.so
+endef
+
+define CJSON_INSTALL_TARGET_CMDS
+	$(INSTALL) -D $(@D)/cJSON/libcJSON.so $(TARGET_DIR)/usr/lib/libcJSON.so
+endef
+
+define CJSON_UNINSTALL_STAGING_CMDS
+	rm -f $(STAGING_DIR)/usr/lib/libcJSON.so
+	rm -f $(STAGING_DIR)/usr/include/cJSON.h
+endef
+
+define CJSON_UNINSTALL_TARGET_CMDS
+	rm -f $(TARGET_DIR)/usr/lib/libcJSON.so
+endef
+
+define CJSON_CLEAN_CMDS
+	cd $(@D)/cJSON && rm -f libcJSON.so
+endef
+
+$(eval $(call GENTARGETS))