diff mbox series

[1/1] package/zynaddsubfx: fix uclibc compile error

Message ID 20230909104707.491203-1-ju.o@free.fr
State Superseded
Headers show
Series [1/1] package/zynaddsubfx: fix uclibc compile error | expand

Commit Message

Julien Olivain Sept. 9, 2023, 10:47 a.m. UTC 
When compiling with uclibc, build fails with errors, such as:

    In file included from /build/zynaddsubfx-3.0.6/src/Nio/NulEngine.h:21,
                     from /build/zynaddsubfx-3.0.6/src/Nio/NulEngine.cpp:14:
    /build/zynaddsubfx-3.0.6/src/Nio/MidiIn.h:37:9: error: 'uint8_t' does not name a type
       37 |         uint8_t midiSysEx(unsigned char data);
          |         ^~~~~~~

This commit fixes the issue by adding upstream patches, not yet
included in a release.

Fixes:
http://autobuild.buildroot.net/results/97b5a30c7be820ac91e745cf60f9b759e962aa5c

Signed-off-by: Julien Olivain <ju.o@free.fr>
---
 ...1-MidiIn-Add-stdint.h-header-include.patch | 26 +++++++++++++++++
 ...002-Bank-Add-stdint.h-header-include.patch | 28 +++++++++++++++++++
 2 files changed, 54 insertions(+)
 create mode 100644 package/zynaddsubfx/0001-MidiIn-Add-stdint.h-header-include.patch
 create mode 100644 package/zynaddsubfx/0002-Bank-Add-stdint.h-header-include.patch

Comments

Thomas Petazzoni Sept. 9, 2023, 3:08 p.m. UTC | #1 
Hello Julien,

On Sat,  9 Sep 2023 12:47:07 +0200
Julien Olivain <ju.o@free.fr> wrote:

> When compiling with uclibc, build fails with errors, such as:
> 
>     In file included from /build/zynaddsubfx-3.0.6/src/Nio/NulEngine.h:21,
>                      from /build/zynaddsubfx-3.0.6/src/Nio/NulEngine.cpp:14:
>     /build/zynaddsubfx-3.0.6/src/Nio/MidiIn.h:37:9: error: 'uint8_t' does not name a type
>        37 |         uint8_t midiSysEx(unsigned char data);
>           |         ^~~~~~~
> 
> This commit fixes the issue by adding upstream patches, not yet
> included in a release.
> 
> Fixes:
> http://autobuild.buildroot.net/results/97b5a30c7be820ac91e745cf60f9b759e962aa5c
> 
> Signed-off-by: Julien Olivain <ju.o@free.fr>

Since when is this build issue occurring? zynaddsubfx has been in
Buildroot for a while, and I don't see any recent update of it. Have
this issue always existed? Is it due to a recent update of uClibc?

Thomas
Julien Olivain Sept. 9, 2023, 9:07 p.m. UTC | #2 
Hi Thomas,

On 09/09/2023 17:08, Thomas Petazzoni wrote:
> Hello Julien,
> 
> On Sat,  9 Sep 2023 12:47:07 +0200
> Julien Olivain <ju.o@free.fr> wrote:
> 
>> When compiling with uclibc, build fails with errors, such as:
>> 
>>     In file included from 
>> /build/zynaddsubfx-3.0.6/src/Nio/NulEngine.h:21,
>>                      from 
>> /build/zynaddsubfx-3.0.6/src/Nio/NulEngine.cpp:14:
>>     /build/zynaddsubfx-3.0.6/src/Nio/MidiIn.h:37:9: error: 'uint8_t' 
>> does not name a type
>>        37 |         uint8_t midiSysEx(unsigned char data);
>>           |         ^~~~~~~
>> 
>> This commit fixes the issue by adding upstream patches, not yet
>> included in a release.
>> 
>> Fixes:
>> http://autobuild.buildroot.net/results/97b5a30c7be820ac91e745cf60f9b759e962aa5c
>> 
>> Signed-off-by: Julien Olivain <ju.o@free.fr>
> 
> Since when is this build issue occurring? zynaddsubfx has been in
> Buildroot for a while, and I don't see any recent update of it. Have
> this issue always existed? Is it due to a recent update of uClibc?

This build failure is the first of its kind, according to autobuild:
http://autobuild.buildroot.net/?reason=zynaddsubfx-3.0.6

Looking at the upstream code, this header inclusion is missing, so the
bug has always been there. My guess is that this "stdint.h" header was
included indirectly from the libc internally. I did not investigated
further. Do you want me to?

Best regards,

Julien.
Thomas Petazzoni Sept. 10, 2023, noon UTC | #3 
Hello,

On Sat, 09 Sep 2023 23:07:19 +0200
Julien Olivain <ju.o@free.fr> wrote:

> This build failure is the first of its kind, according to autobuild:
> http://autobuild.buildroot.net/?reason=zynaddsubfx-3.0.6
> 
> Looking at the upstream code, this header inclusion is missing, so the
> bug has always been there. My guess is that this "stdint.h" header was
> included indirectly from the libc internally. I did not investigated
> further. Do you want me to?

It seems like it is related to gcc 13.x, and that the right thing would
be to include <cstdint>. See commit
27dc49378071a82688d44a5cc592c0200b3991b4 for an example of something
similar. So you could try to build (without your patch) using gcc 12.x
and gcc 13.x and see if that's the difference that causes the build
failure.

Thanks!

Thomas
Julien Olivain Sept. 10, 2023, 1:10 p.m. UTC | #4 
Hi Thomas,

On 10/09/2023 14:00, Thomas Petazzoni wrote:
> Hello,
> 
> On Sat, 09 Sep 2023 23:07:19 +0200
> Julien Olivain <ju.o@free.fr> wrote:
> 
>> This build failure is the first of its kind, according to autobuild:
>> http://autobuild.buildroot.net/?reason=zynaddsubfx-3.0.6
>> 
>> Looking at the upstream code, this header inclusion is missing, so the
>> bug has always been there. My guess is that this "stdint.h" header was
>> included indirectly from the libc internally. I did not investigated
>> further. Do you want me to?
> 
> It seems like it is related to gcc 13.x, and that the right thing would
> be to include <cstdint>. See commit
> 27dc49378071a82688d44a5cc592c0200b3991b4 for an example of something
> similar. So you could try to build (without your patch) using gcc 12.x
> and gcc 13.x and see if that's the difference that causes the build
> failure.

You are right, it _is_ this GCC 13 change. It is unrelated to uclibc nor
or1k. I reproduced the build failure from a simplified autobuild 
defconfig
from [1]. Just switching back to gcc 12 works. I also tested few other
configurations (glibc, native compilation, etc.).

It is described at:
https://gcc.gnu.org/gcc-13/porting_to.html#header-dep-changes

I fixed many of those issues last year, but I apparently forgot ;)

I agree with you, the right thing to do would be to include <cstdint>,
but it seems upstream is struggling a bit to fix those in a consistent
way. See [2] and [3] for example. I'll try to propose improvements to
upstream.

In the meantime, this patch needs a v2. Do you want me to:

1. just change the commit log, while keeping the current upstream 
patches
    with <stdint.h>, or

2. redo also the upstream patches to have <cstdint> consistently
    everywhere in the code?

Best regards,

Julien.

[1] 
http://autobuild.buildroot.net/results/97b5a30c7be820ac91e745cf60f9b759e962aa5c/defconfig
[2] 
https://github.com/zynaddsubfx/zynaddsubfx/commit/70905c96fe7b9ffde19bc4bc05b0dc53a1ed1707
[3] 
https://github.com/zynaddsubfx/zynaddsubfx/commit/806674849b276a560d2996f25ba0ddbfe50838ef
Thomas Petazzoni Sept. 10, 2023, 1:48 p.m. UTC | #5 
On Sun, 10 Sep 2023 15:10:28 +0200
Julien Olivain <ju.o@free.fr> wrote:

> In the meantime, this patch needs a v2. Do you want me to:
> 
> 1. just change the commit log, while keeping the current upstream 
> patches
>     with <stdint.h>, or
> 
> 2. redo also the upstream patches to have <cstdint> consistently
>     everywhere in the code?

Keep the upstream patches as-is, so option (1).

Thanks for the additional research!

Thomas
diff mbox series

Patch

diff --git a/package/zynaddsubfx/0001-MidiIn-Add-stdint.h-header-include.patch b/package/zynaddsubfx/0001-MidiIn-Add-stdint.h-header-include.patch
new file mode 100644
index 0000000000..ea4eb66190
--- /dev/null
+++ b/package/zynaddsubfx/0001-MidiIn-Add-stdint.h-header-include.patch
@@ -0,0 +1,26 @@ 
+From 4f1565f18cf71867f25f31c50c3a5e6995f49ae6 Mon Sep 17 00:00:00 2001
+From: fundamental <mark.d.mccurry@gmail.com>
+Date: Sat, 22 Apr 2023 10:21:09 -0400
+Subject: [PATCH] MidiIn: Add stdint.h header include
+
+Upstream: https://github.com/zynaddsubfx/zynaddsubfx/commit/70905c96fe7b9ffde19bc4bc05b0dc53a1ed1707
+Signed-off-by: Julien Olivain <ju.o@free.fr>
+---
+ src/Nio/MidiIn.h | 1 +
+ 1 file changed, 1 insertion(+)
+
+diff --git a/src/Nio/MidiIn.h b/src/Nio/MidiIn.h
+index ce0bcfec..c6b30c61 100644
+--- a/src/Nio/MidiIn.h
++++ b/src/Nio/MidiIn.h
+@@ -17,6 +17,7 @@
+ #define MIDI_IN_H
+ 
+ #include "Engine.h"
++#include <stdint.h>//uint8_t
+ 
+ namespace zyn {
+ 
+-- 
+2.41.0
+
diff --git a/package/zynaddsubfx/0002-Bank-Add-stdint.h-header-include.patch b/package/zynaddsubfx/0002-Bank-Add-stdint.h-header-include.patch
new file mode 100644
index 0000000000..556854a9e2
--- /dev/null
+++ b/package/zynaddsubfx/0002-Bank-Add-stdint.h-header-include.patch
@@ -0,0 +1,28 @@ 
+From eab3bc1712e5af7e6aef942a24e833ce2c429436 Mon Sep 17 00:00:00 2001
+From: fundamental <mark.d.mccurry@gmail.com>
+Date: Wed, 19 Apr 2023 21:52:01 -0400
+Subject: [PATCH] Bank: Add stdint.h header include
+
+As uint8_t is used, this header should be included.
+
+Upstream: https://github.com/zynaddsubfx/zynaddsubfx/commit/f384d92486d6b515cb628d0f52008a9e03341d8c
+Signed-off-by: Julien Olivain <ju.o@free.fr>
+---
+ src/Misc/Bank.h | 1 +
+ 1 file changed, 1 insertion(+)
+
+diff --git a/src/Misc/Bank.h b/src/Misc/Bank.h
+index 5120441a..3f324dd8 100644
+--- a/src/Misc/Bank.h
++++ b/src/Misc/Bank.h
+@@ -18,6 +18,7 @@
+ #include <vector>
+ #include "../globals.h"
+ #include "Config.h"
++#include <stdint.h>
+ 
+ //entries in a bank
+ #define BANK_SIZE 160
+-- 
+2.41.0
+