subject:"\[armv7 fix\] Build database\/redis"

Re: [armv7 fix] Build database/redis

2018-04-21 Thread Klemens Nanni

On Wed, Apr 18, 2018 at 04:04:43AM +0200, Mark Kettenis wrote:
> I'm travelling and don't have a lot of spare cycles to participate in
> this discussion at the moment.
> 
> I have suggested providing a separate libunwind in base with the LLVM
> unwinder.  I think I even provided a diff at some point.  But it is up
> to you ports guys to decide whether that is helpful or not.  Some
> ports may pick this up automatically but may expect a different
> libunwind implementation and therefore break.
This might work but will definitely need testing.

For now I'd like to unbreak redis on armv7, preferably by removing
-funwind-tables from CFLAGS as suggested by jca.

More Feedback? OK?

Index: Makefile
===
RCS file: /cvs/ports/databases/redis/Makefile,v
retrieving revision 1.101
diff -u -p -r1.101 Makefile
--- Makefile19 Mar 2018 18:15:57 -  1.101
+++ Makefile21 Apr 2018 19:13:00 -
@@ -1,8 +1,12 @@
 # $OpenBSD: Makefile,v 1.101 2018/03/19 18:15:57 sthen Exp $
 
 COMMENT =  persistent key-value database
+
 DISTNAME = redis-4.0.8
+REVISION = 0
+
 CATEGORIES =   databases
+
 HOMEPAGE = https://redis.io/
 
 MAINTAINER =   David Coppa 
Index: patches/patch-src_Makefile
===
RCS file: /cvs/ports/databases/redis/patches/patch-src_Makefile,v
retrieving revision 1.27
diff -u -p -r1.27 patch-src_Makefile
--- patches/patch-src_Makefile  9 Aug 2017 09:16:09 -   1.27
+++ patches/patch-src_Makefile  21 Apr 2018 19:13:00 -
@@ -21,6 +21,15 @@ Index: src/Makefile
  INSTALL=install
  
  # Default allocator defaults to Jemalloc if it's not an ARM
+@@ -40,7 +41,7 @@ endif
+ 
+ # To get ARM stack traces if Redis crashes we need a special C flag.
+ ifneq (,$(findstring armv,$(uname_M)))
+-CFLAGS+=-funwind-tables
++#CFLAGS+=-funwind-tables
+ endif
+ 
+ # Backwards compatibility for selecting an allocator
 @@ -66,7 +67,7 @@ endif
  FINAL_CFLAGS=$(STD) $(WARN) $(OPT) $(DEBUG) $(CFLAGS) $(REDIS_CFLAGS)
  FINAL_LDFLAGS=$(LDFLAGS) $(REDIS_LDFLAGS) $(DEBUG)

Re: [armv7 fix] Build database/redis

2018-04-17 Thread Mark Kettenis

> Date: Tue, 17 Apr 2018 21:43:17 +0200
> From: Klemens Nanni 
> 
> On Sun, Apr 15, 2018 at 06:34:21PM +0200, Markus Hennecke wrote:
> > On Sat, 14 Apr 2018, Jeremie Courreges-Anglas wrote:
> > 
> > > On Thu, Apr 12 2018, Klemens Nanni  wrote:
> > > > On Sat, Apr 07, 2018 at 08:26:10AM +0200, Markus Hennecke wrote:
> > > >> With the attached fix the redis port builds, but is unable to complete 
> > > >> the 
> > > >> regression tests (see https://github.com/antirez/redis/issues/4640).
> > > > What's the status on armv7 without this fix?
> > > [ ] 
> > > > Have you tried the newer
> > > > 4.0.9 version?
> > > 
> > > *I* haven't, but the snippet above is still in the master branch
> > > upstream, so I wouldn't expect an improvement.
> > 
> > No change with 4.0.9. The build still needs one of the fixes 
> > proposed in this thread and the regression tests still fail.
> 4.0.9 looks good here on amd64 and sparc64, too.
> 
> > > > I don't have access to that platform but the diff looks ok port-wise.
> > > >
> > > > There's another redis diff from me pending on ports@. but I'd also like
> > > > to update the port to 4.0.9, maybe we can combine this with your fix?
> > > 
> > > Personally I prefer when different issues are addressed by separate
> > > commits.
> > > 
> > > 
> > > Index: patch-src_Makefile
> > > ===
> > > RCS file: /d/cvs/ports/databases/redis/patches/patch-src_Makefile,v
> > > retrieving revision 1.27
> > > diff -u -p -p -u -r1.27 patch-src_Makefile
> > > --- patch-src_Makefile9 Aug 2017 09:16:09 -   1.27
> > > +++ patch-src_Makefile14 Apr 2018 00:27:57 -
> > > @@ -21,6 +21,15 @@ Index: src/Makefile
> > >   INSTALL=install
> > >   
> > >   # Default allocator defaults to Jemalloc if it's not an ARM
> > > +@@ -40,7 +41,7 @@ endif
> > > + 
> > > + # To get ARM stack traces if Redis crashes we need a special C flag.
> > > + ifneq (,$(findstring armv,$(uname_M)))
> > > +-CFLAGS+=-funwind-tables
> > > ++#CFLAGS+=-funwind-tables
> > > + endif
> > > + 
> > > + # Backwards compatibility for selecting an allocator
> > >  @@ -66,7 +67,7 @@ endif
> > >   FINAL_CFLAGS=$(STD) $(WARN) $(OPT) $(DEBUG) $(CFLAGS) $(REDIS_CFLAGS)
> > >   FINAL_LDFLAGS=$(LDFLAGS) $(REDIS_LDFLAGS) $(DEBUG)
> > > 
> > 
> > This won't pull in libc++abi, so I think it would be a much cleaner 
> > solution port wise.
> I agree.
> 
> OK kn if one of you wants to go ahead and fix armv7.

I'm travelling and don't have a lot of spare cycles to participate in
this discussion at the moment.

I have suggested providing a separate libunwind in base with the LLVM
unwinder.  I think I even provided a diff at some point.  But it is up
to you ports guys to decide whether that is helpful or not.  Some
ports may pick this up automatically but may expect a different
libunwind implementation and therefore break.

Re: [armv7 fix] Build database/redis

2018-04-15 Thread Markus Hennecke

On Sat, 14 Apr 2018, Jeremie Courreges-Anglas wrote:

> On Thu, Apr 12 2018, Klemens Nanni  wrote:
> > On Sat, Apr 07, 2018 at 08:26:10AM +0200, Markus Hennecke wrote:
> >> With the attached fix the redis port builds, but is unable to complete the 
> >> regression tests (see https://github.com/antirez/redis/issues/4640).
> > What's the status on armv7 without this fix?
> [ ] 
> > Have you tried the newer
> > 4.0.9 version?
> 
> *I* haven't, but the snippet above is still in the master branch
> upstream, so I wouldn't expect an improvement.

No change with 4.0.9. The build still needs one of the fixes 
proposed in this thread and the regression tests still fail.

> > I don't have access to that platform but the diff looks ok port-wise.
> >
> > There's another redis diff from me pending on ports@. but I'd also like
> > to update the port to 4.0.9, maybe we can combine this with your fix?
> 
> Personally I prefer when different issues are addressed by separate
> commits.
> 
> 
> Index: patch-src_Makefile
> ===
> RCS file: /d/cvs/ports/databases/redis/patches/patch-src_Makefile,v
> retrieving revision 1.27
> diff -u -p -p -u -r1.27 patch-src_Makefile
> --- patch-src_Makefile9 Aug 2017 09:16:09 -   1.27
> +++ patch-src_Makefile14 Apr 2018 00:27:57 -
> @@ -21,6 +21,15 @@ Index: src/Makefile
>   INSTALL=install
>   
>   # Default allocator defaults to Jemalloc if it's not an ARM
> +@@ -40,7 +41,7 @@ endif
> + 
> + # To get ARM stack traces if Redis crashes we need a special C flag.
> + ifneq (,$(findstring armv,$(uname_M)))
> +-CFLAGS+=-funwind-tables
> ++#CFLAGS+=-funwind-tables
> + endif
> + 
> + # Backwards compatibility for selecting an allocator
>  @@ -66,7 +67,7 @@ endif
>   FINAL_CFLAGS=$(STD) $(WARN) $(OPT) $(DEBUG) $(CFLAGS) $(REDIS_CFLAGS)
>   FINAL_LDFLAGS=$(LDFLAGS) $(REDIS_LDFLAGS) $(DEBUG)
> 

This won't pull in libc++abi, so I think it would be a much cleaner 
solution port wise.

Regards,
Markus

Re: [armv7 fix] Build database/redis

2018-04-13 Thread Jeremie Courreges-Anglas

On Thu, Apr 12 2018, Klemens Nanni  wrote:
> On Sat, Apr 07, 2018 at 08:26:10AM +0200, Markus Hennecke wrote:
>> With the attached fix the redis port builds, but is unable to complete the 
>> regression tests (see https://github.com/antirez/redis/issues/4640).
> What's the status on armv7 without this fix?

Tons of undefined references to functions from libunwind (included in
libc++abi, hence the fix proposed by Markus):

--8<--
[...]
../deps/hiredis/libhiredis.a(hiredis.o):(.ARM.exidx+0xd8): undefined reference 
to `__aeabi_unwind_cpp_pr1'
../deps/hiredis/libhiredis.a(hiredis.o):(.ARM.exidx+0xe0): more undefined 
references to `__aeabi_unwind_cpp_pr1' follow
../deps/hiredis/libhiredis.a(hiredis.o):(.ARM.exidx+0x118): undefined reference 
to `__aeabi_unwind_cpp_pr0'
../deps/hiredis/libhiredis.a(hiredis.o):(.ARM.exidx+0x120): undefined reference 
to `__aeabi_unwind_cpp_pr1'
../deps/hiredis/libhiredis.a(hiredis.o):(.ARM.exidx+0x128): undefined reference 
to `__aeabi_unwind_cpp_pr1'
../deps/hiredis/libhiredis.a(hiredis.o):(.ARM.exidx+0x130): undefined reference 
to `__aeabi_unwind_cpp_pr1'
../deps/hiredis/libhiredis.a(read.o):(.ARM.exidx+0x0): undefined reference to 
`__aeabi_unwind_cpp_pr1'
../deps/hiredis/libhiredis.a(read.o):(.ARM.exidx+0x8): undefined reference to 
`__aeabi_unwind_cpp_pr1'
../deps/hiredis/libhiredis.a(read.o):(.ARM.exidx+0x10): more undefined 
references to `__aeabi_unwind_cpp_pr1' follow
../deps/hiredis/libhiredis.a(read.o):(.ARM.exidx+0x18): undefined reference to 
`__aeabi_unwind_cpp_pr0'
cc: error: linker command failed with exit code 1 (use -v to see invocation)
gmake[1]: *** [Makefile:204: redis-server] Error 1
gmake[1]: Leaving directory '/usr/obj/pobj/redis-4.0.8/redis-4.0.8/src'
gmake: *** [Makefile:6: all] Error 2
*** Error 2 in . (/usr/ports/infrastructure/mk/bsd.port.mk:2740 
'/usr/obj/pobj/redis-4.0.8/.build_done')
*** Error 1 in /usr/ports/databases/redis 
(/usr/ports/infrastructure/mk/bsd.port.mk:2417 'all')
cubox /usr/obj/pobj/redis-4.0.8/redis-4.0.8$
-->8--

Those functions are referenced because of the use of -funwind-tables on
arm platforms, see src/Makefile:

--8<--
# To get ARM stack traces if Redis crashes we need a special C flag.
ifneq (,$(findstring armv,$(uname_M)))
CFLAGS+=-funwind-tables
endif
-->8--

Maybe clang ought to link with libunwind if -funwind-tables is used, but
we don't provide a separate libunwind library.  Maybe Mark (cc'd) has
thoughts to share here.

I can confirm that Markus' diff fixes the build.  Another solution would
be to remove the use of -funwind-tables (patch proposal at the end of
this mail, no bump needed).  IIUC it's been deemed useful because of
backtrace support in the bundled jemalloc (which we forcibly remove).

> Have you tried the newer
> 4.0.9 version?

*I* haven't, but the snippet above is still in the master branch
upstream, so I wouldn't expect an improvement.

> I don't have access to that platform but the diff looks ok port-wise.
>
> There's another redis diff from me pending on ports@. but I'd also like
> to update the port to 4.0.9, maybe we can combine this with your fix?

Personally I prefer when different issues are addressed by separate
commits.


Index: patch-src_Makefile
===
RCS file: /d/cvs/ports/databases/redis/patches/patch-src_Makefile,v
retrieving revision 1.27
diff -u -p -p -u -r1.27 patch-src_Makefile
--- patch-src_Makefile  9 Aug 2017 09:16:09 -   1.27
+++ patch-src_Makefile  14 Apr 2018 00:27:57 -
@@ -21,6 +21,15 @@ Index: src/Makefile
  INSTALL=install
  
  # Default allocator defaults to Jemalloc if it's not an ARM
+@@ -40,7 +41,7 @@ endif
+ 
+ # To get ARM stack traces if Redis crashes we need a special C flag.
+ ifneq (,$(findstring armv,$(uname_M)))
+-CFLAGS+=-funwind-tables
++#CFLAGS+=-funwind-tables
+ endif
+ 
+ # Backwards compatibility for selecting an allocator
 @@ -66,7 +67,7 @@ endif
  FINAL_CFLAGS=$(STD) $(WARN) $(OPT) $(DEBUG) $(CFLAGS) $(REDIS_CFLAGS)
  FINAL_LDFLAGS=$(LDFLAGS) $(REDIS_LDFLAGS) $(DEBUG)

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Re: [armv7 fix] Build database/redis

2018-04-12 Thread Klemens Nanni

On Sat, Apr 07, 2018 at 08:26:10AM +0200, Markus Hennecke wrote:
> With the attached fix the redis port builds, but is unable to complete the 
> regression tests (see https://github.com/antirez/redis/issues/4640).
What's the status on armv7 without this fix? Have you tried the newer
4.0.9 version?

I don't have access to that platform but the diff looks ok port-wise.

There's another redis diff from me pending on ports@. but I'd also like
to update the port to 4.0.9, maybe we can combine this with your fix?

[armv7 fix] Build database/redis

2018-04-07 Thread Markus Hennecke

With the attached fix the redis port builds, but is unable to complete the 
regression tests (see https://github.com/antirez/redis/issues/4640).

Index: Makefile
===
RCS file: /cvs/ports/databases/redis/Makefile,v
retrieving revision 1.101
diff -u -p -r1.101 Makefile
--- Makefile19 Mar 2018 18:15:57 -  1.101
+++ Makefile6 Apr 2018 13:12:40 -
@@ -2,6 +2,7 @@
 
 COMMENT =  persistent key-value database
 DISTNAME = redis-4.0.8
+REVISION = 0
 CATEGORIES =   databases
 HOMEPAGE = https://redis.io/
 
Index: patches/patch-src_Makefile
===
RCS file: /cvs/ports/databases/redis/patches/patch-src_Makefile,v
retrieving revision 1.27
diff -u -p -r1.27 patch-src_Makefile
--- patches/patch-src_Makefile  9 Aug 2017 09:16:09 -   1.27
+++ patches/patch-src_Makefile  6 Apr 2018 13:12:40 -
@@ -21,7 +21,18 @@ Index: src/Makefile
  INSTALL=install
  
  # Default allocator defaults to Jemalloc if it's not an ARM
-@@ -66,7 +67,7 @@ endif
+@@ -38,6 +39,10 @@ endif
+ endif
+ endif
+ 
++ifeq ($(uname_M),armv7)
++LDFLAGS+=-lc++abi
++endif
++
+ # To get ARM stack traces if Redis crashes we need a special C flag.
+ ifneq (,$(findstring armv,$(uname_M)))
+ CFLAGS+=-funwind-tables
+@@ -66,7 +71,7 @@ endif
  FINAL_CFLAGS=$(STD) $(WARN) $(OPT) $(DEBUG) $(CFLAGS) $(REDIS_CFLAGS)
  FINAL_LDFLAGS=$(LDFLAGS) $(REDIS_LDFLAGS) $(DEBUG)
  FINAL_LIBS=-lm
@@ -30,7 +41,7 @@ Index: src/Makefile
  
  ifeq ($(uname_S),SunOS)
# SunOS
-@@ -107,7 +108,7 @@ endif
+@@ -107,7 +112,7 @@ endif
  endif
  endif
  # Include paths to dependencies
@@ -39,7 +50,7 @@ Index: src/Makefile
  
  ifeq ($(MALLOC),tcmalloc)
FINAL_CFLAGS+= -DUSE_TCMALLOC
-@@ -129,6 +130,10 @@ REDIS_CC=$(QUIET_CC)$(CC) $(FINAL_CFLAGS)
+@@ -129,6 +134,10 @@ REDIS_CC=$(QUIET_CC)$(CC) $(FINAL_CFLAGS)
  REDIS_LD=$(QUIET_LINK)$(CC) $(FINAL_LDFLAGS)
  REDIS_INSTALL=$(QUIET_INSTALL)$(INSTALL)
  
@@ -50,7 +61,7 @@ Index: src/Makefile
  CCCOLOR="\033[34m"
  LINKCOLOR="\033[34;1m"
  SRCCOLOR="\033[33m"
-@@ -144,7 +149,7 @@ endif
+@@ -144,7 +153,7 @@ endif
  
  REDIS_SERVER_NAME=redis-server
  REDIS_SENTINEL_NAME=redis-sentinel
@@ -59,7 +70,7 @@ Index: src/Makefile
  REDIS_CLI_NAME=redis-cli
  REDIS_CLI_OBJ=anet.o adlist.o redis-cli.o zmalloc.o release.o anet.o ae.o 
crc64.o
  REDIS_BENCHMARK_NAME=redis-benchmark
-@@ -196,7 +201,7 @@ endif
+@@ -196,7 +205,7 @@ endif
  
  # redis-server
  $(REDIS_SERVER_NAME): $(REDIS_SERVER_OBJ)
@@ -68,7 +79,7 @@ Index: src/Makefile
  
  # redis-sentinel
  $(REDIS_SENTINEL_NAME): $(REDIS_SERVER_NAME)
-@@ -239,7 +244,7 @@ distclean: clean
+@@ -239,7 +248,7 @@ distclean: clean
  .PHONY: distclean
  
  test: $(REDIS_SERVER_NAME) $(REDIS_CHECK_AOF_NAME)
@@ -77,7 +88,7 @@ Index: src/Makefile
  
  test-sentinel: $(REDIS_SENTINEL_NAME)
@(cd ..; ./runtest-sentinel)
-@@ -283,10 +288,11 @@ src/help.h:
+@@ -283,10 +292,11 @@ src/help.h:
@../utils/generate-command-help.rb > help.h
  
  install: all

Re: [armv7 fix] Build database/redis

Re: [armv7 fix] Build database/redis

Re: [armv7 fix] Build database/redis

Re: [armv7 fix] Build database/redis

Re: [armv7 fix] Build database/redis

[armv7 fix] Build database/redis

6 matches

Site Navigation

Mail list logo

Footer information