[PATCH 0/1] gnu: ragel: Fix build of knot on aarch64-linux.

Done

Details

3 participants

Ludovic Courtès
Tobias Geerinckx-Rice
Simon South

Owner: unassigned

Submitted by: Simon South

Severity: normal

Simon South wrote on 16 Oct 2021 19:04

Recipients:(address . guix-patches@gnu.org)

Message-ID:cover.1634402642.git.simon@simonsouth.net

This change fixes the build of Knot on AArch64 by patching Ragel to be

explicit in its use of signed "char" types and ranges rather than assuming

"char" is signed by default on all platforms, as it is not on aarch64-linux.

Presently Knot's test suite is failing on aarch64-linux due to the

src/libknot/ypbody.c file being improperly recreated in the package's

"update-parser" phase. From Knot's runtests.log:

ok 139 - set input string

# wanted: 0

# seen: -999

not ok 140 - parse key with a value in UTF-8

not ok 141 - compare UTF-8 value

1..141

# Looks like you failed 2 tests of 141

With the patch applied, ypbody.c is generated as expected (including now the

explicit use of "signed char" where intended) and the tests pass.

I've tested these changes on AArch64 and x86-64. On the latter I've

sucessfully re-built the output of "guix refresh --list-dependent ragel" with

the exception of ccextractor, which fails (even without this patch) for an

unrelated reason. (On AArch64 this rebuild would be an unreasonably large

task, though the immediate dependents all build fine.)

Simon South

simon@simonsouth.net

Simon South (1):

gnu: ragel: Fix build of knot on aarch64-linux.

gnu/local.mk | 1 +

.../ragel-specify-char-signedness.patch | 58 +++++++++++++++++++

gnu/packages/ragel.scm | 4 +-

3 files changed, 62 insertions(+), 1 deletion(-)

create mode 100644 gnu/packages/patches/ragel-specify-char-signedness.patch

base-commit: 34b0aa16e77bdbb5b847267eb0f825a590e3d101

2.33.0

Simon South wrote on 16 Oct 2021 19:06

[PATCH 1/1] gnu: ragel: Fix build of knot on aarch64-linux.

Recipients:(address . 51241@debbugs.gnu.org)

Message-ID:8d3c7556a41b12a899c52181a5f6689111f036b4.1634402642.git.simon@simonsouth.net

Make explicit Ragel's assumption of the "char" type being signed on both its
build and target platforms, allowing the current version of Knot and perhaps
other dependent packages to build successfully on aarch64-linux where "char"
is unsigned by default.

* gnu/packages/patches/ragel-specify-char-signedness.patch: New file.
* gnu/local.mk (dist_patch_DATA): Add it.
* gnu/packages/ragel.scm (ragel)[source]: Apply it.
---
 gnu/local.mk                                  |  1 +
 .../ragel-specify-char-signedness.patch       | 58 +++++++++++++++++++
 gnu/packages/ragel.scm                        |  4 +-
 3 files changed, 62 insertions(+), 1 deletion(-)
 create mode 100644 gnu/packages/patches/ragel-specify-char-signedness.patch

Toggle diff (93 lines)diff --git a/gnu/local.mk b/gnu/local.mk
index d1803e7f59..a4dd01a40f 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -1716,6 +1716,7 @@ dist_patch_DATA =						\
   %D%/packages/patches/quassel-qt-514-compat.patch		\
   %D%/packages/patches/quickswitch-fix-dmenu-check.patch	\
   %D%/packages/patches/qtwebkit-pbutils-include.patch		\
+  %D%/packages/patches/ragel-specify-char-signedness.patch	\
   %D%/packages/patches/randomjungle-disable-static-build.patch	\
   %D%/packages/patches/rapicorn-isnan.patch			\
   %D%/packages/patches/rapidjson-gcc-compat.patch		\
diff --git a/gnu/packages/patches/ragel-specify-char-signedness.patch b/gnu/packages/patches/ragel-specify-char-signedness.patch
new file mode 100644
index 0000000000..fccb79d432
--- /dev/null
+++ b/gnu/packages/patches/ragel-specify-char-signedness.patch
@@ -0,0 +1,58 @@
+Ragel assumes C and C++'s "char" type is signed by default but this is not
+true for every architecture gcc supports.  Prevent build failures of dependent
+packages (on architectures such as AArch64 where "char" is normally unsigned)
+by making explicit Ragel's assumption of character signedness.
+
+This is functionally very similar to upstream commit e2650a7, "common: Fix
+ambiguous CHAR_MIN/MAX definition", which is also meant to address this issue.
+However this patch applies to Ragel version 6 and it removes altogether the
+ambiguous "char" type from generated C code, causing Ragel to instead specify
+"signed char" or "unsigned char" as appropriate.
+
+diff --git a/ragel/common.cpp b/ragel/common.cpp
+index 8e9f8ed0..649f388c 100644
+--- a/ragel/common.cpp
++++ b/ragel/common.cpp
+@@ -27,7 +27,7 @@
+ 
+ HostType hostTypesC[] =
+ {
+-	{ "char",     0,       "char",    true,   true,  false,  CHAR_MIN,  CHAR_MAX,   0, 0,              sizeof(char) },
++	{ "signed",   "char",  "char",    true,   true,  false,  SCHAR_MIN, SCHAR_MAX,  0, 0,              sizeof(signed char) },
+ 	{ "unsigned", "char",  "uchar",   false,  true,  false,  0, 0,                  0,     UCHAR_MAX,  sizeof(unsigned char) },
+ 	{ "short",    0,       "short",   true,   true,  false,  SHRT_MIN,  SHRT_MAX,   0, 0,              sizeof(short) },
+ 	{ "unsigned", "short", "ushort",  false,  true,  false,  0, 0,                  0,     USHRT_MAX,  sizeof(unsigned short) },
+@@ -66,7 +66,7 @@ HostType hostTypesC[] =
+ 
+ HostType hostTypesD[] =
+ {
+-	{ "byte",    0,  "byte",    true,   true,  false,  CHAR_MIN,  CHAR_MAX,    0, 0,                   1 },
++	{ "byte",    0,  "byte",    true,   true,  false,  SCHAR_MIN, SCHAR_MAX,   0, 0,                   1 },
+ 	{ "ubyte",   0,  "ubyte",   false,  true,  false,  0, 0,                   0,         UCHAR_MAX,   1 },
+ 	{ "char",    0,  "char",    false,  true,  false,  0, 0,                   0,         UCHAR_MAX,   1 },
+ 	{ "short",   0,  "short",   true,   true,  false,  SHRT_MIN,  SHRT_MAX,    0, 0,                   2 },
+@@ -93,7 +93,7 @@ HostType hostTypesGo[] =
+ 
+ HostType hostTypesJava[] = 
+ {
+-	{ "byte",    0,  "byte",   true,   true,  false,  CHAR_MIN,  CHAR_MAX,    0, 0,                   1 },
++	{ "byte",    0,  "byte",   true,   true,  false,  SCHAR_MIN, SCHAR_MAX,   0, 0,                   1 },
+ 	{ "short",   0,  "short",  true,   true,  false,  SHRT_MIN,  SHRT_MAX,    0, 0,                   2 },
+ 	{ "char",    0,  "char",   false,  true,  false,  0, 0,                   0,         USHRT_MAX,   2 },
+ 	{ "int",     0,  "int",    true,   true,  false,  INT_MIN,   INT_MAX,     0, 0,                   4 },
+@@ -102,13 +102,13 @@ HostType hostTypesJava[] =
+ /* What are the appropriate types for ruby? */
+ HostType hostTypesRuby[] = 
+ {
+-	{ "char",    0,  "char",   true,   true,  false,  CHAR_MIN,  CHAR_MAX,    0, 0, 1 },
++	{ "char",    0,  "char",   true,   true,  false,  SCHAR_MIN, SCHAR_MAX,   0, 0, 1 },
+ 	{ "int",     0,  "int",    true,   true,  false,  INT_MIN,   INT_MAX,     0, 0, 4 },
+ };
+ 
+ HostType hostTypesCSharp[] =
+ {
+-	{ "sbyte",   0,  "sbyte",   true,   true,  false,  CHAR_MIN,  CHAR_MAX,    0, 0,                   1 },
++	{ "sbyte",   0,  "sbyte",   true,   true,  false,  SCHAR_MIN, SCHAR_MAX,   0, 0,                   1 },
+ 	{ "byte",    0,  "byte",    false,  true,  false,  0, 0,                   0,         UCHAR_MAX,   1 },
+ 	{ "short",   0,  "short",   true,   true,  false,  SHRT_MIN,  SHRT_MAX,    0, 0,                   2 },
+ 	{ "ushort",  0,  "ushort",  false,  true,  false,  0, 0,                   0,         USHRT_MAX,   2 },
diff --git a/gnu/packages/ragel.scm b/gnu/packages/ragel.scm
index 1d9b67a6e0..e3fc980bd7 100644
--- a/gnu/packages/ragel.scm
+++ b/gnu/packages/ragel.scm
@@ -34,7 +34,9 @@ (define-public ragel
                                   version ".tar.gz"))
               (sha256
                (base32
-                "0gvcsl62gh6sg73nwaxav4a5ja23zcnyxncdcdnqa2yjcpdnw5az"))))
+                "0gvcsl62gh6sg73nwaxav4a5ja23zcnyxncdcdnqa2yjcpdnw5az"))
+              (patches
+               (search-patches "ragel-specify-char-signedness.patch"))))
     (build-system gnu-build-system)
     (home-page "https://www.colm.net/open-source/ragel/")
     (synopsis "State machine compiler")
-- 
2.33.0

Tobias Geerinckx-Rice wrote on 16 Oct 2021 19:42

Re: [bug#51241] [PATCH 0/1] gnu: ragel: Fix build of knot on aarch64-linux.

Recipients:(name . Simon South)(address . simon@simonsouth.net)

Message-ID:87pms4c17r.fsf@nckx

Simon,

Thanks for taking care of Knot. The changes themselves LGTM.

Simon South ???

Toggle quote (2 lines)

> With the patch applied

Mmm, that's some very raw diff! :-)

Is this an (applied?) upstream patch? Or did you write it, and if

so, have you submitted it for upstream inclusion anywhere its

progress can be tracked?

Kind regards,

T G-R

-----BEGIN PGP SIGNATURE-----

iIMEARYKACsWIQT12iAyS4c9C3o4dnINsP+IT1VteQUCYWsP2A0cbWVAdG9iaWFz

LmdyAAoJEA2w/4hPVW15/XUA/2sFhkwifhAydI+ZRva4TzPsjjLH4bWf/hbQgrwi

3P5mAP9UHl91GErru51lfcvG5u4cf01puuRqZF1iK+CsFh4cBQ==

=DOgZ

-----END PGP SIGNATURE-----

Simon South wrote on 16 Oct 2021 22:09

Recipients:(name . Tobias Geerinckx-Rice)(address . me@tobias.gr)(address . 51241@debbugs.gnu.org)

Message-ID:87v91w91ft.fsf@simonsouth.net

Tobias Geerinckx-Rice <me@tobias.gr> writes:

Toggle quote (3 lines)

> Or did you write it, and if so, have you submitted it for upstream

> inclusion anywhere its progress can be tracked?

Yes, the patch is of my own creation (borrowing heavily from the

upstream commit I mention) but I haven't tried submitting it upstream

yet. I agree that is where it really belongs, though.

I'll give that a shot and see what response I get.

Simon South

simon@simonsouth.net

Tobias Geerinckx-Rice wrote on 16 Oct 2021 23:37

Recipients:(name . Simon South)(address . simon@simonsouth.net)(address . 51241@debbugs.gnu.org)

Message-ID:87k0icbqdy.fsf@nckx

Simon South ???

Toggle quote (2 lines)

> I'll give that a shot and see what response I get.

Thanks! Let us know how it goes.

We don't have to wait for an upstream response to add it to Guix,

but it's nice to be able to refer to the issue URL.

Kind regards,

T G-R

-----BEGIN PGP SIGNATURE-----

iIMEARYKACsWIQT12iAyS4c9C3o4dnINsP+IT1VteQUCYWtGqQ0cbWVAdG9iaWFz

LmdyAAoJEA2w/4hPVW15O4UA/0gFau6WaK2qOqTEz5pJ3x4cx55v5UPoXE+RPO3f

rmL1AQDMlwlUs0uHt6op0IerKFZVb0bGTrJEO430NFlny7EJBw==

=6sne

-----END PGP SIGNATURE-----

Simon South wrote on 12 Nov 2021 21:44

[PATCH v2 0/1] gnu: ragel: Fix build of knot on aarch64-linux.

Recipients:(address . 51241@debbugs.gnu.org)

Message-ID:cover.1636745005.git.simon@simonsouth.net

After corresponding with Ragel's author, here's a patch that applies the

solution implemented upstream[0], backported from Ragel's "ragel-6" branch.

I've tested this on AArch64 and x86-64 and with these changes applied, Knot

builds fine again on both platforms.

On x86-64, I've once again rebuilt the output of "guix refresh

--list-dependent ragel" without issue, aside from ccextractor; on AArch64,

Ragel's immediate dependents[1] all build fine.

[0] https://github.com/adrian-thurston/ragel/commit/2e638fccd81e96ce09841adc4b295b5ce694ea73

[1] gpick, knot, rspamd, ruby-json-pure, ruby-parser and ruby-regexp-parser.

Simon South

simon@simonsouth.net

Simon South (1):

gnu: ragel: Fix build of knot on aarch64-linux.

gnu/local.mk | 1 +

.../ragel-decide-signedness-of-char.patch | 42 +++++++++++++++++++

gnu/packages/ragel.scm | 7 +++-

3 files changed, 49 insertions(+), 1 deletion(-)

create mode 100644 gnu/packages/patches/ragel-decide-signedness-of-char.patch

2.33.1

Simon South wrote on 12 Nov 2021 21:44

[PATCH v2 1/1] gnu: ragel: Fix build of knot on aarch64-linux.

Recipients:(address . 51241@debbugs.gnu.org)

Message-ID:fd192b7ad36efec8675d25356006385926fe4c23.1636745005.git.simon@simonsouth.net

Apply a patch backported from Ragel's "ragel-6" branch that allows it to
reliably generate usable code on systems such as aarch64-linux where the C/C++
"char" type is unsigned by default, fixing the build of Knot on these systems.

* gnu/packages/patches/ragel-decide-signedness-of-char.patch: New file.
* gnu/local.mk (dist_patch_DATA): Add it.
* gnu/packages/ragel.scm (ragel)[source]: Apply it.
---
 gnu/local.mk                                  |  1 +
 .../ragel-decide-signedness-of-char.patch     | 42 +++++++++++++++++++
 gnu/packages/ragel.scm                        |  7 +++-
 3 files changed, 49 insertions(+), 1 deletion(-)
 create mode 100644 gnu/packages/patches/ragel-decide-signedness-of-char.patch

Toggle diff (80 lines)diff --git a/gnu/local.mk b/gnu/local.mk
index 208875754b..2b66d7c07c 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -1717,6 +1717,7 @@ dist_patch_DATA =						\
   %D%/packages/patches/quassel-qt-514-compat.patch		\
   %D%/packages/patches/quickswitch-fix-dmenu-check.patch	\
   %D%/packages/patches/qtwebkit-pbutils-include.patch		\
+  %D%/packages/patches/ragel-decide-signedness-of-char.patch	\
   %D%/packages/patches/randomjungle-disable-static-build.patch	\
   %D%/packages/patches/rapicorn-isnan.patch			\
   %D%/packages/patches/rapidjson-gcc-compat.patch		\
diff --git a/gnu/packages/patches/ragel-decide-signedness-of-char.patch b/gnu/packages/patches/ragel-decide-signedness-of-char.patch
new file mode 100644
index 0000000000..b3b2bf958a
--- /dev/null
+++ b/gnu/packages/patches/ragel-decide-signedness-of-char.patch
@@ -0,0 +1,42 @@
+From 2e638fccd81e96ce09841adc4b295b5ce694ea73 Mon Sep 17 00:00:00 2001
+From: Adrian Thurston <thurston@colm.net>
+Date: Sat, 6 Nov 2021 12:20:05 -0700
+Subject: [PATCH] C char type: decide signedness of char based on CHAR_MIN
+
+Previously had char fixed to signed char, this is not useful on ARM as it does
+not align with the host type. Instead, decide at runtime (or probably compile
+time) if char is signed or not.
+---
+ ragel/common.cpp | 16 ++++++++--------
+ 1 file changed, 8 insertions(+), 8 deletions(-)
+
+diff --git a/ragel/common.cpp b/ragel/common.cpp
+index 8e9f8ed0..55875c06 100644
+--- a/ragel/common.cpp
++++ b/ragel/common.cpp
+@@ -27,14 +27,14 @@
+ 
+ HostType hostTypesC[] =
+ {
+-	{ "char",     0,       "char",    true,   true,  false,  CHAR_MIN,  CHAR_MAX,   0, 0,              sizeof(char) },
+-	{ "unsigned", "char",  "uchar",   false,  true,  false,  0, 0,                  0,     UCHAR_MAX,  sizeof(unsigned char) },
+-	{ "short",    0,       "short",   true,   true,  false,  SHRT_MIN,  SHRT_MAX,   0, 0,              sizeof(short) },
+-	{ "unsigned", "short", "ushort",  false,  true,  false,  0, 0,                  0,     USHRT_MAX,  sizeof(unsigned short) },
+-	{ "int",      0,       "int",     true,   true,  false,  INT_MIN,   INT_MAX,    0, 0,              sizeof(int) },
+-	{ "unsigned", "int",   "uint",    false,  true,  false,  0, 0,                  0,     UINT_MAX,   sizeof(unsigned int) },
+-	{ "long",     0,       "long",    true,   true,  false,  LONG_MIN,  LONG_MAX,   0, 0,              sizeof(long) },
+-	{ "unsigned", "long",  "ulong",   false,  true,  false,  0, 0,                  0,     ULONG_MAX,  sizeof(unsigned long) }
++	{ "char",     0,       "char",    (CHAR_MIN != 0), true,  false,  SCHAR_MIN, SCHAR_MAX,  0, UCHAR_MAX, sizeof(char) },
++	{ "unsigned", "char",  "uchar",   false,           true,  false,  0, 0,                  0, UCHAR_MAX, sizeof(unsigned char) },
++	{ "short",    0,       "short",   true,            true,  false,  SHRT_MIN,  SHRT_MAX,   0, 0,         sizeof(short) },
++	{ "unsigned", "short", "ushort",  false,           true,  false,  0, 0,                  0, USHRT_MAX, sizeof(unsigned short) },
++	{ "int",      0,       "int",     true,            true,  false,  INT_MIN,   INT_MAX,    0, 0,         sizeof(int) },
++	{ "unsigned", "int",   "uint",    false,           true,  false,  0, 0,                  0, UINT_MAX,  sizeof(unsigned int) },
++	{ "long",     0,       "long",    true,            true,  false,  LONG_MIN,  LONG_MAX,   0, 0,         sizeof(long) },
++	{ "unsigned", "long",  "ulong",   false,           true,  false,  0, 0,                  0, ULONG_MAX, sizeof(unsigned long) }
+ };
+ 
+ #define S8BIT_MIN  -128
+-- 
+2.33.1
+
diff --git a/gnu/packages/ragel.scm b/gnu/packages/ragel.scm
index 1d9b67a6e0..8b5100330a 100644
--- a/gnu/packages/ragel.scm
+++ b/gnu/packages/ragel.scm
@@ -34,7 +34,12 @@ (define-public ragel
                                   version ".tar.gz"))
               (sha256
                (base32
-                "0gvcsl62gh6sg73nwaxav4a5ja23zcnyxncdcdnqa2yjcpdnw5az"))))
+                "0gvcsl62gh6sg73nwaxav4a5ja23zcnyxncdcdnqa2yjcpdnw5az"))
+              (patches
+               (search-patches
+                ;; A backported fix for aarch64-linux, where the C/C++ "char"
+                ;; type is unsigned by default.
+                "ragel-decide-signedness-of-char.patch"))))
     (build-system gnu-build-system)
     (home-page "https://www.colm.net/open-source/ragel/")
     (synopsis "State machine compiler")
-- 
2.33.1

Ludovic Courtès wrote on 19 Dec 2021 23:30

Re: bug#51241: [PATCH 0/1] gnu: ragel: Fix build of knot on aarch64-linux.

Recipients:(name . Simon South)(address . simon@simonsouth.net)(address . 51241@debbugs.gnu.org)

Message-ID:8735momdml.fsf_-_@gnu.org

Hi,

Simon South <simon@simonsouth.net> skribis:

Toggle quote (8 lines)

> Apply a patch backported from Ragel's "ragel-6" branch that allows it to

> reliably generate usable code on systems such as aarch64-linux where the C/C++

> "char" type is unsigned by default, fixing the build of Knot on these systems.

> * gnu/packages/patches/ragel-decide-signedness-of-char.patch: New file.

> * gnu/local.mk (dist_patch_DATA): Add it.

> * gnu/packages/ragel.scm (ragel)[source]: Apply it.

Could you check whether this is still needed on current master,

ca. 9b38d9b3b34edb55f7f42b72a611b39e5164cf9f?

‘guix refresh -l ragel’ reports 5K dependents, so we’d need to apply the

patch conditionally for aarch64 in a phase, or something along these

lines.

Thanks,

Ludo’.

Simon South wrote on 20 Dec 2021 23:57

Recipients:(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 51241@debbugs.gnu.org)

Message-ID:874k72yjdv.fsf@simonsouth.net

Ludovic Courtès <ludo@gnu.org> writes:

Toggle quote (3 lines)

> Could you check whether this is still needed on current master,

> ca. 9b38d9b3b34edb55f7f42b72a611b39e5164cf9f?

Unfortunately yes; Knot is still failing two "yparser" tests in master

(2c469f04a3be) on AArch64.

Toggle quote (4 lines)

> ‘guix refresh -l ragel’ reports 5K dependents, so we’d need to apply the

> patch conditionally for aarch64 in a phase, or something along these

> lines.

I can submit a new patch with this change.  Or would it make more sense
to apply the existing patch to the version-1.4.0 branch, since it's
fixing an issue in master?

-- 
Simon South
simon@simonsouth.net

Ludovic Courtès wrote on 21 Dec 2021 10:12

Recipients:(name . Simon South)(address . simon@simonsouth.net)(address . 51241@debbugs.gnu.org)

Message-ID:877dbyb9tq.fsf@gnu.org

Hi,

Simon South <simon@simonsouth.net> skribis:

Toggle quote (15 lines)> Ludovic Courtès <ludo@gnu.org> writes:
>> Could you check whether this is still needed on current master,
>> ca. 9b38d9b3b34edb55f7f42b72a611b39e5164cf9f?
>
> Unfortunately yes; Knot is still failing two "yparser" tests in master
> (2c469f04a3be) on AArch64.
>
>> ‘guix refresh -l ragel’ reports 5K dependents, so we’d need to apply the
>> patch conditionally for aarch64 in a phase, or something along these
>> lines.
>
> I can submit a new patch with this change.  Or would it make more sense
> to apply the existing patch to the version-1.4.0 branch, since it's
> fixing an issue in master?

Let’s apply a patch that minimizes rebuilds on ‘master’ and we’ll

cherry-pick in ‘version-1.4.0’.

Thanks,

Ludo’.

Simon South wrote on 29 Dec 2021 18:46

Recipients:(address . 51241@debbugs.gnu.org)(address . ludo@gnu.org)

Message-ID:cover.1640799363.git.simon@simonsouth.net

Ludovic Courtès <ludo@gnu.org> writes:

Toggle quote (3 lines)

> Let’s apply a patch that minimizes rebuilds on ‘master’ and we’ll

> cherry-pick in ‘version-1.4.0’.

Here's an updated patch that should do the trick, tested on AArch64 and on

x86-64 (where it has no effect).

Note I've shortened the name of the patch file in addition to modifying the

commit message from the original.

Simon South (1):

gnu: ragel: Fix build of knot on aarch64-linux.

gnu/local.mk | 1 +

.../patches/ragel-char-signedness.patch | 42 +++++++++++++++++++

gnu/packages/ragel.scm | 29 ++++++++++++-

3 files changed, 71 insertions(+), 1 deletion(-)

create mode 100644 gnu/packages/patches/ragel-char-signedness.patch

base-commit: 2be274915023cdc0f4b0f4a5ab3690f53c2156c4

2.34.0

Simon South wrote on 29 Dec 2021 18:46

[PATCH 1/1] gnu: ragel: Fix build of knot on aarch64-linux.

Recipients:(address . 51241@debbugs.gnu.org)(address . ludo@gnu.org)

Message-ID:8d836816bdc6015fce6a8d4dba10cac38157364b.1640799363.git.simon@simonsouth.net

Apply a patch backported from Ragel's "ragel-6" branch that allows it to
reliably generate usable code on aarch64-linux where the C/C++ "char" type is
unsigned by default, fixing the build of Knot on this platform.

* gnu/packages/patches/ragel-char-signedness.patch: New file.
* gnu/local.mk (dist_patch_DATA): Add it.
* gnu/packages/ragel.scm (ragel)[arguments]: Add custom phase for AArch64 that
applies the patch.
[native-inputs]: Add "patch" and patch file on AArch64.
---
 gnu/local.mk                                  |  1 +
 .../patches/ragel-char-signedness.patch       | 42 +++++++++++++++++++
 gnu/packages/ragel.scm                        | 29 ++++++++++++-
 3 files changed, 71 insertions(+), 1 deletion(-)
 create mode 100644 gnu/packages/patches/ragel-char-signedness.patch

Toggle diff (116 lines)diff --git a/gnu/local.mk b/gnu/local.mk
index 21e536a635..ebc5426e81 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -1726,6 +1726,7 @@ dist_patch_DATA =						\
   %D%/packages/patches/qtwebkit-fix-building-with-python-3.9.patch	\
   %D%/packages/patches/qtwebkit-fix-building-with-icu-68.patch	\
   %D%/packages/patches/qtwebkit-fix-building-with-glib-2.68.patch	\
+  %D%/packages/patches/ragel-char-signedness.patch		\
   %D%/packages/patches/randomjungle-disable-static-build.patch	\
   %D%/packages/patches/range-v3-build-with-gcc10.patch	\
   %D%/packages/patches/rapicorn-isnan.patch			\
diff --git a/gnu/packages/patches/ragel-char-signedness.patch b/gnu/packages/patches/ragel-char-signedness.patch
new file mode 100644
index 0000000000..b3b2bf958a
--- /dev/null
+++ b/gnu/packages/patches/ragel-char-signedness.patch
@@ -0,0 +1,42 @@
+From 2e638fccd81e96ce09841adc4b295b5ce694ea73 Mon Sep 17 00:00:00 2001
+From: Adrian Thurston <thurston@colm.net>
+Date: Sat, 6 Nov 2021 12:20:05 -0700
+Subject: [PATCH] C char type: decide signedness of char based on CHAR_MIN
+
+Previously had char fixed to signed char, this is not useful on ARM as it does
+not align with the host type. Instead, decide at runtime (or probably compile
+time) if char is signed or not.
+---
+ ragel/common.cpp | 16 ++++++++--------
+ 1 file changed, 8 insertions(+), 8 deletions(-)
+
+diff --git a/ragel/common.cpp b/ragel/common.cpp
+index 8e9f8ed0..55875c06 100644
+--- a/ragel/common.cpp
++++ b/ragel/common.cpp
+@@ -27,14 +27,14 @@
+ 
+ HostType hostTypesC[] =
+ {
+-	{ "char",     0,       "char",    true,   true,  false,  CHAR_MIN,  CHAR_MAX,   0, 0,              sizeof(char) },
+-	{ "unsigned", "char",  "uchar",   false,  true,  false,  0, 0,                  0,     UCHAR_MAX,  sizeof(unsigned char) },
+-	{ "short",    0,       "short",   true,   true,  false,  SHRT_MIN,  SHRT_MAX,   0, 0,              sizeof(short) },
+-	{ "unsigned", "short", "ushort",  false,  true,  false,  0, 0,                  0,     USHRT_MAX,  sizeof(unsigned short) },
+-	{ "int",      0,       "int",     true,   true,  false,  INT_MIN,   INT_MAX,    0, 0,              sizeof(int) },
+-	{ "unsigned", "int",   "uint",    false,  true,  false,  0, 0,                  0,     UINT_MAX,   sizeof(unsigned int) },
+-	{ "long",     0,       "long",    true,   true,  false,  LONG_MIN,  LONG_MAX,   0, 0,              sizeof(long) },
+-	{ "unsigned", "long",  "ulong",   false,  true,  false,  0, 0,                  0,     ULONG_MAX,  sizeof(unsigned long) }
++	{ "char",     0,       "char",    (CHAR_MIN != 0), true,  false,  SCHAR_MIN, SCHAR_MAX,  0, UCHAR_MAX, sizeof(char) },
++	{ "unsigned", "char",  "uchar",   false,           true,  false,  0, 0,                  0, UCHAR_MAX, sizeof(unsigned char) },
++	{ "short",    0,       "short",   true,            true,  false,  SHRT_MIN,  SHRT_MAX,   0, 0,         sizeof(short) },
++	{ "unsigned", "short", "ushort",  false,           true,  false,  0, 0,                  0, USHRT_MAX, sizeof(unsigned short) },
++	{ "int",      0,       "int",     true,            true,  false,  INT_MIN,   INT_MAX,    0, 0,         sizeof(int) },
++	{ "unsigned", "int",   "uint",    false,           true,  false,  0, 0,                  0, UINT_MAX,  sizeof(unsigned int) },
++	{ "long",     0,       "long",    true,            true,  false,  LONG_MIN,  LONG_MAX,   0, 0,         sizeof(long) },
++	{ "unsigned", "long",  "ulong",   false,           true,  false,  0, 0,                  0, ULONG_MAX, sizeof(unsigned long) }
+ };
+ 
+ #define S8BIT_MIN  -128
+-- 
+2.33.1
+
diff --git a/gnu/packages/ragel.scm b/gnu/packages/ragel.scm
index 1d9b67a6e0..d4016ed5ba 100644
--- a/gnu/packages/ragel.scm
+++ b/gnu/packages/ragel.scm
@@ -1,6 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2015 Ricardo Wurmus <rekado@elephly.net>
 ;;; Copyright © 2019 Tobias Geerinckx-Rice <me@tobias.gr>
+;;; Copyright © 2021 Simon South <simon@simonsouth.net>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -22,7 +23,9 @@ (define-module (gnu packages ragel)
   #:use-module (guix packages)
   #:use-module (guix download)
   #:use-module (guix build-system gnu)
-  #:use-module (gnu packages))
+  #:use-module (guix utils)
+  #:use-module (gnu packages)
+  #:use-module (gnu packages base))
 
 (define-public ragel
   (package
@@ -36,6 +39,30 @@ (define-public ragel
                (base32
                 "0gvcsl62gh6sg73nwaxav4a5ja23zcnyxncdcdnqa2yjcpdnw5az"))))
     (build-system gnu-build-system)
+    (arguments
+     (if (target-aarch64?)
+         '(#:phases
+           (modify-phases %standard-phases
+             (add-after 'unpack 'apply-char-signedness-fix
+               ;; Apply a backported fix for aarch64-linux, where the C/C++
+               ;; "char" type is unsigned by default.
+               ;;
+               ;; The patch is applied in this custom phase and not via the
+               ;; "origin" object above to avoid rebuilding a large number of
+               ;; packages on other platforms.
+               (lambda _
+                 (let ((patch
+                        (search-input-file %build-inputs "/bin/patch"))
+                       (char-signedness-patch
+                        (assoc-ref %build-inputs "char-signedness-patch")))
+                   (invoke patch "-p1" "-i" char-signedness-patch))))))
+         '()))
+    (native-inputs
+     (if (target-aarch64?)
+         `(("char-signedness-patch"
+            ,(search-patch "ragel-char-signedness.patch"))
+           ("patch" ,patch))
+         '()))
     (home-page "https://www.colm.net/open-source/ragel/")
     (synopsis "State machine compiler")
     (description
-- 
2.34.0

Ludovic Courtès wrote on 31 Dec 2021 18:29

Recipients:(name . Simon South)(address . simon@simonsouth.net)(address . 51241-done@debbugs.gnu.org)

Message-ID:87fsq8ptrl.fsf@gnu.org

Hi,

Simon South <simon@simonsouth.net> skribis:

Toggle quote (10 lines)> Apply a patch backported from Ragel's "ragel-6" branch that allows it to
> reliably generate usable code on aarch64-linux where the C/C++ "char" type is
> unsigned by default, fixing the build of Knot on this platform.
>
> * gnu/packages/patches/ragel-char-signedness.patch: New file.
> * gnu/local.mk (dist_patch_DATA): Add it.
> * gnu/packages/ragel.scm (ragel)[arguments]: Add custom phase for AArch64 that
> applies the patch.
> [native-inputs]: Add "patch" and patch file on AArch64.

Applied, thanks!

Ludo’.

Closed

Your comment

This issue is archived.

To comment on this conversation send an email to 51241@debbugs.gnu.org

is:open	open issues
is:done	closed issues
submitter:<who>	search issue submitter
author:<who>	search by message author
date:yesterday..now	search by issue date
mdate:3m..2d	search by message date