[1/1] msgpack: bump to version 2.1.5

From: bradford barr <bradford@density.io>

Signed-off-by: bradford barr <bradford@density.io>
---
 package/msgpack/Config.in    | 12 ++++++++++++
 package/msgpack/msgpack.hash |  2 +-
 package/msgpack/msgpack.mk   | 16 ++++++++++++----
 3 files changed, 25 insertions(+), 5 deletions(-)

Hello,

On Thu,  7 Sep 2017 11:51:08 -0400, Bradford Barr wrote:
> From: bradford barr <bradford@density.io>
> 
> Signed-off-by: bradford barr <bradford@density.io>

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.

> diff --git a/package/msgpack/Config.in b/package/msgpack/Config.in
> index b8e8213..e0626b2 100644
> --- a/package/msgpack/Config.in
> +++ b/package/msgpack/Config.in
> @@ -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 ?

> +	bool "header only"
> +	depends on BR2_INSTALL_LIBSTDCPP

Not needed, the main option already depends on this.

> +	help
> +	  MessagePack can be installed as a header only library.

And why would you do it ?

> -$(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

Missed the reply all button. Sorry.

B

On Tue, Sep 12, 2017 at 10:16 AM, Bradford Barr <bradford@density.io> wrote:
> Hey there. Thanks for the feedback.
>
> On Sat, Sep 9, 2017 at 5:09 PM, Thomas Petazzoni
> <thomas.petazzoni@free-electrons.com> wrote:
>> Hello,
>>
>> On Thu,  7 Sep 2017 11:51:08 -0400, Bradford Barr wrote:
>>> From: bradford barr <bradford@density.io>
>>>
>>> Signed-off-by: bradford barr <bradford@density.io>
>>
>> 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.
>>
>>> diff --git a/package/msgpack/Config.in b/package/msgpack/Config.in
>>> index b8e8213..e0626b2 100644
>>> --- a/package/msgpack/Config.in
>>> +++ b/package/msgpack/Config.in
>>> @@ -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.
>
>>
>>> +     bool "header only"
>>> +     depends on BR2_INSTALL_LIBSTDCPP
>>
>> Not needed, the main option already depends on this.
>
> Got it. Will fix.
>
>>
>>> +     help
>>> +       MessagePack can be installed as a header only library.
>>
>> And why would you do it ?
>
> See above.
>
>>
>>> -$(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
>
>>
>> Thanks!
>>
>> Thomas
>> --
>> Thomas Petazzoni, CTO, Free Electrons
>> Embedded Linux and Kernel engineering
>> http://free-electrons.com

On 12-09-17 16:18, Bradford Barr wrote:
> On Tue, Sep 12, 2017 at 10:16 AM, Bradford Barr <bradford@density.io> wrote:
>> Hey there. Thanks for the feedback.
>>
>> On Sat, Sep 9, 2017 at 5:09 PM, Thomas Petazzoni
>> <thomas.petazzoni@free-electrons.com> wrote:
>>> Hello,
>>>
>>> On Thu,  7 Sep 2017 11:51:08 -0400, Bradford Barr wrote:
>>>> From: bradford barr <bradford@density.io>
>>>>
>>>> Signed-off-by: bradford barr <bradford@density.io>
>>> 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.
>>>
>>>> diff --git a/package/msgpack/Config.in b/package/msgpack/Config.in
>>>> index b8e8213..e0626b2 100644
>>>> --- a/package/msgpack/Config.in
>>>> +++ b/package/msgpack/Config.in
>>>> @@ -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

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

>> 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

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

On Wed, Sep 13, 2017 at 3:58 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
>
>
> On 12-09-17 16:18, Bradford Barr wrote:
>> On Tue, Sep 12, 2017 at 10:16 AM, Bradford Barr <bradford@density.io> wrote:
>>> Hey there. Thanks for the feedback.
>>>
>>> On Sat, Sep 9, 2017 at 5:09 PM, Thomas Petazzoni
>>> <thomas.petazzoni@free-electrons.com> wrote:
>>>> Hello,
>>>>
>>>> On Thu,  7 Sep 2017 11:51:08 -0400, Bradford Barr wrote:
>>>>> From: bradford barr <bradford@density.io>
>>>>>
>>>>> Signed-off-by: bradford barr <bradford@density.io>
>>>> 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.
>>>>
>>>>> diff --git a/package/msgpack/Config.in b/package/msgpack/Config.in
>>>>> index b8e8213..e0626b2 100644
>>>>> --- a/package/msgpack/Config.in
>>>>> +++ b/package/msgpack/Config.in
>>>>> @@ -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
>
>  Since msgpack is a C++ library, this is always the case, no?
>
>>> 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@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot