lichray / nvi2

A multibyte fork of the nvi editor for BSD
Other
144 stars 34 forks source link

Build fails on some macOS versions does to wrong assumption on sysroot and possibly missing `SLIST_REMOVE_AFTER` #133

Open barracuda156 opened 4 months ago

barracuda156 commented 4 months ago

@lichray There was an earlier issue related to this: https://github.com/lichray/nvi2/issues/69 However there are remaining problems. The first is that ${CMAKE_OSX_SYSROOT}/usr/include/db.h expands to //usr/include/db.h (notice double slash), when it is actually in / and not inside Xcode app, and this breaks include:

[ 15%] Building C object CMakeFiles/nvi.dir/cl/cl_funcs.c.o
/opt/local/bin/gcc-mp-13 -DDB_H_ABS_PATH="<//usr/include/db.h>" -D_XOPEN_SOURCE_EXTENDED -D__REGEX_PRIVATE -I/opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_editors_nvi2/nvi2/work/build -I/opt/local/include -I/opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_editors_nvi2/nvi2/work/nvi2-2.2.1/regex -pipe -Os -DNDEBUG -isystem/opt/local/include/LegacySupport -I/opt/local/include -arch ppc -mmacosx-version-min=10.6 -Wno-string-compare -Wstack-protector -fstack-protector -Wstrict-aliasing -fstrict-aliasing -MD -MT CMakeFiles/nvi.dir/cl/cl_funcs.c.o -MF CMakeFiles/nvi.dir/cl/cl_funcs.c.o.d -o CMakeFiles/nvi.dir/cl/cl_funcs.c.o -c /opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_editors_nvi2/nvi2/work/nvi2-2.2.1/cl/cl_funcs.c
In file included from /opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_editors_nvi2/nvi2/work/nvi2-2.2.1/cl/cl_screen.c:28:
/opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_editors_nvi2/nvi2/work/nvi2-2.2.1/cl/../common/common.h:15:23: error: missing terminating > character
   15 | #include DB_H_ABS_PATH
      |                       ^
<command-line>: error: empty filename in #include

Once that is fixed, it fails on:

[ 15%] Building C object CMakeFiles/nvi.dir/cl/cl_read.c.o
/opt/local/bin/gcc-mp-13 -DDB_H_ABS_PATH="</usr/include/db.h>" -D_XOPEN_SOURCE_EXTENDED -D__REGEX_PRIVATE -I/opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_editors_nvi2/nvi2/work/build -I/opt/local/include -I/opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_editors_nvi2/nvi2/work/nvi2-2.2.1/regex -pipe -Os -DNDEBUG -isystem/opt/local/include/LegacySupport -I/opt/local/include -arch ppc -mmacosx-version-min=10.6 -Wno-string-compare -Wstack-protector -fstack-protector -Wstrict-aliasing -fstrict-aliasing -MD -MT CMakeFiles/nvi.dir/cl/cl_read.c.o -MF CMakeFiles/nvi.dir/cl/cl_read.c.o.d -o CMakeFiles/nvi.dir/cl/cl_read.c.o -c /opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_editors_nvi2/nvi2/work/nvi2-2.2.1/cl/cl_read.c
/opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_editors_nvi2/nvi2/work/nvi2-2.2.1/cl/cl_term.c: In function 'cl_term_end':
/opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_editors_nvi2/nvi2/work/nvi2-2.2.1/cl/cl_term.c:195:33: warning: implicit declaration of function 'SLIST_REMOVE_AFTER'; did you mean 'SLIST_REMOVE_HEAD'? [-Wimplicit-function-declaration]
  195 |                                 SLIST_REMOVE_AFTER(pre_qp, q);
      |                                 ^~~~~~~~~~~~~~~~~~
      |                                 SLIST_REMOVE_HEAD
/opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_editors_nvi2/nvi2/work/nvi2-2.2.1/cl/cl_term.c:195:60: error: 'q' undeclared (first use in this function); did you mean 'qp'?
  195 |                                 SLIST_REMOVE_AFTER(pre_qp, q);
      |                                                            ^
      |                                                            qp

This is because sys/queue.h is not guaranteed to have it on macOS.

Apple own code suggests this:

#ifndef SLIST_REMOVE_AFTER
#define SLIST_REMOVE_AFTER(elm, field) do {                             \
        SLIST_NEXT(elm, field) =                                        \
            SLIST_NEXT(SLIST_NEXT(elm, field), field);                  \
} while (0)
#endif

https://opensource.apple.com/source/Libc/Libc-997.1.1/gen/FreeBSD/popen.c.auto.html

Since this is used in more than one file, it can be placed in common.h instead. Then the build succeeds.

barracuda156 commented 4 months ago

In fact, just removing that Apple hack fixes the build :)

barracuda156 commented 4 months ago

Literally, just removing the hack fixes it:

--- CMakeLists.txt  2023-09-25 16:47:42.000000000 +0800
+++ CMakeLists.txt  2024-05-02 04:27:25.000000000 +0800
@@ -209,18 +209,8 @@
 if(NOT DBOPEN_IN_LIBC)
     target_link_libraries(nvi PRIVATE db1)
 endif()
-if (APPLE)
-    # Avoid using an incompatible db.h installed to /usr/local (since this is
-    # part of the default search path on macOS)
-    set(DB_H_GUESS "${CMAKE_OSX_SYSROOT}/usr/include/db.h")
-    if (NOT EXISTS ${DB_H_GUESS})
-        message(FATAL_ERROR "Could not find db.h at the expected path (${DB_H_GUESS}).")
-    endif()
-    add_definitions("-DDB_H_ABS_PATH=<${DB_H_GUESS}>")
-else()
-    find_path(DB_INCLUDE_DIR db.h PATH_SUFFIXES db1)
-    target_include_directories(nvi PRIVATE ${DB_INCLUDE_DIR})
-endif()
+find_path(DB_INCLUDE_DIR db.h PATH_SUFFIXES db1)
+target_include_directories(nvi PRIVATE ${DB_INCLUDE_DIR})

 check_include_files(libutil.h HAVE_LIBUTIL_H)
 check_include_files(ncurses.h HAVE_NCURSES_H)

--- common/common.h 2023-09-25 16:47:42.000000000 +0800
+++ common/common.h 2024-05-02 06:04:46.000000000 +0800
@@ -11,11 +11,7 @@
 #define TCSASOFT 0
 #endif

-#ifdef DB_H_ABS_PATH
-#include DB_H_ABS_PATH
-#else
 #include <db.h>
-#endif
 #include <regex.h>     /* May refer to the bundled regex. */

 /*

Tested on 10.6 and 14.4.1.

lichray commented 4 months ago

It seems you're building using GCC on macOS. If so, CMAKE_OSX_SYSROOT doesn't make sense. We should use the previous fix only when using Xcode toolchain, then.

barracuda156 commented 4 months ago

@lichray On Sonoma with Clang it did not break the build indeed, but it was unneeded. I removed that hack, and nvi2 builds fine on every system with clang: https://ports.macports.org/port/nvi2/details

Log from Sonoma as an example: https://build.macports.org/builders/ports-14_x86_64-builder/builds/37229/steps/install-port/logs/stdio

(gcc build tested locally, no buildbot for it.)