lichray / nvi2

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

Build failure on macOS #69

Closed bitigchi closed 3 years ago

bitigchi commented 4 years ago

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.

lichray commented 4 years ago

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)?

bitigchi commented 4 years ago

@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=]
matijaskala commented 4 years ago

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

lichray commented 4 years ago

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).

matijaskala commented 4 years ago

I added ifdefs so that it still compiles with db1.

arichardson commented 3 years ago

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

arichardson commented 3 years ago

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.

jrtc27 commented 3 years ago

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.

zhihaoy commented 3 years ago

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

arichardson commented 3 years ago

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.

arichardson commented 3 years ago

Although the homebrew formula says

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

so hopefully it's compatible?

matijaskala commented 3 years ago

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

matijaskala commented 3 years ago

@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?

arichardson commented 3 years ago

@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)
matijaskala commented 3 years ago

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

matijaskala commented 3 years ago

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.

jrtc27 commented 3 years ago

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

matijaskala commented 3 years ago

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

jrtc27 commented 3 years ago

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.

arichardson commented 3 years ago

Maybe the check for ncurses should use https://cmake.org/cmake/help/v3.18/module/FindCurses.html?

jrtc27 commented 3 years ago

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.

arichardson commented 3 years ago

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);
jrtc27 commented 3 years ago

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).

matijaskala commented 3 years ago

https://github.com/matijaskala/nvi2/tree/apple

barracuda156 commented 4 months ago

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.