estraier / tkrzw

a set of implementations of DBM
Apache License 2.0
164 stars 20 forks source link

Alpine build name clash for PAGE_SIZE #9

Closed mirlos-pt closed 3 years ago

mirlos-pt commented 3 years ago

There is a build failure when compiling on an alpine (python, but that's not relevant) image.

To reproduce, prepare an image for interactive use:

mkdir tkrzw_build ; cd tkrzw_build
cat >Dockerfile <<END
FROM python:3.8-alpine
RUN apk add --update git g++ make python-dev gtest-dev &&\
    git clone --depth 1 https://github.com/estraier/tkrzw.git
END
sudo docker build -t tkrzw:pre .
sudo docker run -it --ulimit memlock=$((512*1024)):$((1024*1024)) --name tkrzw-builder tkrzw:pre /bin/sh

Inside the container:

/ # ulimit -l
512
/ # cd tkrzw
/tkrzw # ./configure
#================================================================
# Configuring Tkrzw version 0.9.3.
#================================================================
checking for gcc... gcc
checking whether the C compiler works... yes
checking for C compiler default output file name... a.out
checking for suffix of executables... 
checking whether we are cross compiling... no
checking for suffix of object files... o
checking whether we are using the GNU C compiler... yes
checking whether gcc accepts -g... yes
checking for gcc option to accept ISO C89... none needed
checking for g++... g++
checking whether we are using the GNU C++ compiler... yes
checking whether g++ accepts -g... yes
checking how to run the C++ preprocessor... g++ -E
checking for grep that handles long lines and -e... /bin/grep
checking for egrep... /bin/grep -E
checking for ANSI C header files... yes
checking for sys/types.h... yes
checking for sys/stat.h... yes
checking for stdlib.h... yes
checking for string.h... yes
checking for memory.h... yes
checking for strings.h... yes
checking for memory.h... yes
checking for strings.h... yes
checking for inttypes.h... yes
checking for stdint.h... yes
checking for unistd.h... yes
checking whether byte ordering is bigendian... no
checking for main in -lc... yes
checking for main in -lm... yes
checking for main in -lpthread... yes
checking for main in -lrt... yes
checking for main in -lstdc++... yes
checking for main in -ltkrzw... no
checking for stdlib.h... (cached) yes
checking for stdint.h... (cached) yes
checking for unistd.h... (cached) yes
checking fcntl.h usability... yes
checking fcntl.h presence... yes
checking for fcntl.h... yes
checking dirent.h usability... yes
checking dirent.h presence... yes
checking for dirent.h... yes
checking pthread.h usability... yes
checking pthread.h presence... yes
checking for pthread.h... yes
checking utility usability... yes
checking utility presence... yes
checking for utility... yes
checking string usability... yes
checking string presence... yes
checking for string... yes
checking atomic usability... yes
checking atomic presence... yes
checking for atomic... yes
checking mutex usability... yes
checking mutex presence... yes
checking for mutex... yes
checking thread usability... yes
checking thread presence... yes
checking for thread... yes
checking regex usability... yes
checking regex presence... yes
checking for regex... yes
checking cstdint usability... yes
checking cstdint presence... yes
checking for cstdint... yes
configure: creating ./config.status
config.status: creating Makefile
config.status: creating tkrzw.pc
#================================================================
# Ready to make.
#================================================================
/tkrzw # make
g++ -c -I. -I/usr/local/include -DNDEBUG -D_GNU_SOURCE=1 -D_ISOC99_SOURCE=1 -D_POSIX_C_SOURCE=200809L -D_FILE_OFFSET_BITS=64 -D_REENTRANT -D__EXTENSIONS__ -D_TKRZW_PREFIX="\"/usr/local\"" -D_TKRZW_INCLUDEDIR="\"/usr/local/include\"" -D_TKRZW_LIBDIR="\"/usr/local/lib\"" -D_TKRZW_BINDIR="\"/usr/local/bin\"" -D_TKRZW_LIBEXECDIR="\"/usr/local/libexec\"" -D_TKRZW_APPINC="\"-I/usr/local/include\"" -D_TKRZW_APPLIBS="\"-L/usr/local/lib -ltkrzw -lstdc++ -lrt -lpthread -lm -lc \"" -D_TKRZW_PKG_VERSION="\"0.9.3\"" -D_TKRZW_LIB_VERSION="\"0.3.0\"" -MMD -g -O2 -std=c++17 -pthread -fPIC -fsigned-char tkrzw_lib_common.cc
In file included from /usr/include/limits.h:40,
                 from /usr/include/sys/param.h:33,
                 from tkrzw_sys_config.h:126,
                 from tkrzw_lib_common.cc:15:
tkrzw_lib_common.cc:19:15: error: expected unqualified-id before numeric constant
   19 | const int32_t PAGE_SIZE = sysconf(_SC_PAGESIZE);
      |               ^~~~~~~~~
make: *** [Makefile:560: tkrzw_lib_common.o] Error 1

The build proceeds, and make test check succeeds, after:

/tkrzw # sed -i -e 's/\<PAGE_SIZE\>/TKRZW_PAGE_SIZE/g' *.cc *.h
estraier commented 3 years ago

Thanks for catching it. As tkrzw::PAGE_SIZE is an external name, it a part of the public API. So, I want to keep the name if possible. Do you know where the PAGE_SIZE macro comes from?

estraier commented 3 years ago

Maybe, the underlying libc library "musl" is the cause. https://git.musl-libc.org/cgit/musl/tree/arch/x86_64/bits/limits.h

Adding -DNPAGE_SIZE to CPPFLAGS in Makefile could solve this issue. Could you try it?

mirlos-pt commented 3 years ago

It comes from the musl C library indeed.

/usr/include # grep -C 5 PAGE_SIZE limits.h
#endif

#if defined(_GNU_SOURCE) || defined(_BSD_SOURCE) || defined(_XOPEN_SOURCE)

#ifdef PAGESIZE
#define PAGE_SIZE PAGESIZE
#endif
#define NZERO 20
#define NL_LANGMAX 32

#endif
/usr/include # apk info --who-owns limits.h 
/usr/include/limits.h is owned by musl-dev-1.2.2-r0

The CPPFLAGS change is ineffective:

/tkrzw # make
g++ -c -DNPAGE_SIZE -I. -I/usr/local/include -DNDEBUG -D_GNU_SOURCE=1 -D_ISOC99_SOURCE=1 -D_POSIX_C_SOURCE=200809L -D_FILE_OFFSET_BITS=64 -D_REENTRANT -D__EXTENSIONS__ -D_TKRZW_PREFIX
="\"/usr/local\"" -D_TKRZW_INCLUDEDIR="\"/usr/local/include\"" -D_TKRZW_LIBDIR="\"/usr/local/lib\"" -D_TKRZW_BINDIR="\"/usr/local/bin\"" -D_TKRZW_LIBEXECDIR="\"/usr/local/libexec\"" -
D_TKRZW_APPINC="\"-I/usr/local/include\"" -D_TKRZW_APPLIBS="\"-L/usr/local/lib -ltkrzw -lstdc++ -lrt -lpthread -lm -lc \"" -D_TKRZW_PKG_VERSION="\"0.9.3\"" -D_TKRZW_LIB_VERSION="\"0.3
.0\"" -MMD -g -O2 -std=c++17 -pthread -fPIC -fsigned-char tkrzw_lib_common.cc
In file included from /usr/include/limits.h:40,
                 from /usr/include/sys/param.h:33,
                 from tkrzw_sys_config.h:126,
                 from tkrzw_lib_common.cc:15:
tkrzw_lib_common.cc:19:15: error: expected unqualified-id before numeric constant
   19 | const int32_t PAGE_SIZE = sysconf(_SC_PAGESIZE);
      |               ^~~~~~~~~
make: *** [Makefile:560: tkrzw_lib_common.o] Error 1

Undefining in the header above makes it compile too. If clients don't include the header, then that might be an ok solution.

diff --git a/tkrzw_sys_config.h b/tkrzw_sys_config.h
index dbde3a4..2bd5121 100644
--- a/tkrzw_sys_config.h
+++ b/tkrzw_sys_config.h
@@ -133,6 +133,11 @@ extern "C" {
 #include <sys/resource.h>
 #include <fcntl.h>
 #include <dirent.h>
+
+#ifdef PAGE_SIZE
+#undef PAGE_SIZE
+#endif
+
 }  // extern "C"

 namespace tkrzw {
estraier commented 3 years ago

I see. Then, I'll undef macros which can conflicts the constants defined in tkrzw_sys_config.h.

estraier commented 3 years ago

I committed the update.