| Message ID | fd192b7ad36efec8675d25356006385926fe4c23.1636745005.git.simon@simonsouth.net |
|---|---|
| State | Accepted |
| Headers | show |
| Series | gnu: ragel: Fix build of knot on aarch64-linux. | expand |
| Context | Check | Description |
|---|---|---|
| cbaines/comparison | success | View comparision |
| cbaines/git branch | success | View Git branch |
| cbaines/applying patch | success | View Laminar job |
| cbaines/issue | success | View issue |
Hi, Simon South <simon@simonsouth.net> skribis: > 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’.
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?
Hi, Simon South <simon@simonsouth.net> skribis: > 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’.
Ludovic Courtès <ludo@gnu.org> writes: > 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
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")