Skip to contentSkip to conversation

msgpack: bump to version 2.1.5

PATCH Superseded Bradford Barr 3 participants 5 messages Sep 7, 2017 – Sep 13, 2017 1 message not in the archive

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Bradford Barr

author
#0 lore raw

PATCH 1/1 msgpack: bump to version 2.1.5

From: bradford barr <[email protected]>

Signed-off-by bradford barr <[email protected]>
---
 package/msgpack/Config.in    | 12 ++++++++++++
 package/msgpack/msgpack.hash |  2 +-
 package/msgpack/msgpack.mk   | 16 ++++++++++++----
 3 files changed, 25 insertions(+), 5 deletions(-)
package/msgpack/Config.in +12 -0
index b8e8213..e0626b2 100644
@@ -13,3 +13,15 @@ config BR2_PACKAGE_MSGPACK
 comment "msgpack needs a toolchain w/ C++"
 	depends on !BR2_INSTALL_LIBSTDCPP
 	depends on BR2_TOOLCHAIN_HAS_SYNC_4
+
+if BR2_PACKAGE_MSGPACK
+
+config BR2_PACKAGE_MSGPACK_HEADER_ONLY
+	bool "header only"
+	depends on BR2_INSTALL_LIBSTDCPP
+	help
+	  MessagePack can be installed as a header only library.
+
+	  https://github.com/msgpack/msgpack-c#c-header-only-library
+
+endif
package/msgpack/msgpack.hash +1 -1
index 8cd8cb6..173717d 100644
@@ -1,2 +1,2 @@
 # Locally computed:
-sha256  97a371ef950c89f48e8dba6abeccab07f1887e9ba6dab921de0f985c7d5075fe  msgpack-0.5.4.tar.gz
+sha256  9c87f80fc651b900772deaef0ab154b63160c74d292529b5be6d06d6485d4640	msgpack-2.1.5.tar.gz
package/msgpack/msgpack.mk +12 -4
index 9e23948..0d6217d 100644
@@ -4,9 +4,17 @@
 #
 ################################################################################
 
-MSGPACK_VERSION = 0.5.4
-MSGPACK_SITE = http://downloads.sourceforge.net/project/msgpack/msgpack/cpp
-MSGPACK_LICENSE = Apache-2.0
+MSGPACK_VERSION = 2.1.5
+MSGPACK_SITE = $(call github,msgpack,msgpack-c,cpp-$(MSGPACK_VERSION))
+MSGPACK_LICENSE = BSL-1.0
 MSGPACK_LICENSE_FILES = COPYING
 
-$(eval $(autotools-package))
+MSGPACK_INSTALL_STAGING = YES
+
+ifeq ($(BR2_PACKAGE_MSGPACK_HEADER_ONLY),y)
+MSGPACK_INSTALL_TARGET = NO
+else
+MSGPACK_INSTALL_TARGET = YES
+endif
+
+$(eval $(cmake-package))
Signature · 1 line
2.9.3
#1 lore raw

PATCH 1/1 msgpack: bump to version 2.1.5

Hello,

Quoted 3 lines · Bradford Barr

From: bradford barr <[email protected]>

Signed-off-by bradford barr <[email protected]>

This needs a bigger commit log, because it's doing a lot more than a version bump: the license is changing, and you introduce this "header only" option.

package/msgpack/Config.in config BR2_PACKAGE_MSGPACK · line 19

Quoted 12 lines
package/msgpack/Config.in +4 -0
index b8e8213..e0626b2 100644
@@ -13,3 +13,15 @@ config BR2_PACKAGE_MSGPACK
 comment "msgpack needs a toolchain w/ C++"
 	depends on !BR2_INSTALL_LIBSTDCPP
 	depends on BR2_TOOLCHAIN_HAS_SYNC_4
+
+if BR2_PACKAGE_MSGPACK
+
+config BR2_PACKAGE_MSGPACK_HEADER_ONLY

Could you give a rational for adding this ?

package/msgpack/Config.in config BR2_PACKAGE_MSGPACK · line 21

Quoted 2 lines
package/msgpack/Config.in +2 -0
+	bool "header only"
+	depends on BR2_INSTALL_LIBSTDCPP

Not needed, the main option already depends on this.

package/msgpack/Config.in config BR2_PACKAGE_MSGPACK · line 23

Quoted 2 lines
package/msgpack/Config.in +2 -0
+	help
+	  MessagePack can be installed as a header only library.

And why would you do it ?

package/msgpack/Config.in config BR2_PACKAGE_MSGPACK

Quoted 8 lines
package/msgpack/Config.in +7 -1
-$(eval $(autotools-package))
+MSGPACK_INSTALL_STAGING = YES
+
+ifeq ($(BR2_PACKAGE_MSGPACK_HEADER_ONLY),y)
+MSGPACK_INSTALL_TARGET = NO
+else
+MSGPACK_INSTALL_TARGET = YES
+endif

Passing MSGPACK_INSTALL_TARGET = NO is sufficient, since "YES" is the default value.

Thanks!

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

A message that is not in the archive (sent off-list or not yet received): CAPpUvYg0OiL5tFaf9D3WBWJsxqeV5Pnx=F_Uk0o5TVeFZpcuHg@mail.gmail.com

author
#3 lore raw

PATCH 1/1 msgpack: bump to version 2.1.5

Missed the reply all button. Sorry.

B

Quoted 81 lines · Bradford Barr

Hey there. Thanks for the feedback.

Quoted 11 lines · Thomas Petazzoni

Hello,

Quoted 3 lines · Bradford Barr

From: bradford barr <[email protected]>

Signed-off-by bradford barr <[email protected]>

This needs a bigger commit log, because it's doing a lot more than a version bump: the license is changing, and you introduce this "header only" option.

Quoted 14 lines

package/msgpack/Config.in config BR2_PACKAGE_MSGPACK · line 19

Quoted 12 lines
package/msgpack/Config.in +4 -0
index b8e8213..e0626b2 100644
@@ -13,3 +13,15 @@ config BR2_PACKAGE_MSGPACK
 comment "msgpack needs a toolchain w/ C++"
      depends on !BR2_INSTALL_LIBSTDCPP
      depends on BR2_TOOLCHAIN_HAS_SYNC_4
+
+if BR2_PACKAGE_MSGPACK
+
+config BR2_PACKAGE_MSGPACK_HEADER_ONLY

Could you give a rational for adding this ?

If all applications that used msgpack were written in C++ using the header file library you don't need to install the C libraries. They'll just sit there unused taking up space. I thought a flag that would optionally install the C libraries would be a good way to prevent installing unused libraries. If I'm off-base here let me know. I just thought this would be the right way to do things.

Quoted 5 lines

package/msgpack/Config.in config BR2_PACKAGE_MSGPACK · line 21

Quoted 3 lines
package/msgpack/Config.in +2 -0
+     bool "header only"
+     depends on BR2_INSTALL_LIBSTDCPP

Not needed, the main option already depends on this.

Got it. Will fix.

Quoted 5 lines

package/msgpack/Config.in config BR2_PACKAGE_MSGPACK · line 23

Quoted 3 lines
package/msgpack/Config.in +2 -0
+     help
+       MessagePack can be installed as a header only library.

And why would you do it ?

See above.

Quoted 12 lines

package/msgpack/Config.in config BR2_PACKAGE_MSGPACK

Quoted 9 lines
package/msgpack/Config.in +7 -1
-$(eval $(autotools-package))
+MSGPACK_INSTALL_STAGING = YES
+
+ifeq ($(BR2_PACKAGE_MSGPACK_HEADER_ONLY),y)
+MSGPACK_INSTALL_TARGET = NO
+else
+MSGPACK_INSTALL_TARGET = YES
+endif

Passing MSGPACK_INSTALL_TARGET = NO is sufficient, since "YES" is the default value.

Makes sense, I'll fix that.

Thanks again, I'll roll a new patch after I hear what to do with the header only option.

B

Quoted 8 lines

Thanks!

Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

#4 lore raw

PATCH 1/1 msgpack: bump to version 2.1.5

Quoted 28 lines · Bradford Barr
Quoted 27 lines · Bradford Barr

Hey there. Thanks for the feedback.

Quoted 23 lines · Thomas Petazzoni

Hello,

Quoted 3 lines · Bradford Barr

From: bradford barr <[email protected]>

Signed-off-by bradford barr <[email protected]>

This needs a bigger commit log, because it's doing a lot more than a version bump: the license is changing, and you introduce this "header only" option.

package/msgpack/Config.in config BR2_PACKAGE_MSGPACK · line 19

Quoted 12 lines
package/msgpack/Config.in +4 -0
index b8e8213..e0626b2 100644
@@ -13,3 +13,15 @@ config BR2_PACKAGE_MSGPACK
 comment "msgpack needs a toolchain w/ C++"
      depends on !BR2_INSTALL_LIBSTDCPP
      depends on BR2_TOOLCHAIN_HAS_SYNC_4
+
+if BR2_PACKAGE_MSGPACK
+
+config BR2_PACKAGE_MSGPACK_HEADER_ONLY

Could you give a rational for adding this ?

Quoted 1 line
Quoted 1 line

If all applications that used msgpack were written in C++ using the

Since msgpack is a C++ library, this is always the case, no?

Quoted 5 lines
Quoted 5 lines

header file library you don't need to install the C libraries. They'll just sit there unused taking up space. I thought a flag that would optionally install the C libraries would be a good way to prevent installing unused libraries. If I'm off-base here let me know. I just thought this would be the right way to do things.

Makes sense. It should be a separate patch from the version bump, though, so we
can apply it independently - chances are that this new option would need a few iterations...

A _HEADER_ONLY option has one big problem: it makes it impossible for packages
that need the library to select msgpack. Indeed, they would need to select msgpack and not header_only, which is not possible. And adding a depends on header_only gives a circular dependency.

So the right approach would be to have an option INSTALL_LIB. You should then
check the existing packages that can use msgpack (only python-msgpack I think) whether or not they need the library installed.

Regards,
Arnout

Signature · 6 lines
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
author
#5 lore raw

PATCH 1/1 msgpack: bump to version 2.1.5

Sounds good. I'll resubmit the version bump as a single patch, and the INSTALL_LIB option as a second patch.

Thanks for your help.
B

Quoted 71 lines · Arnout Vandecappelle
Quoted 28 lines · Bradford Barr
Quoted 27 lines · Bradford Barr

Hey there. Thanks for the feedback.

Quoted 23 lines · Thomas Petazzoni

Hello,

Quoted 3 lines · Bradford Barr

From: bradford barr <[email protected]>

Signed-off-by bradford barr <[email protected]>

This needs a bigger commit log, because it's doing a lot more than a version bump: the license is changing, and you introduce this "header only" option.

package/msgpack/Config.in config BR2_PACKAGE_MSGPACK · line 19

Quoted 12 lines
package/msgpack/Config.in +4 -0
index b8e8213..e0626b2 100644
@@ -13,3 +13,15 @@ config BR2_PACKAGE_MSGPACK
 comment "msgpack needs a toolchain w/ C++"
      depends on !BR2_INSTALL_LIBSTDCPP
      depends on BR2_TOOLCHAIN_HAS_SYNC_4
+
+if BR2_PACKAGE_MSGPACK
+
+config BR2_PACKAGE_MSGPACK_HEADER_ONLY

Could you give a rational for adding this ?

Quoted 1 line
Quoted 1 line

If all applications that used msgpack were written in C++ using the

Since msgpack is a C++ library, this is always the case, no?

Quoted 5 lines
Quoted 5 lines

header file library you don't need to install the C libraries. They'll just sit there unused taking up space. I thought a flag that would optionally install the C libraries would be a good way to prevent installing unused libraries. If I'm off-base here let me know. I just thought this would be the right way to do things.

Makes sense. It should be a separate patch from the version bump, though, so we
can apply it independently - chances are that this new option would need a few iterations...

A _HEADER_ONLY option has one big problem: it makes it impossible for packages
that need the library to select msgpack. Indeed, they would need to select msgpack and not header_only, which is not possible. And adding a depends on header_only gives a circular dependency.

So the right approach would be to have an option INSTALL_LIB. You should then
check the existing packages that can use msgpack (only python-msgpack I think) whether or not they need the library installed.

Regards,
Arnout

--

Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot