lichray/nvi2

Build failure on macOS

bitigchi opened this issue · 25 comments

Latest master fails to build on macOS.

Steps:

  1. Run cmake .
  2. Run make

Output:

Scanning dependencies of target regex
[  1%] Building C object CMakeFiles/regex.dir/Users/emirsari/Projeler/GitHub/nvi2/regex/regcomp.c.o
[  2%] Building C object CMakeFiles/regex.dir/Users/emirsari/Projeler/GitHub/nvi2/regex/regerror.c.o
[  3%] Building C object CMakeFiles/regex.dir/Users/emirsari/Projeler/GitHub/nvi2/regex/regexec.c.o
[  4%] Building C object CMakeFiles/regex.dir/Users/emirsari/Projeler/GitHub/nvi2/regex/regfree.c.o
[  5%] Linking C static library libregex.a
[  5%] Built target regex
[  6%] Generating /Users/emirsari/Projeler/GitHub/nvi2/ex/version.h
[  6%] Generating /Users/emirsari/Projeler/GitHub/nvi2/cl/extern.h
[  7%] Generating /Users/emirsari/Projeler/GitHub/nvi2/common/extern.h
[  8%] Generating /Users/emirsari/Projeler/GitHub/nvi2/ex/extern.h
[  9%] Generating /Users/emirsari/Projeler/GitHub/nvi2/vi/extern.h
[ 10%] Generating /Users/emirsari/Projeler/GitHub/nvi2/common/options_def.h
[ 10%] Generating /Users/emirsari/Projeler/GitHub/nvi2/ex/ex_def.h
Scanning dependencies of target nvi
[ 11%] Building C object CMakeFiles/nvi.dir/Users/emirsari/Projeler/GitHub/nvi2/cl/cl_funcs.c.o
In file included from /Users/emirsari/Projeler/GitHub/nvi2/cl/cl_funcs.c:32:
/Users/emirsari/Projeler/GitHub/nvi2/cl/../common/common.h:12:10: fatal error: '/usr/include/db.h'
      file not found
#include "/usr/include/db.h"    /* Only include db1. */
         ^~~~~~~~~~~~~~~~~~~
1 error generated.
make[2]: *** [CMakeFiles/nvi.dir/Users/emirsari/Projeler/GitHub/nvi2/cl/cl_funcs.c.o] Error 1
make[1]: *** [CMakeFiles/nvi.dir/all] Error 2
make: *** [all] Error 2

ss

berkeley-db 18.1.32_1 is already installed on my system.

nvi2 only supports db1 (the one comes in several BSDs' libc). Oracle's BDB 18 is not related.

It used to work, https://cmake.org/pipermail/cmake/2017-July/065734.html, but maybe recent versions of OSX deleted it, can you search on your system to see if there is a db.h (It may be moved into SDK folders)?

@lichray, in macOS, it's in /usr/local/include/. I've managed to fix it by changing the path in common.h.

Nevertheless, a couple of more errors:

/Users/emirsari/Projeler/GitHub/nvi2/cl/../common/cut.h:35:2: error: unknown type name 'recno_t'
        recno_t  lno;                   /* 1-N: file line. */
        ^
In file included from /Users/emirsari/Projeler/GitHub/nvi2/cl/cl_funcs.c:32:
In file included from /Users/emirsari/Projeler/GitHub/nvi2/cl/../common/common.h:79:
/Users/emirsari/Projeler/GitHub/nvi2/cl/../common/mark.h:26:2: error: unknown type name 'recno_t'
        recno_t  lno;                   /* Line number. */
        ^
/Users/emirsari/Projeler/GitHub/nvi2/cl/../common/mark.h:32:2: error: unknown type name 'recno_t'
        recno_t  lno;                   /* Line number. */
        ^
In file included from /Users/emirsari/Projeler/GitHub/nvi2/cl/cl_funcs.c:32:
In file included from /Users/emirsari/Projeler/GitHub/nvi2/cl/../common/common.h:81:
/Users/emirsari/Projeler/GitHub/nvi2/cl/../common/../ex/ex.h:71:2: error: unknown type name 'recno_t'
        recno_t start, stop;            /* Start/stop of the range. */
        ^
/Users/emirsari/Projeler/GitHub/nvi2/cl/../common/../ex/ex.h:79:2: error: unknown type name 'recno_t'
        recno_t   if_lno;               /* Associated line number. */
        ^
/Users/emirsari/Projeler/GitHub/nvi2/cl/../common/../ex/ex.h:96:2: error: unknown type name 'recno_t'
        recno_t   range_lno;            /* @/global range: set line number. */
        ^
/Users/emirsari/Projeler/GitHub/nvi2/cl/../common/../ex/ex.h:115:2: error: unknown type name
      'recno_t'
        recno_t   lineno;               /* Command: line number. */
        ^
In file included from /Users/emirsari/Projeler/GitHub/nvi2/cl/cl_funcs.c:32:
In file included from /Users/emirsari/Projeler/GitHub/nvi2/cl/../common/common.h:81:
In file included from /Users/emirsari/Projeler/GitHub/nvi2/cl/../common/../ex/ex.h:233:
/Users/emirsari/Projeler/GitHub/nvi2/cl/../common/../ex/extern.h:44:32: warning: type specifier
      missing, defaults to 'int' [-Wimplicit-int]
int ex_g_insdel(SCR *, lnop_t, recno_t);
                               ^
/Users/emirsari/Projeler/GitHub/nvi2/cl/../common/../ex/extern.h:72:46: error: unknown type name
      'recno_t'
int ex_readfp(SCR *, char *, FILE *, MARK *, recno_t *, int);
                                             ^
/Users/emirsari/Projeler/GitHub/nvi2/cl/../common/../ex/extern.h:78:22: warning: type specifier
      missing, defaults to 'int' [-Wimplicit-int]
int sscr_exec(SCR *, recno_t);
                     ^
/Users/emirsari/Projeler/GitHub/nvi2/cl/../common/../ex/extern.h:115:41: warning: type specifier
      missing, defaults to 'int' [-Wimplicit-int]
void ex_cinit(SCR *, EXCMD *, int, int, recno_t, recno_t, int);
                                        ^
/Users/emirsari/Projeler/GitHub/nvi2/cl/../common/../ex/extern.h:115:50: warning: type specifier
      missing, defaults to 'int' [-Wimplicit-int]
void ex_cinit(SCR *, EXCMD *, int, int, recno_t, recno_t, int);
                                                 ^
/Users/emirsari/Projeler/GitHub/nvi2/cl/../common/../ex/extern.h:115:50: error: redefinition of
      parameter 'recno_t'
/Users/emirsari/Projeler/GitHub/nvi2/cl/../common/../ex/extern.h:115:41: note: previous declaration
      is here
void ex_cinit(SCR *, EXCMD *, int, int, recno_t, recno_t, int);
                                        ^
In file included from /Users/emirsari/Projeler/GitHub/nvi2/cl/cl_funcs.c:32:
In file included from /Users/emirsari/Projeler/GitHub/nvi2/cl/../common/common.h:82:
/Users/emirsari/Projeler/GitHub/nvi2/cl/../common/gs.h:28:2: error: unknown type name 'recno_t'
        recno_t  lno;                   /* 1-N: file cursor line. */
        ^
/Users/emirsari/Projeler/GitHub/nvi2/cl/../common/gs.h:92:2: error: unknown type name 'recno_t'
        recno_t   if_lno;               /* Current associated line number. */
        ^
In file included from /Users/emirsari/Projeler/GitHub/nvi2/cl/cl_funcs.c:32:
In file included from /Users/emirsari/Projeler/GitHub/nvi2/cl/../common/common.h:83:
/Users/emirsari/Projeler/GitHub/nvi2/cl/../common/screen.h:62:2: error: unknown type name 'recno_t'
        recno_t  lno;                   /* 1-N: file line. */
        ^
/Users/emirsari/Projeler/GitHub/nvi2/cl/../common/screen.h:74:2: error: unknown type name 'recno_t'
        recno_t  rptlchange;            /* Ex/vi: last L_CHANGED lno. */
        ^
/Users/emirsari/Projeler/GitHub/nvi2/cl/../common/screen.h:75:2: error: unknown type name 'recno_t'
        recno_t  rptlines[L_YANKED + 1];/* Ex/vi: lines changed by last op. */
        ^
/Users/emirsari/Projeler/GitHub/nvi2/cl/../common/screen.h:81:2: error: unknown type name 'recno_t'
        recno_t  defscroll;             /* Vi: ^D, ^U scroll information. */
        ^
In file included from /Users/emirsari/Projeler/GitHub/nvi2/cl/cl_funcs.c:32:
In file included from /Users/emirsari/Projeler/GitHub/nvi2/cl/../common/common.h:84:
/Users/emirsari/Projeler/GitHub/nvi2/cl/../common/exf.h:24:2: error: unknown type name 'recno_t'
        recno_t  c_lno;                 /* Cached line number. */
        ^
/Users/emirsari/Projeler/GitHub/nvi2/cl/../common/exf.h:25:2: error: unknown type name 'recno_t'
        recno_t  c_nlines;              /* Cached lines in the file. */
        ^
/Users/emirsari/Projeler/GitHub/nvi2/cl/../common/exf.h:30:2: error: unknown type name 'recno_t'
        recno_t  l_high;                /* Log last + 1 record number. */
        ^
/Users/emirsari/Projeler/GitHub/nvi2/cl/../common/exf.h:31:2: error: unknown type name 'recno_t'
        recno_t  l_cur;                 /* Log current record number. */
        ^
In file included from /Users/emirsari/Projeler/GitHub/nvi2/cl/cl_funcs.c:32:
In file included from /Users/emirsari/Projeler/GitHub/nvi2/cl/../common/common.h:88:
/Users/emirsari/Projeler/GitHub/nvi2/cl/../common/extern.h:6:21: warning: type specifier missing,
      defaults to 'int' [-Wimplicit-int]
int cut_line(SCR *, recno_t, size_t, size_t, CB *);
                    ^
/Users/emirsari/Projeler/GitHub/nvi2/cl/../common/extern.h:35:20: warning: type specifier missing,
      defaults to 'int' [-Wimplicit-int]
int db_eget(SCR *, recno_t, CHAR_T **, size_t *, int *);
                   ^
/Users/emirsari/Projeler/GitHub/nvi2/cl/../common/extern.h:36:19: warning: type specifier missing,
      defaults to 'int' [-Wimplicit-int]
int db_get(SCR *, recno_t, u_int32_t, CHAR_T **, size_t *);
                  ^
/Users/emirsari/Projeler/GitHub/nvi2/cl/../common/extern.h:37:22: warning: type specifier missing,
      defaults to 'int' [-Wimplicit-int]
int db_delete(SCR *, recno_t);
                     ^
/Users/emirsari/Projeler/GitHub/nvi2/cl/../common/extern.h:38:27: warning: type specifier missing,
      defaults to 'int' [-Wimplicit-int]
int db_append(SCR *, int, recno_t, CHAR_T *, size_t);
                          ^
/Users/emirsari/Projeler/GitHub/nvi2/cl/../common/extern.h:39:22: warning: type specifier missing,
      defaults to 'int' [-Wimplicit-int]
int db_insert(SCR *, recno_t, CHAR_T *, size_t);
                     ^
/Users/emirsari/Projeler/GitHub/nvi2/cl/../common/extern.h:40:19: warning: type specifier missing,
      defaults to 'int' [-Wimplicit-int]
int db_set(SCR *, recno_t, CHAR_T *, size_t);
                  ^
/Users/emirsari/Projeler/GitHub/nvi2/cl/../common/extern.h:41:21: warning: type specifier missing,
      defaults to 'int' [-Wimplicit-int]
int db_exist(SCR *, recno_t);
                    ^
fatal error: too many errors emitted, stopping now [-ferror-limit=]

I ported nvi to db5.3. It might work with BDB 18.

It might be a better option, but db1 support is a hard requirement for this project (for BSDs to use in their base systems).

I added ifdefs so that it still compiles with db1.

Using an absolute include path is also a problem when building this as part of the FreeBSD base system. I have made changes to allow building FreeBSD on Linux/macOS hosts, and using a host header when building world breaks the build.

See https://reviews.freebsd.org/D26480

Note: the absolute path could also cause problems when building FreeBSD world on a FreeBSD host if there is some divergence between the host db.h and the target one (e.g. upgrading major OS versions). Ignoring the user-provided --sysroot= flag is extremely dangerous since it breaks cross-compilation.

My suggestion would be to use the following:

/* Only include db1 */
#if __has_include(<db1/db.h>)
#include <db1/db.h>
#else
#include <db.h>
#endif

That way it always respects --sysroot, and will always prefer a db1-namespaced db.h if it exists.

__has_include was added in GCC 5 and Clang 2.7.0 (r85834), but if you need to support ancient compilers then you can have a fallback if __has_include isn't defined.

I like the solution using__has_include, although I don't know whether db1header is namespaced aa such under OSX.

macOS seems to have /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/db.h which should be compatible, but homebrew installs /usr/local/include/db.h which looks like it might not work.

Although the homebrew formula says

# --enable-compat185 is necessary because our build shadows
# the system berkeley db 1.x

so hopefully it's compatible?

db5.3 with --enable-compat185 is not fully compatible with db1

@arichardson is '/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include' included in the list of includes that clang -E - -v < /dev/null > /dev/null prints?

@matijaskala Yes since that's the default sysroot. However /usr/local/include comes first:

#include <...> search starts here:
 /usr/local/include
 /Library/Developer/CommandLineTools/usr/lib/clang/11.0.3/include
 /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include
 /Library/Developer/CommandLineTools/usr/include
 /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/System/Library/Frameworks (framework directory)

I think it would make sense to have cmake iterate over these directories and pick the one that has db1

After reading some cmake documentation I think that find_path(DB1_HEADER db.h) (with no other arguments) would set ${DB1_HEADER} to /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/db.h on osx and /usr/include/db.h on bsd.

find_path will honour the compiler's search order so won't change anything and will see /usr/local/include/db.h first.

In that case we can inspect ${CMAKE_SYSTEM_INCLUDE_PATH} and check version of every possible db.h we find

Why do we need to do this? I just tried building nvi2 on macOS and it worked just fine with my proposed common.h change (though I did need some unrelated patches/hacks for macOS compatibility).

Patches needed:

From e51c8b5306cbd5261061661024d4a36edfa1cf8a Mon Sep 17 00:00:00 2001
From: Jessica Clarke <jrtc27@jrtc27.com>
Date: Mon, 12 Oct 2020 17:25:28 +0100
Subject: [PATCH] Don't try and link with libtinfo if it doesn't exist

By default, ncurses does not build a separate libtinfo and instead
bundles it all into libncurses, so if libtinfo is missing then we should
just ignore it.
---
 CMakeLists.txt | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 996e0e7..10dd59e 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -142,7 +142,10 @@ else()
     target_compile_options(nvi PRIVATE -Wno-pointer-sign)
 endif()
 
-target_link_libraries(nvi PRIVATE ${CURSES_LIBRARY} ${TERMINFO_LIBRARY})
+target_link_libraries(nvi PRIVATE ${CURSES_LIBRARY})
+if(TERMINFO_LIBRARY)
+    target_link_libraries(nvi PRIVATE ${TERMINFO_LIBRARY})
+endif()
 
 if(USE_ICONV)
     check_function_exists(iconv ICONV_IN_LIBC)
-- 
2.28.0

plus st_mtim is called st_mtimespec on macOS so there needs to be some CMake detection to work out which field name to use.

Yeah, it could and probably should, I just wanted the quickest thing that would make it work so I could verify the thing I actually wanted to.

I can confirm that it builds just fine on macOS with the __has_include_patch() despite having homebrew's db.h header.

However, I did need the following patch:

commit 37ea21bc980251dc6be318a0ee18735411b11fe5
Author: Alex Richardson <Alexander.Richardson@cl.cam.ac.uk>
Date:   Mon Oct 12 17:43:42 2020 +0100

    Fix macos 10.15 build
    
    It looks like 45ab2b58973b40110c19704fbd1f756c1a33424d broke this.

diff --git a/common/exf.c b/common/exf.c
index f9eb215..f1fee80 100644
--- a/common/exf.c
+++ b/common/exf.c
@@ -199,7 +199,7 @@ file_init(SCR *sp, FREF *frp, char *rcv_name, int flags)
 		if (!LF_ISSET(FS_OPENERR))
 			F_SET(frp, FR_NEWFILE);
 
-		ep->mtim = sb.st_mtim;
+		ep->mtim = sb.st_mtimespec;
 	} else {
 		/*
 		 * XXX
@@ -218,7 +218,7 @@ file_init(SCR *sp, FREF *frp, char *rcv_name, int flags)
 		ep->mdev = sb.st_dev;
 		ep->minode = sb.st_ino;
 
-		ep->mtim = sb.st_mtim;
+		ep->mtim = sb.st_mtimespec;
 
 		if (!S_ISREG(sb.st_mode))
 			msgq_str(sp, M_ERR, oname,
@@ -796,7 +796,7 @@ file_write(SCR *sp, MARK *fm, MARK *tm, char *name, int flags)
 		if (noname && !LF_ISSET(FS_FORCE | FS_APPEND) &&
 		    ((F_ISSET(ep, F_DEVSET) &&
 		    (sb.st_dev != ep->mdev || sb.st_ino != ep->minode)) ||
-		    timespeccmp(&sb.st_mtim, &ep->mtim, !=))) {
+		    timespeccmp(&sb.st_mtimespec, &ep->mtim, !=))) {
 			msgq_str(sp, M_ERR, name, LF_ISSET(FS_POSSIBLE) ?
 "250|%s: file modified more recently than this copy; use ! to override" :
 "251|%s: file modified more recently than this copy");
@@ -895,7 +895,7 @@ success_open:
 			ep->mdev = sb.st_dev;
 			ep->minode = sb.st_ino;
 
-			ep->mtim = sb.st_mtim;
+			ep->mtim = sb.st_mtimespec;
 		}
 	}
 
diff --git a/common/recover.c b/common/recover.c
index 120cf4f..2fc6d43 100644
--- a/common/recover.c
+++ b/common/recover.c
@@ -701,7 +701,7 @@ rcv_read(SCR *sp, FREF *frp)
 		/* If we've found more than one, take the most recent. */
 		(void)fstat(fileno(fp), &sb);
 		if (recp == NULL ||
-		    timespeccmp(&rec_mtim, &sb.st_mtim, <)) {
+		    timespeccmp(&rec_mtim, &sb.st_mtimespec, <)) {
 			p = recp;
 			t = pathp;
 			recp = recpath;
@@ -710,7 +710,7 @@ rcv_read(SCR *sp, FREF *frp)
 				free(p);
 				free(t);
 			}
-			rec_mtim = sb.st_mtim;
+			rec_mtim = sb.st_mtimespec;
 			if (sv_fd != -1)
 				(void)close(sv_fd);
 			sv_fd = dup(fileno(fp));
diff --git a/ex/ex_cscope.c b/ex/ex_cscope.c
index 74d7f8a..0b687cd 100644
--- a/ex/ex_cscope.c
+++ b/ex/ex_cscope.c
@@ -261,7 +261,7 @@ cscope_add(SCR *sp, EXCMD *cmdp, CHAR_T *dname)
 	csc->dname = csc->buf;
 	csc->dlen = len;
 	memcpy(csc->dname, np, len);
-	csc->mtim = sb.st_mtim;
+	csc->mtim = sb.st_mtimespec;
 
 	/* Get the search paths for the cscope. */
 	if (get_paths(sp, csc))
@@ -813,7 +813,7 @@ csc_file(SCR *sp, CSC *csc, char *name, char **dirp, size_t *dlenp, int *isolder
 			*dirp = *pp;
 			*dlenp = strlen(*pp);
 			*isolderp = timespeccmp(
-			    &sb.st_mtim, &csc->mtim, <);
+			    &sb.st_mtimespec, &csc->mtim, <);
 			return;
 		}
 		free(buf);

Yes, that's what I said was broken in my comment. But that's not a real fix as it then just breaks Linux and the BSDs instead. The issue is that the sub-second precision fields aren't standardised and so there needs to be configure-time detection of whether to use the Linux/BSD names or the Darwin names (that or a bunch of #ifdef __APPLE__ or similar, but that's rather ugly and doesn't generalise to unknown platforms).

The statement made above re sysroot path is inaccurate in general. Some macOS versions use that, but not other.
/usr/local is a prefix used by some third-party software like Homebrew. It is not the prefix of macOS as such.