On Thu, 6 Feb 2020 16:12:29 +0000
Stuart Henderson wrote:

> On 2020/02/06 10:15, Daniel Jakots wrote:
> > Hi Theo,
> > 
> > Thanks for working on that!
> > 
> > On Thu, 6 Feb 2020 15:21:23 +0100, Theo Buehler <t...@theobuehler.org>
> > wrote:
> > 
> > > First of all, despite the length of this mail, most of the update
> > > is straightforward. The diff is an in-place upgrade from 4.0.14
> > > to 5.0.7.
> 
> When the topic came up before, there was some mention of 5.x not
> working on some arches, I would like to see this tested on at least
> i386, sparc64, aarch64 before it goes in. I can do some tests on i386
> and probably also aarch64 but not sparc64.

I've spotted an occurrence of char being considered as signed on all
arch, which is not the case on arm* and powerpc. Theo found the
issue is there on our current version in CVS, so it won't break builds,
but it should be corrected.

Below is the original diff with an additional patch to fix that. I did
run the tests and it's still fine.

> > > The release notes (which contain migration instructions at the
> > > very end)
> > > 
> > > https://raw.githubusercontent.com/antirez/redis/5.0/00-RELEASENOTES
> > 
> > Since Redis 5.0 is still able to read 4.0 mdb format, I assume for a
> > user there is nothing to do. I guess you don't plan to add anything
> > to current.html do you?
> > 
> > > look as if there is no risk of losing data due to the migration
> > > and looking around on the net I couldn't find any reports of
> > > breakage. However, I don't know and can't know for sure.
> > > 
> > > Since we're playing with user data here, it might be more prudent
> > > to provide a redis5 port, and to leave it to the users to decide
> > > if and when they want to migrate instead of forcing it upon them
> > > right when the update goes in (or when they upgrade to 6.7).
> > 
> > AFAIK, redis is (or should be) used as a cache i.e. losing its data
> > is a small inconvenience but shouldn't be a problem.
> 
> noooooo, this would be a big problem....
> 
> >                                                   I don't think
> > duplicating the port is worth it.
> 
> I do agree with that.
> 
> > > A detail: redis-sentinel is installed as a symlink to
> > > redis-server. Ports seem to do this frequently, but I wonder if
> > > that should be turned into a hard link as is usually done in base?
> > 
> > What are the pro/cons of using a hard link instead?
> 
> Not sure about pros, but here's a con: you need to check inode numbers
> to see for sure where they point (I hate this with the hardlinked
> files in base...)
> 


Index: Makefile
===================================================================
RCS file: /cvs/ports/databases/redis/Makefile,v
retrieving revision 1.108
diff -u -p -u -p -r1.108 Makefile
--- Makefile    12 Jul 2019 20:44:01 -0000      1.108
+++ Makefile    6 Feb 2020 17:21:51 -0000
@@ -1,10 +1,9 @@
 # $OpenBSD: Makefile,v 1.108 2019/07/12 20:44:01 sthen Exp $
 
 COMMENT =              persistent key-value database
-DISTNAME =             redis-4.0.14
+DISTNAME =             redis-5.0.7
 CATEGORIES =           databases
 HOMEPAGE =             https://redis.io/
-REVISION =             0
 
 # BSD
 PERMIT_PACKAGE =       Yes
Index: distinfo
===================================================================
RCS file: /cvs/ports/databases/redis/distinfo,v
retrieving revision 1.83
diff -u -p -u -p -r1.83 distinfo
--- distinfo    2 Apr 2019 22:13:11 -0000       1.83
+++ distinfo    6 Feb 2020 17:21:51 -0000
@@ -1,2 +1,2 @@
-SHA256 (redis-4.0.14.tar.gz) = Hh4YQgqGz7KFkzEjsEqC4evaIL+woolHJ0Wgh1h+k6c=
-SIZE (redis-4.0.14.tar.gz) = 1740967
+SHA256 (redis-5.0.7.tar.gz) = Ydt06r9oAfBX/SS1kCMvLzN9QiKA/RlIbsoDvofTqCs=
+SIZE (redis-5.0.7.tar.gz) = 1984203
Index: patches/patch-deps_Makefile
===================================================================
RCS file: /cvs/ports/databases/redis/patches/patch-deps_Makefile,v
retrieving revision 1.10
diff -u -p -u -p -r1.10 patch-deps_Makefile
--- patches/patch-deps_Makefile 9 Aug 2017 09:16:09 -0000       1.10
+++ patches/patch-deps_Makefile 6 Feb 2020 17:21:51 -0000
@@ -48,7 +48,7 @@ Index: deps/Makefile
 -
 -jemalloc: .make-prerequisites
 -      @printf '%b %b\n' $(MAKECOLOR)MAKE$(ENDCOLOR) $(BINCOLOR)$@$(ENDCOLOR)
--      cd jemalloc && ./configure --with-lg-quantum=3 
--with-jemalloc-prefix=je_ --enable-cc-silence CFLAGS="$(JEMALLOC_CFLAGS)" 
LDFLAGS="$(JEMALLOC_LDFLAGS)"
+-      cd jemalloc && ./configure --with-version=5.1.0-0-g0 
--with-lg-quantum=3 --with-jemalloc-prefix=je_ --enable-cc-silence 
CFLAGS="$(JEMALLOC_CFLAGS)" LDFLAGS="$(JEMALLOC_LDFLAGS)"
 -      cd jemalloc && $(MAKE) CFLAGS="$(JEMALLOC_CFLAGS)" 
LDFLAGS="$(JEMALLOC_LDFLAGS)" lib/libjemalloc.a
 -
 -.PHONY: jemalloc
Index: patches/patch-deps_linenoise_linenoise_c
===================================================================
RCS file: patches/patch-deps_linenoise_linenoise_c
diff -N patches/patch-deps_linenoise_linenoise_c
--- /dev/null   1 Jan 1970 00:00:00 -0000
+++ patches/patch-deps_linenoise_linenoise_c    6 Feb 2020 17:21:51 -0000
@@ -0,0 +1,18 @@
+$OpenBSD$
+
+Fix an occurrence where char is considered signed on all platforms,
+it's not the case on powerpc and arm.
+
+Index: deps/linenoise/linenoise.c
+--- deps/linenoise/linenoise.c.orig
++++ deps/linenoise/linenoise.c
+@@ -793,7 +793,7 @@ static int linenoiseEdit(int stdin_fd, int stdout_fd, 
+ 
+     if (write(l.ofd,prompt,l.plen) == -1) return -1;
+     while(1) {
+-        char c;
++        signed char c;
+         int nread;
+         char seq[3];
+ 
+
Index: patches/patch-redis_conf
===================================================================
RCS file: /cvs/ports/databases/redis/patches/patch-redis_conf,v
retrieving revision 1.20
diff -u -p -u -p -r1.20 patch-redis_conf
--- patches/patch-redis_conf    9 Aug 2017 09:16:09 -0000       1.20
+++ patches/patch-redis_conf    6 Feb 2020 17:21:51 -0000
@@ -72,7 +72,7 @@ Index: redis.conf
  
  ################################# REPLICATION 
#################################
  
-@@ -497,7 +499,7 @@ slave-priority 100
+@@ -504,7 +506,7 @@ replica-priority 100
  # 150k passwords per second against a good box. This means that you should
  # use a very strong password otherwise it will be very easy to break.
  #
@@ -81,7 +81,7 @@ Index: redis.conf
  
  # Command renaming.
  #
-@@ -530,6 +532,7 @@ slave-priority 100
+@@ -537,6 +539,7 @@ replica-priority 100
  # an error 'max number of clients reached'.
  #
  # maxclients 10000
Index: patches/patch-sentinel_conf
===================================================================
RCS file: /cvs/ports/databases/redis/patches/patch-sentinel_conf,v
retrieving revision 1.6
diff -u -p -u -p -r1.6 patch-sentinel_conf
--- patches/patch-sentinel_conf 2 Sep 2018 11:08:51 -0000       1.6
+++ patches/patch-sentinel_conf 6 Feb 2020 17:21:51 -0000
@@ -2,7 +2,7 @@ $OpenBSD: patch-sentinel_conf,v 1.6 2018
 Index: sentinel.conf
 --- sentinel.conf.orig
 +++ sentinel.conf
-@@ -167,7 +167,7 @@ sentinel failover-timeout mymaster 180000
+@@ -182,7 +182,7 @@ sentinel failover-timeout mymaster 180000
  #
  # Example:
  #
@@ -11,7 +11,7 @@ Index: sentinel.conf
  
  # CLIENTS RECONFIGURATION SCRIPT
  #
-@@ -192,7 +192,7 @@ sentinel failover-timeout mymaster 180000
+@@ -207,7 +207,7 @@ sentinel failover-timeout mymaster 180000
  #
  # Example:
  #
Index: patches/patch-src_Makefile
===================================================================
RCS file: /cvs/ports/databases/redis/patches/patch-src_Makefile,v
retrieving revision 1.29
diff -u -p -u -p -r1.29 patch-src_Makefile
--- patches/patch-src_Makefile  24 Apr 2018 14:35:29 -0000      1.29
+++ patches/patch-src_Makefile  6 Feb 2020 17:21:51 -0000
@@ -11,7 +11,7 @@ Index: src/Makefile
  NODEPS:=clean distclean
  
  # Default settings
-@@ -26,6 +26,7 @@ OPT=$(OPTIMIZATION)
+@@ -31,6 +31,7 @@ OPT=$(OPTIMIZATION)
  
  PREFIX?=/usr/local
  INSTALL_BIN=$(PREFIX)/bin
@@ -19,16 +19,16 @@ 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.
+@@ -48,7 +49,7 @@ ifneq (,$(filter aarch64 armv,$(uname_M)))
+         CFLAGS+=-funwind-tables
+ else
  ifneq (,$(findstring armv,$(uname_M)))
 -        CFLAGS+=-funwind-tables
 +#        CFLAGS+=-funwind-tables
  endif
+ endif
  
- # Backwards compatibility for selecting an allocator
-@@ -107,7 +108,7 @@ endif
+@@ -127,7 +128,7 @@ endif
  endif
  endif
  # Include paths to dependencies
@@ -37,15 +37,15 @@ Index: src/Makefile
  
  ifeq ($(MALLOC),tcmalloc)
        FINAL_CFLAGS+= -DUSE_TCMALLOC
-@@ -145,6 +146,7 @@ endif
+@@ -165,6 +166,7 @@ endif
  REDIS_SERVER_NAME=redis-server
  REDIS_SENTINEL_NAME=redis-sentinel
- REDIS_SERVER_OBJ=adlist.o quicklist.o ae.o anet.o dict.o server.o sds.o 
zmalloc.o lzf_c.o lzf_d.o pqsort.o zipmap.o sha1.o ziplist.o release.o 
networking.o util.o object.o db.o replication.o rdb.o t_string.o t_list.o 
t_set.o t_zset.o t_hash.o config.o aof.o pubsub.o multi.o debug.o sort.o 
intset.o syncio.o cluster.o crc16.o endianconv.o slowlog.o scripting.o bio.o 
rio.o rand.o memtest.o crc64.o bitops.o sentinel.o notify.o setproctitle.o 
blocked.o hyperloglog.o latency.o sparkline.o redis-check-rdb.o 
redis-check-aof.o geo.o lazyfree.o module.o evict.o expire.o geohash.o 
geohash_helper.o childinfo.o defrag.o siphash.o rax.o
+ REDIS_SERVER_OBJ=adlist.o quicklist.o ae.o anet.o dict.o server.o sds.o 
zmalloc.o lzf_c.o lzf_d.o pqsort.o zipmap.o sha1.o ziplist.o release.o 
networking.o util.o object.o db.o replication.o rdb.o t_string.o t_list.o 
t_set.o t_zset.o t_hash.o config.o aof.o pubsub.o multi.o debug.o sort.o 
intset.o syncio.o cluster.o crc16.o endianconv.o slowlog.o scripting.o bio.o 
rio.o rand.o memtest.o crc64.o bitops.o sentinel.o notify.o setproctitle.o 
blocked.o hyperloglog.o latency.o sparkline.o redis-check-rdb.o 
redis-check-aof.o geo.o lazyfree.o module.o evict.o expire.o geohash.o 
geohash_helper.o childinfo.o defrag.o siphash.o rax.o t_stream.o listpack.o 
localtime.o lolwut.o lolwut5.o
 +REDIS_SERVER_OBJ+=fpconv.o strbuf.o lua_bit.o lua_cjson.o lua_cmsgpack.o 
lua_struct.o
  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_CLI_OBJ=anet.o adlist.o dict.o redis-cli.o zmalloc.o release.o anet.o 
ae.o crc64.o siphash.o crc16.o
  REDIS_BENCHMARK_NAME=redis-benchmark
-@@ -196,7 +198,7 @@ endif
+@@ -216,7 +218,7 @@ endif
  
  # redis-server
  $(REDIS_SERVER_NAME): $(REDIS_SERVER_OBJ)
@@ -54,7 +54,7 @@ Index: src/Makefile
  
  # redis-sentinel
  $(REDIS_SENTINEL_NAME): $(REDIS_SERVER_NAME)
-@@ -239,7 +241,7 @@ distclean: clean
+@@ -259,7 +261,7 @@ distclean: clean
  .PHONY: distclean
  
  test: $(REDIS_SERVER_NAME) $(REDIS_CHECK_AOF_NAME)
@@ -63,7 +63,7 @@ Index: src/Makefile
  
  test-sentinel: $(REDIS_SENTINEL_NAME)
        @(cd ..; ./runtest-sentinel)
-@@ -283,10 +285,11 @@ src/help.h:
+@@ -303,13 +305,14 @@ src/help.h:
        @../utils/generate-command-help.rb > help.h
  
  install: all
@@ -78,3 +78,6 @@ Index: src/Makefile
        $(REDIS_INSTALL) $(REDIS_CHECK_AOF_NAME) $(INSTALL_BIN)
 -      @ln -sf $(REDIS_SERVER_NAME) $(INSTALL_BIN)/$(REDIS_SENTINEL_NAME)
 +      @ln -sf $(REDIS_SERVER_NAME) $(INSTALL_SBIN)/$(REDIS_SENTINEL_NAME)
+ 
+ uninstall:
+       rm -f 
$(INSTALL_BIN)/{$(REDIS_SERVER_NAME),$(REDIS_BENCHMARK_NAME),$(REDIS_CLI_NAME),$(REDIS_CHECK_RDB_NAME),$(REDIS_CHECK_AOF_NAME),$(REDIS_SENTINEL_NAME)}
Index: patches/patch-src_server_h
===================================================================
RCS file: /cvs/ports/databases/redis/patches/patch-src_server_h,v
retrieving revision 1.3
diff -u -p -u -p -r1.3 patch-src_server_h
--- patches/patch-src_server_h  9 Aug 2017 09:16:09 -0000       1.3
+++ patches/patch-src_server_h  6 Feb 2020 17:21:51 -0000
@@ -2,7 +2,7 @@ $OpenBSD: patch-src_server_h,v 1.3 2017/
 Index: src/server.h
 --- src/server.h.orig
 +++ src/server.h
-@@ -98,7 +98,7 @@ typedef long long mstime_t; /* millisecond time type. 
+@@ -101,7 +101,7 @@ typedef long long ustime_t; /* microsecond time type. 
  #define AOF_READ_DIFF_INTERVAL_BYTES (1024*10)
  #define CONFIG_DEFAULT_SLOWLOG_LOG_SLOWER_THAN 10000
  #define CONFIG_DEFAULT_SLOWLOG_MAX_LEN 128
@@ -11,7 +11,7 @@ Index: src/server.h
  #define CONFIG_AUTHPASS_MAX_LEN 512
  #define CONFIG_DEFAULT_SLAVE_PRIORITY 100
  #define CONFIG_DEFAULT_REPL_TIMEOUT 60
-@@ -109,7 +109,7 @@ typedef long long mstime_t; /* millisecond time type. 
+@@ -112,7 +112,7 @@ typedef long long ustime_t; /* microsecond time type. 
  #define CONFIG_DEFAULT_REPL_BACKLOG_TIME_LIMIT (60*60)  /* 1 hour */
  #define CONFIG_REPL_BACKLOG_MIN_SIZE (1024*16)          /* 16k */
  #define CONFIG_BGSAVE_RETRY_DELAY 5 /* Wait a few secs before trying again. */
Index: patches/patch-src_zmalloc_c
===================================================================
RCS file: patches/patch-src_zmalloc_c
diff -N patches/patch-src_zmalloc_c
--- /dev/null   1 Jan 1970 00:00:00 -0000
+++ patches/patch-src_zmalloc_c 6 Feb 2020 17:21:51 -0000
@@ -0,0 +1,61 @@
+$OpenBSD$
+
+zmalloc_size() returns a lie based on the following assumption:
+
+    /* Assume at least that all the allocations are padded at sizeof(long) by
+     * the underlying allocator. */
+
+Make that lie a reality by rounding up the actually allocated sizes.
+
+Index: src/zmalloc.c
+--- src/zmalloc.c.orig
++++ src/zmalloc.c
+@@ -56,6 +56,13 @@ void zlibc_free(void *ptr) {
+ #endif
+ #endif
+ 
++static size_t zmalloc_roundsize(size_t size) {
++#ifndef HAVE_MALLOC_SIZE
++     if (size&(sizeof(long)-1)) size += sizeof(long)-(size&(sizeof(long)-1));
++#endif
++     return size;
++}
++
+ /* Explicitly override malloc/free etc when using tcmalloc. */
+ #if defined(USE_TCMALLOC)
+ #define malloc(size) tc_malloc(size)
+@@ -96,8 +103,11 @@ static void zmalloc_default_oom(size_t size) {
+ static void (*zmalloc_oom_handler)(size_t) = zmalloc_default_oom;
+ 
+ void *zmalloc(size_t size) {
+-    void *ptr = malloc(size+PREFIX_SIZE);
++    void *ptr;
+ 
++    size = zmalloc_roundsize(size);
++    ptr = malloc(size+PREFIX_SIZE);
++
+     if (!ptr) zmalloc_oom_handler(size);
+ #ifdef HAVE_MALLOC_SIZE
+     update_zmalloc_stat_alloc(zmalloc_size(ptr));
+@@ -128,8 +138,11 @@ void zfree_no_tcache(void *ptr) {
+ #endif
+ 
+ void *zcalloc(size_t size) {
+-    void *ptr = calloc(1, size+PREFIX_SIZE);
++    void *ptr;
+ 
++    size = zmalloc_roundsize(size);
++    ptr = calloc(1, size+PREFIX_SIZE);
++
+     if (!ptr) zmalloc_oom_handler(size);
+ #ifdef HAVE_MALLOC_SIZE
+     update_zmalloc_stat_alloc(zmalloc_size(ptr));
+@@ -147,6 +160,8 @@ void *zrealloc(void *ptr, size_t size) {
+ #endif
+     size_t oldsize;
+     void *newptr;
++
++    size = zmalloc_roundsize(size);
+ 
+     if (ptr == NULL) return zmalloc(size);
+ #ifdef HAVE_MALLOC_SIZE

Reply via email to