unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
* [bug#51241] [PATCH 0/1] gnu: ragel: Fix build of knot on aarch64-linux.
@ 2021-10-16 17:04 Simon South
  2021-10-16 17:06 ` [bug#51241] [PATCH 1/1] " Simon South
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Simon South @ 2021-10-16 17:04 UTC (permalink / raw)
  To: 51241

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





^ permalink raw reply	[flat|nested] 13+ messages in thread

* [bug#51241] [PATCH 1/1] gnu: ragel: Fix build of knot on aarch64-linux.
  2021-10-16 17:04 [bug#51241] [PATCH 0/1] gnu: ragel: Fix build of knot on aarch64-linux Simon South
@ 2021-10-16 17:06 ` Simon South
  2021-10-16 17:42 ` [bug#51241] [PATCH 0/1] " Tobias Geerinckx-Rice via Guix-patches via
  2021-11-12 20:44 ` [bug#51241] [PATCH v2 " Simon South
  2 siblings, 0 replies; 13+ messages in thread
From: Simon South @ 2021-10-16 17:06 UTC (permalink / raw)
  To: 51241

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

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





^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [bug#51241] [PATCH 0/1] gnu: ragel: Fix build of knot on aarch64-linux.
  2021-10-16 17:04 [bug#51241] [PATCH 0/1] gnu: ragel: Fix build of knot on aarch64-linux Simon South
  2021-10-16 17:06 ` [bug#51241] [PATCH 1/1] " Simon South
@ 2021-10-16 17:42 ` Tobias Geerinckx-Rice via Guix-patches via
  2021-10-16 20:09   ` Simon South
  2021-11-12 20:44 ` [bug#51241] [PATCH v2 " Simon South
  2 siblings, 1 reply; 13+ messages in thread
From: Tobias Geerinckx-Rice via Guix-patches via @ 2021-10-16 17:42 UTC (permalink / raw)
  To: Simon South; +Cc: 51241

[-- Attachment #1: Type: text/plain, Size: 350 bytes --]

Simon,

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

Simon South 写道:
> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 247 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [bug#51241] [PATCH 0/1] gnu: ragel: Fix build of knot on aarch64-linux.
  2021-10-16 17:42 ` [bug#51241] [PATCH 0/1] " Tobias Geerinckx-Rice via Guix-patches via
@ 2021-10-16 20:09   ` Simon South
  2021-10-16 21:37     ` Tobias Geerinckx-Rice via Guix-patches via
  0 siblings, 1 reply; 13+ messages in thread
From: Simon South @ 2021-10-16 20:09 UTC (permalink / raw)
  To: Tobias Geerinckx-Rice; +Cc: 51241

Tobias Geerinckx-Rice <me@tobias.gr> writes:
> 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




^ permalink raw reply	[flat|nested] 13+ messages in thread

* [bug#51241] [PATCH 0/1] gnu: ragel: Fix build of knot on aarch64-linux.
  2021-10-16 20:09   ` Simon South
@ 2021-10-16 21:37     ` Tobias Geerinckx-Rice via Guix-patches via
  0 siblings, 0 replies; 13+ messages in thread
From: Tobias Geerinckx-Rice via Guix-patches via @ 2021-10-16 21:37 UTC (permalink / raw)
  To: Simon South; +Cc: 51241

[-- Attachment #1: Type: text/plain, Size: 263 bytes --]

Simon South 写道:
> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 247 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [bug#51241] [PATCH v2 0/1] gnu: ragel: Fix build of knot on aarch64-linux.
  2021-10-16 17:04 [bug#51241] [PATCH 0/1] gnu: ragel: Fix build of knot on aarch64-linux Simon South
  2021-10-16 17:06 ` [bug#51241] [PATCH 1/1] " Simon South
  2021-10-16 17:42 ` [bug#51241] [PATCH 0/1] " Tobias Geerinckx-Rice via Guix-patches via
@ 2021-11-12 20:44 ` Simon South
  2021-11-12 20:44   ` [bug#51241] [PATCH v2 1/1] " Simon South
  2 siblings, 1 reply; 13+ messages in thread
From: Simon South @ 2021-11-12 20:44 UTC (permalink / raw)
  To: 51241

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





^ permalink raw reply	[flat|nested] 13+ messages in thread

* [bug#51241] [PATCH v2 1/1] gnu: ragel: Fix build of knot on aarch64-linux.
  2021-11-12 20:44 ` [bug#51241] [PATCH v2 " Simon South
@ 2021-11-12 20:44   ` Simon South
  2021-12-19 22:30     ` [bug#51241] [PATCH 0/1] " Ludovic Courtès
  0 siblings, 1 reply; 13+ messages in thread
From: Simon South @ 2021-11-12 20:44 UTC (permalink / raw)
  To: 51241

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

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





^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [bug#51241] [PATCH 0/1] gnu: ragel: Fix build of knot on aarch64-linux.
  2021-11-12 20:44   ` [bug#51241] [PATCH v2 1/1] " Simon South
@ 2021-12-19 22:30     ` Ludovic Courtès
  2021-12-20 22:57       ` Simon South
  0 siblings, 1 reply; 13+ messages in thread
From: Ludovic Courtès @ 2021-12-19 22:30 UTC (permalink / raw)
  To: Simon South; +Cc: 51241

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




^ permalink raw reply	[flat|nested] 13+ messages in thread

* [bug#51241] [PATCH 0/1] gnu: ragel: Fix build of knot on aarch64-linux.
  2021-12-19 22:30     ` [bug#51241] [PATCH 0/1] " Ludovic Courtès
@ 2021-12-20 22:57       ` Simon South
  2021-12-21  9:12         ` Ludovic Courtès
  0 siblings, 1 reply; 13+ messages in thread
From: Simon South @ 2021-12-20 22:57 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 51241

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?

-- 
Simon South
simon@simonsouth.net




^ permalink raw reply	[flat|nested] 13+ messages in thread

* [bug#51241] [PATCH 0/1] gnu: ragel: Fix build of knot on aarch64-linux.
  2021-12-20 22:57       ` Simon South
@ 2021-12-21  9:12         ` Ludovic Courtès
  2021-12-29 17:46           ` Simon South
  0 siblings, 1 reply; 13+ messages in thread
From: Ludovic Courtès @ 2021-12-21  9:12 UTC (permalink / raw)
  To: Simon South; +Cc: 51241

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




^ permalink raw reply	[flat|nested] 13+ messages in thread

* [bug#51241] [PATCH 0/1] gnu: ragel: Fix build of knot on aarch64-linux.
  2021-12-21  9:12         ` Ludovic Courtès
@ 2021-12-29 17:46           ` Simon South
  2021-12-29 17:46             ` [bug#51241] [PATCH 1/1] " Simon South
  0 siblings, 1 reply; 13+ messages in thread
From: Simon South @ 2021-12-29 17:46 UTC (permalink / raw)
  To: 51241; +Cc: 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
-- 
2.34.0





^ permalink raw reply	[flat|nested] 13+ messages in thread

* [bug#51241] [PATCH 1/1] gnu: ragel: Fix build of knot on aarch64-linux.
  2021-12-29 17:46           ` Simon South
@ 2021-12-29 17:46             ` Simon South
  2021-12-31 17:29               ` bug#51241: " Ludovic Courtès
  0 siblings, 1 reply; 13+ messages in thread
From: Simon South @ 2021-12-29 17:46 UTC (permalink / raw)
  To: 51241; +Cc: ludo

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

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





^ permalink raw reply related	[flat|nested] 13+ messages in thread

* bug#51241: [PATCH 1/1] gnu: ragel: Fix build of knot on aarch64-linux.
  2021-12-29 17:46             ` [bug#51241] [PATCH 1/1] " Simon South
@ 2021-12-31 17:29               ` Ludovic Courtès
  0 siblings, 0 replies; 13+ messages in thread
From: Ludovic Courtès @ 2021-12-31 17:29 UTC (permalink / raw)
  To: Simon South; +Cc: 51241-done

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




^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2021-12-31 17:30 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-10-16 17:04 [bug#51241] [PATCH 0/1] gnu: ragel: Fix build of knot on aarch64-linux Simon South
2021-10-16 17:06 ` [bug#51241] [PATCH 1/1] " Simon South
2021-10-16 17:42 ` [bug#51241] [PATCH 0/1] " Tobias Geerinckx-Rice via Guix-patches via
2021-10-16 20:09   ` Simon South
2021-10-16 21:37     ` Tobias Geerinckx-Rice via Guix-patches via
2021-11-12 20:44 ` [bug#51241] [PATCH v2 " Simon South
2021-11-12 20:44   ` [bug#51241] [PATCH v2 1/1] " Simon South
2021-12-19 22:30     ` [bug#51241] [PATCH 0/1] " Ludovic Courtès
2021-12-20 22:57       ` Simon South
2021-12-21  9:12         ` Ludovic Courtès
2021-12-29 17:46           ` Simon South
2021-12-29 17:46             ` [bug#51241] [PATCH 1/1] " Simon South
2021-12-31 17:29               ` bug#51241: " Ludovic Courtès

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).