[Buildroot] [PATCH] package/bluez5_utils: Fix builds with musl-libc

[Buildroot] [PATCH] package/bluez5_utils: Fix builds with musl-libc

[Florian Fainelli]

Backport upstream commit ca6546fe521360fcf905bc115b893f322e706cb2
("shared: define MAX_INPUT for musl") to fix builds with musl-libc.

Fixes:
- http://autobuild.buildroot.net/results/3fd24a604a29eff70e20f82690e3f058f04903f9/
- http://autobuild.buildroot.net/results/bd57f1813be699cd5ce2ae94f5f511a71fe10428/
- http://autobuild.buildroot.net/results/90d528cc18a367af2f68e1c0673d962fe399e1ac/
- http://autobuild.buildroot.net/results/552b700ee129e1c7f651de13f6766eae5325dcbc/

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 .../bluez5_utils/0001-musl-max-input.patch    | 35 +++++++++++++++++++
 1 file changed, 35 insertions(+)
 create mode 100644 package/bluez5_utils/0001-musl-max-input.patch

diff --git a/package/bluez5_utils/0001-musl-max-input.patch b/package/bluez5_utils/0001-musl-max-input.patch
new file mode 100644
index 000000000000..87fbb68f8f40
--- /dev/null
+++ b/package/bluez5_utils/0001-musl-max-input.patch
@@ -0,0 +1,35 @@
+From ca6546fe521360fcf905bc115b893f322e706cb2 Mon Sep 17 00:00:00 2001
+From: Sam James <sam@gentoo.org>
+Date: Tue, 31 Jan 2023 05:52:58 +0000
+Subject: shared: define MAX_INPUT for musl
+
+musl systems don't have MAX_INPUT. Just define it to _POSIX_MAX_INPUT
+which musl does have if it's not already defined.
+
+Note that on glibc, the values match anyway (as of glibc-2.36), and
+indeed POSIX_MAX_INPUT has the same value on musl too.
+
+Bug: https://bugs.gentoo.org/888467
+---
+ src/shared/util.c | 5 +++++
+ 1 file changed, 5 insertions(+)
+
+diff --git a/src/shared/util.c b/src/shared/util.c
+index 2f0ae0e86..b466fb001 100644
+--- a/src/shared/util.c
++++ b/src/shared/util.c
+@@ -28,6 +28,11 @@
+ #include <sys/random.h>
+ #endif
+ 
++/* define MAX_INPUT for musl */
++#ifndef MAX_INPUT
++#define MAX_INPUT _POSIX_MAX_INPUT
++#endif
++
+ #include "src/shared/util.h"
+ 
+ void *util_malloc(size_t size)
+-- 
+cgit 
+
-- 
2.34.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH] package/bluez5_utils: Fix builds with musl-libc

[Yann E. MORIN]

Florian, All,

On 2023-03-26 06:09 -0700, Florian Fainelli spake thusly:
> Backport upstream commit ca6546fe521360fcf905bc115b893f322e706cb2
> ("shared: define MAX_INPUT for musl") to fix builds with musl-libc.
> 
> Fixes:
> - http://autobuild.buildroot.net/results/3fd24a604a29eff70e20f82690e3f058f04903f9/
> - http://autobuild.buildroot.net/results/bd57f1813be699cd5ce2ae94f5f511a71fe10428/
> - http://autobuild.buildroot.net/results/90d528cc18a367af2f68e1c0673d962fe399e1ac/
> - http://autobuild.buildroot.net/results/552b700ee129e1c7f651de13f6766eae5325dcbc/
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  .../bluez5_utils/0001-musl-max-input.patch    | 35 +++++++++++++++++++
>  1 file changed, 35 insertions(+)
>  create mode 100644 package/bluez5_utils/0001-musl-max-input.patch
> 
> diff --git a/package/bluez5_utils/0001-musl-max-input.patch b/package/bluez5_utils/0001-musl-max-input.patch
> new file mode 100644
> index 000000000000..87fbb68f8f40
> --- /dev/null
> +++ b/package/bluez5_utils/0001-musl-max-input.patch
> @@ -0,0 +1,35 @@
> +From ca6546fe521360fcf905bc115b893f322e706cb2 Mon Sep 17 00:00:00 2001
> +From: Sam James <sam@gentoo.org>
> +Date: Tue, 31 Jan 2023 05:52:58 +0000
> +Subject: shared: define MAX_INPUT for musl
> +
> +musl systems don't have MAX_INPUT. Just define it to _POSIX_MAX_INPUT
> +which musl does have if it's not already defined.
> +
> +Note that on glibc, the values match anyway (as of glibc-2.36), and
> +indeed POSIX_MAX_INPUT has the same value on musl too.
> +
> +Bug: https://bugs.gentoo.org/888467

Please add your SoB to this patch, and a note that it is a blackport
(yes, the commit log mentions it is a backport, but when we need to
assess this patch in the future, having the notre in the patch itself is
much easier). I usually do that with a note like:

    Original commit log...

    [yann.morin.1998@free.fr: backport from upstream]
    Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>

Note: if the path header has the upstream commit sha1 and applies as-is
(possibly with just a bit of fuzz), then "backport from upstream" is
enough. But if you had to rewrite the ptch, and the header does not have
the upstream sha1, then add it to the note:

    [yann.morin.1998@free.fr: backport and adapt from upsteam 1234CDEF]

The most important missing part here that I can't add is your SoB, so
can you please respin?

Regards,
Yann E. MORIN.

> +---
> + src/shared/util.c | 5 +++++
> + 1 file changed, 5 insertions(+)
> +
> +diff --git a/src/shared/util.c b/src/shared/util.c
> +index 2f0ae0e86..b466fb001 100644
> +--- a/src/shared/util.c
> ++++ b/src/shared/util.c
> +@@ -28,6 +28,11 @@
> + #include <sys/random.h>
> + #endif
> + 
> ++/* define MAX_INPUT for musl */
> ++#ifndef MAX_INPUT
> ++#define MAX_INPUT _POSIX_MAX_INPUT
> ++#endif
> ++
> + #include "src/shared/util.h"
> + 
> + void *util_malloc(size_t size)
> +-- 
> +cgit 
> +
> -- 
> 2.34.1
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] Indicating upstream status [was: [PATCH] package/bluez5_utils: Fix builds with musl-libc]

[Arnout Vandecappelle]

On 28/03/2023 22:25, Yann E. MORIN wrote:
[snip]
> Please add your SoB to this patch, and a note that it is a blackport
> (yes, the commit log mentions it is a backport, but when we need to
> assess this patch in the future, having the notre in the patch itself is
> much easier). I usually do that with a note like:
> 
>      Original commit log...
> 
>      [yann.morin.1998@free.fr: backport from upstream]
>      Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>

  Although I like the []-style, I think we should also converge on a fixed style 
to indicate upstream status in patches, which can be validated by check-package. 
Therefore, I'd like to propose to standardize on:

Upstream: backport [yann.morin.1998@free.fr]

  OTOH we already have a lot of patches with the [] style, so perhaps it's 
better to accept that one in check-package as well. My only concern is that 
there may be some unrelated []-comment in a patch, so it's no sure way of saying 
that upstream status is properly documented.

> Note: if the path header has the upstream commit sha1 and applies as-is
> (possibly with just a bit of fuzz), then "backport from upstream" is
> enough. But if you had to rewrite the ptch, and the header does not have
> the upstream sha1, then add it to the note:
> 
>      [yann.morin.1998@free.fr: backport and adapt from upsteam 1234CDEF]

  So this would be

Upstream: backport from 1234CDEF [yann.morin.1998@free.fr]

  Regards,
  Arnout

> 
> The most important missing part here that I can't add is your SoB, so
> can you please respin?
> 
> Regards,
> Yann E. MORIN.
> 
>> +---
>> + src/shared/util.c | 5 +++++
>> + 1 file changed, 5 insertions(+)
>> +
>> +diff --git a/src/shared/util.c b/src/shared/util.c
>> +index 2f0ae0e86..b466fb001 100644
>> +--- a/src/shared/util.c
>> ++++ b/src/shared/util.c
>> +@@ -28,6 +28,11 @@
>> + #include <sys/random.h>
>> + #endif
>> +
>> ++/* define MAX_INPUT for musl */
>> ++#ifndef MAX_INPUT
>> ++#define MAX_INPUT _POSIX_MAX_INPUT
>> ++#endif
>> ++
>> + #include "src/shared/util.h"
>> +
>> + void *util_malloc(size_t size)
>> +--
>> +cgit
>> +
>> -- 
>> 2.34.1
>>
> 
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] Indicating upstream status [was: [PATCH] package/bluez5_utils: Fix builds with musl-libc]

[Thomas Petazzoni via buildroot]

On Wed, 29 Mar 2023 10:44:38 +0200
Arnout Vandecappelle <arnout@mind.be> wrote:

> > Note: if the path header has the upstream commit sha1 and applies as-is
> > (possibly with just a bit of fuzz), then "backport from upstream" is
> > enough. But if you had to rewrite the ptch, and the header does not have
> > the upstream sha1, then add it to the note:
> > 
> >      [yann.morin.1998@free.fr: backport and adapt from upsteam 1234CDEF]  
> 
>   So this would be
> 
> Upstream: backport from 1234CDEF [yann.morin.1998@free.fr]

If we standardize in a format, then it should handle all common
situations. A patch being a backport from an upstream commit is one
situation. But here are several more situations:

- Patch taken from the upstream project, but not merged yet (from a
  pull request, a mail on a mailing list, etc.)

- Patch taken from a downstream distribution (Debian, Gentoo, Alpine
  Linux, etc.)

- Patch submitted upstream by us, but not merged yet (quite similar to
  case 1 above)

- Patch not submitted upstream for reason XYZ

Best regards,

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] Indicating upstream status [was: [PATCH] package/bluez5_utils: Fix builds with musl-libc]

[Arnout Vandecappelle]

On 29/03/2023 10:50, Thomas Petazzoni wrote:
> On Wed, 29 Mar 2023 10:44:38 +0200
> Arnout Vandecappelle <arnout@mind.be> wrote:
> 
>>> Note: if the path header has the upstream commit sha1 and applies as-is
>>> (possibly with just a bit of fuzz), then "backport from upstream" is
>>> enough. But if you had to rewrite the ptch, and the header does not have
>>> the upstream sha1, then add it to the note:
>>>
>>>       [yann.morin.1998@free.fr: backport and adapt from upsteam 1234CDEF]
>>
>>    So this would be
>>
>> Upstream: backport from 1234CDEF [yann.morin.1998@free.fr]
> 
> If we standardize in a format, then it should handle all common
> situations. A patch being a backport from an upstream commit is one
> situation. But here are several more situations:
> 
> - Patch taken from the upstream project, but not merged yet (from a
>    pull request, a mail on a mailing list, etc.)
> 
> - Patch taken from a downstream distribution (Debian, Gentoo, Alpine
>    Linux, etc.)
> 
> - Patch submitted upstream by us, but not merged yet (quite similar to
>    case 1 above)
> 
> - Patch not submitted upstream for reason XYZ

  Yeah, that's why I propose "Upstream:" - it's generic enough to cover all of 
these cases. The contents can be freeform I think, check-package should just 
check that the Upstream tag is present. And the documentation should give 
examples of the five cases you mention.

  The "patch taken from downstream" is the only one which doesn't fit in the 
"Upstream" thing. But it's also a situation we don't really like: we basically 
don't want local patches, and a patch taken from some other distro _is_ a local 
patch.

  Regards,
  Arnout


> 
> Best regards,
> 
> Thomas
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] Indicating upstream status [was: [PATCH] package/bluez5_utils: Fix builds with musl-libc]

[Thomas Petazzoni via buildroot]

On Wed, 29 Mar 2023 11:06:47 +0200
Arnout Vandecappelle <arnout@mind.be> wrote:

>   Yeah, that's why I propose "Upstream:" - it's generic enough to cover all of 
> these cases. The contents can be freeform I think, check-package should just 
> check that the Upstream tag is present. And the documentation should give 
> examples of the five cases you mention.

Sure.

>   The "patch taken from downstream" is the only one which doesn't fit in the 
> "Upstream" thing. But it's also a situation we don't really like: we basically 
> don't want local patches, and a patch taken from some other distro _is_ a local 
> patch.

The reality is that we have such patches (for example to fix musl build
issues), and we somehow have to live with them.

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] Indicating upstream status [was: [PATCH] package/bluez5_utils: Fix builds with musl-libc]

[Florian Fainelli]

On 3/29/2023 2:28 AM, Thomas Petazzoni wrote:
> On Wed, 29 Mar 2023 11:06:47 +0200
> Arnout Vandecappelle <arnout@mind.be> wrote:
> 
>>    Yeah, that's why I propose "Upstream:" - it's generic enough to cover all of
>> these cases. The contents can be freeform I think, check-package should just
>> check that the Upstream tag is present. And the documentation should give
>> examples of the five cases you mention.
> 
> Sure.

Yocto uses the following format [1]:

Upstream-status: 
(Pending|Submitted|Accepted|Backport|Denied|Inappropriate [reason])

would you want to adopt something similar? OpenWrt to some extent does 
something like that as well although in a less unified fashion, 
sometimes the patches are placed in a specific patch directory 
(backport-x.y.z, upstream-x.y.z) and sometimes that information is 
placed in the patch file itself.

[1]: 
http://wiki.yoctoproject.org/wiki/Best_Known_Methods_%28BKMs%29_for_Package_Updating#Patch_Comments

> 
>>    The "patch taken from downstream" is the only one which doesn't fit in the
>> "Upstream" thing. But it's also a situation we don't really like: we basically
>> don't want local patches, and a patch taken from some other distro _is_ a local
>> patch.
> 
> The reality is that we have such patches (for example to fix musl build
> issues), and we somehow have to live with them.
> 
> Thomas

-- 
Florian
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] Indicating upstream status [was: [PATCH] package/bluez5_utils: Fix builds with musl-libc]

[Arnout Vandecappelle]

On 29/03/2023 14:39, Florian Fainelli wrote:
> 
> 
> On 3/29/2023 2:28 AM, Thomas Petazzoni wrote:
>> On Wed, 29 Mar 2023 11:06:47 +0200
>> Arnout Vandecappelle <arnout@mind.be> wrote:
>>
>>>    Yeah, that's why I propose "Upstream:" - it's generic enough to cover all of
>>> these cases. The contents can be freeform I think, check-package should just
>>> check that the Upstream tag is present. And the documentation should give
>>> examples of the five cases you mention.
>>
>> Sure.
> 
> Yocto uses the following format [1]:
> 
> Upstream-status: (Pending|Submitted|Accepted|Backport|Denied|Inappropriate 
> [reason])

  That's where I got my inspiration :-) I just find "Upstream-status" a bit 
long, so I abbreviated it to "Upstream". Also, "Submitted" and "Accepted" are 
not really useful because the upstream status is going to change while we don't 
change our patch. In addition, a simple URL there is enough information. That's 
how I got to:

For Submitted/Accepted:
Upstream: <PR/mailing list/patchwork URL>

For Backport/Accepted:
Upstream: <commit hash> [submitter@email.address]

For Denied:
Upstream: Denied <PR/mailing list/patchwork URL>

For Inappropriate:
Upstream: N/A + explanation why

  Only "Pending" is a little iffy - we just don't want "Pending" patches I guess.

  Regards,
  Arnout

> would you want to adopt something similar? OpenWrt to some extent does something 
> like that as well although in a less unified fashion, sometimes the patches are 
> placed in a specific patch directory (backport-x.y.z, upstream-x.y.z) and 
> sometimes that information is placed in the patch file itself.
> 
> [1]: 
> http://wiki.yoctoproject.org/wiki/Best_Known_Methods_%28BKMs%29_for_Package_Updating#Patch_Comments
> 
>>
>>>    The "patch taken from downstream" is the only one which doesn't fit in the
>>> "Upstream" thing. But it's also a situation we don't really like: we basically
>>> don't want local patches, and a patch taken from some other distro _is_ a local
>>> patch.
>>
>> The reality is that we have such patches (for example to fix musl build
>> issues), and we somehow have to live with them.
>>
>> Thomas
> 
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] Indicating upstream status [was: [PATCH] package/bluez5_utils: Fix builds with musl-libc]

[Thomas Petazzoni via buildroot]

On Thu, 30 Mar 2023 09:03:50 +0200
Arnout Vandecappelle <arnout@mind.be> wrote:

>   That's where I got my inspiration :-) I just find "Upstream-status" a bit 
> long, so I abbreviated it to "Upstream". Also, "Submitted" and "Accepted" are 
> not really useful because the upstream status is going to change while we don't 
> change our patch. In addition, a simple URL there is enough information. That's 
> how I got to:
> 
> For Submitted/Accepted:
> Upstream: <PR/mailing list/patchwork URL>
> 
> For Backport/Accepted:
> Upstream: <commit hash> [submitter@email.address]
> 
> For Denied:
> Upstream: Denied <PR/mailing list/patchwork URL>
> 
> For Inappropriate:
> Upstream: N/A + explanation why
> 
>   Only "Pending" is a little iffy - we just don't want "Pending" patches I guess.

This sounds good to me.

For the Backport/Accepted, I tend to like when it's the URL to the
commit on Github/Gitlab/Git repo rather than just the hash because then
I can just click on it and see the patch. Also who would be the
[submitter@email.address] in this case? The author of the upstream
patch? Why do we need to replicate that information in Buildroot?

Best regards,

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] Indicating upstream status [was: [PATCH] package/bluez5_utils: Fix builds with musl-libc]

[Arnout Vandecappelle]

On 30/03/2023 14:21, Thomas Petazzoni wrote:
> On Thu, 30 Mar 2023 09:03:50 +0200
> Arnout Vandecappelle <arnout@mind.be> wrote:
> 
>>    That's where I got my inspiration :-) I just find "Upstream-status" a bit
>> long, so I abbreviated it to "Upstream". Also, "Submitted" and "Accepted" are
>> not really useful because the upstream status is going to change while we don't
>> change our patch. In addition, a simple URL there is enough information. That's
>> how I got to:
>>
>> For Submitted/Accepted:
>> Upstream: <PR/mailing list/patchwork URL>
>>
>> For Backport/Accepted:
>> Upstream: <commit hash> [submitter@email.address]
>>
>> For Denied:
>> Upstream: Denied <PR/mailing list/patchwork URL>
>>
>> For Inappropriate:
>> Upstream: N/A + explanation why
>>
>>    Only "Pending" is a little iffy - we just don't want "Pending" patches I guess.
> 
> This sounds good to me.
> 
> For the Backport/Accepted, I tend to like when it's the URL to the
> commit on Github/Gitlab/Git repo rather than just the hash because then
> I can just click on it and see the patch. Also who would be the
> [submitter@email.address] in this case? The author of the upstream
> patch? Why do we need to replicate that information in Buildroot?

  No, the person who did the backport. It's inherited from Yann's original
[yann.morin.1998@free.fr: backport and adapt from upsteam 1234CDEF]

  OTOH we have the SoB a few lines below that says exactly that, and also the 
author of the commit in buildroot that adds the patch, so it's pretty redundant 
information.

  Regards,
  Arnout

> 
> Best regards,
> 
> Thomas
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] Indicating upstream status [was: [PATCH] package/bluez5_utils: Fix builds with musl-libc]

[Thomas Petazzoni via buildroot]

On Thu, 30 Mar 2023 14:33:24 +0200
Arnout Vandecappelle <arnout@mind.be> wrote:

>   No, the person who did the backport. It's inherited from Yann's original
> [yann.morin.1998@free.fr: backport and adapt from upsteam 1234CDEF]
> 
>   OTOH we have the SoB a few lines below that says exactly that, and also the 
> author of the commit in buildroot that adds the patch, so it's pretty redundant 
> information.

Exactly what I was going to say: who did the backport is documented by
the SoB just below.

To me:

Upstream: https://github.com/foo/bar/commit/<hash>
Signed-off-by: John Doe <john.doe@buildroot.org>

is sufficient.

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot