fnc is not using the right compiler or arch flags

[ryandesign (Ryan Carsten Schmidt)]

The fnc port uses use_configure no. Thus it becomes the portfile's responsibility to ensure that the right compiler and flags are being used, and the portfile currently does not do this.

The makefile 1.0 portgroup can often be employed to help you do this. Read the portgroup to find out how it works. Using this portgroup will make it unnecessary to manually set use_configure or destroot.destdir.

While you're updating the port for this, you can remove the distname line, since the value you've set it to is the default value.

Herby, Cc'ing you since you merged the PR. Any time you see use_configure no it should be a big red flag to check for the right compiler and flags.

[ryandesign (Ryan Carsten Schmidt)]

To clarify what's meant by "right compiler and flags", look at a ​build log:

cc  -Wall -Werror -Wno-sign-compare -pedantic -std=c99 -DNDEBUG=1 -DSQLITE_DQS=0 -DSQLITE_THREADSAFE=0 -DSQLITE_DEFAULT_MEMSTATUS=0 -DSQLITE_DEFAULT_WAL_SYNCHRONOUS=1 -DSQLITE_LIKE_DOESNT_MATCH_BLOBS -DSQLITE_OMIT_DECLTYPE -DSQLITE_OMIT_PROGRESS_CALLBACK -DSQLITE_OMIT_SHARED_CACHE -DSQLITE_OMIT_LOAD_EXTENSION -DSQLITE_MAX_EXPR_DEPTH=0 -DSQLITE_USE_ALLOCA -DSQLITE_ENABLE_LOCKING_STYLE=0 -DSQLITE_DEFAULT_FILE_FORMAT=4 -DSQLITE_ENABLE_EXPLAIN_COMMENTS -DSQLITE_ENABLE_FTS4 -DSQLITE_ENABLE_DBSTAT_VTAB -DSQLITE_ENABLE_JSON1 -DSQLITE_ENABLE_FTS5 -DSQLITE_ENABLE_STMTVTAB -DSQLITE_HAVE_ZLIB -DSQLITE_INTROSPECTION_PRAGMAS -DSQLITE_ENABLE_DBPAGE_VTAB -DSQLITE_TRUSTED_SCHEMA=0 -c lib/sqlite3.c -o lib/sqlite3.o
cc  -Wall -Werror -Wsign-compare -pedantic -std=c99 -c lib/libfossil.c -o lib/libfossil.o
cc  -Wall -Werror -Wsign-compare -pedantic -std=c99 -I./lib -I./include -I/usr/include/ncursesw -D_XOPEN_SOURCE_EXTENDED -DVERSION=0.6 -c src/fnc.c -o src/fnc.o
cc -o src/fnc src/fnc.o lib/libfossil.o lib/sqlite3.o  -lm -lutil -lz -lpthread -fPIC -lncurses -lpanel -liconv

• It is using cc when it should be using whatever MacPorts has set the variable ${configure.cc} to
• It should be using the flags MacPorts set in ${configure.cflags}, ${configure.cppflags} and ${configure.ldflags}

I also see a -I/usr/include/ncursesw in there which is wrong since the port declares a dependency on the ncurses port so clearly we're wanting to use MacPorts ncurses, not any that might be provided by the OS. MacPorts cppflags come preset to -I${prefix}/include so by using those the right flag will be there. You may still need to take additional measures to remove the incorrect flag, depending on how it's getting into the build. (Read the Makefile to find out.)

When you fix this, since the build will then be using different flags (such as different optimization flags, which will result in a different executable being produced than before), the port's revision should be increased.

[mcjsk (Mark Jamsek)]

Replying to ryandesign:

I also see a -I/usr/include/ncursesw in there which is wrong since the port declares a dependency on the ncurses port so clearly we're wanting to use MacPorts ncurses, not any that might be provided by the OS. MacPorts cppflags come preset to -I${prefix}/include so by using those the right flag will be there. You may still need to take additional measures to remove the incorrect flag, depending on how it's getting into the build. (Read the Makefile to find out.)

Thanks, ryandesign@. I'll tend to this.

Rather than depend on MacPorts ncurses, can we use the macOS base install instead? Originally, we didn't have the port:ncurses included in the depends_lib.

[ryandesign (Ryan Carsten Schmidt)]

Replying to mcjsk:

Replying to ryandesign:

I also see a -I/usr/include/ncursesw in there which is wrong since the port declares a dependency on the ncurses port so clearly we're wanting to use MacPorts ncurses, not any that might be provided by the OS. MacPorts cppflags come preset to -I${prefix}/include so by using those the right flag will be there. You may still need to take additional measures to remove the incorrect flag, depending on how it's getting into the build. (Read the Makefile to find out.)

Thanks, ryandesign@. I'll tend to this.

The makefile portgroup was added to the port which takes care of using the right compiler and flags. But -I/usr/include/ncursesw is still present in the most recent build log and should be removed. I see it's simply hardcoded into FNC_CFLAGS in the fnc.bld.mk file so a patch should be added to remove that part.

Rather than depend on MacPorts ncurses, can we use the macOS base install instead? Originally, we didn't have the port:ncurses included in the depends_lib.

We usually don't want to do that, as explained in wiki:FAQ#ownlibs.

[mcjsk (Mark Jamsek)]

Thanks for the hints ryandesign!

wrt #67290, it looks like the missing declaration build warnings are for the gcc fall through attribute in upstream libfossil.c. From source code inspection, the attribute is correctly used but I don't think this exists in older gcc (< 7, IIRC). In any case, we can patch out -Werror from the libfossil compilation.

I think the below diff addresses the ncursesw include and -Werror issues. If so, please let me know and I'll submit a PR.

Thanks again!

---

diff /home/mark/src/macport-ports
commit - 0a81b08c5c8033f3b2f71d1185ec0e129e49a612
path + /home/mark/src/macport-ports
blob - ef0e1e3feae0cb775fe263136db728db0c09d816
file + devel/fnc/Portfile
--- devel/fnc/Portfile
+++ devel/fnc/Portfile
@@ -5,7 +5,7 @@ revision        0
 
 name            fnc
 version         0.15
-revision        0
+revision        1
 categories      devel
 license         ISC
 maintainers     {bsdbox.org:mark @mcjsk} \
@@ -27,8 +27,11 @@ depends_lib-append      port:zlib
 
 build.type      bsd
 
-depends_lib-append      port:zlib
+patchfiles      libf-Werror-include-ncursesw.diff
 
+depends_lib-append      port:zlib \
+                        port:ncurses
+
 destroot {
     xinstall -m 755 ${worksrcpath}/src/${name} ${destroot}${prefix}/bin/${name}
     xinstall -m 444 ${worksrcpath}/src/${name}.1 ${destroot}${prefix}/share/man/man1/${name}.1
blob - /dev/null
file + devel/fnc/files/libf-Werror-include-ncursesw.diff (mode 644)
--- /dev/null
+++ devel/fnc/files/libf-Werror-include-ncursesw.diff
@@ -0,0 +1,22 @@
+Index: fnc.bld.mk
+=======================================================================
+hash - d5125d91b651e4115400e52857e80fd35cd8fb25ccf50e3828db32c27f119b63
+hash + 0461726b7bef3caa495e6e17283581bc33b1f0d439da29c2ef477439a27d6d45
+--- fnc.bld.mk
++++ fnc.bld.mk
+@@ -37,13 +37,13 @@ FOSSIL_CFLAGS =	${CFLAGS} -Wall -Werror -Wsign-compare 
+ 		-DSQLITE_TRUSTED_SCHEMA=0
+ 
+ # FLAGS NEEDED TO BUILD LIBFOSSIL
+-FOSSIL_CFLAGS =	${CFLAGS} -Wall -Werror -Wsign-compare -pedantic -std=c99
++FOSSIL_CFLAGS =	${CFLAGS} -Wall -Wsign-compare -pedantic -std=c99
+ 
+ # On SOME Linux (e.g., Ubuntu 18.04.6), we have to include wchar curses from
+ # I/.../ncursesw, but linking to -lncursesw (w/ no special -L path) works fine.
+ # FLAGS NEEDED TO BUILD FNC
+ FNC_CFLAGS =	${CFLAGS} -Wall -Werror -Wsign-compare -pedantic -std=c99 \
+-		-I./lib -I./include -I/usr/include/ncursesw \
++		-I./lib -I./include \
+ 		-D_XOPEN_SOURCE_EXTENDED -DVERSION=${VERSION} -DHASH=${HASH} \
+ 		-DDATE="${DATE}"
+ 

[ryandesign (Ryan Carsten Schmidt)]

Replying to mcjsk:

I think the below diff addresses the ncursesw include and -Werror issues. If so, please let me know and I'll submit a PR.

It looks like it does. I haven't tested it. Please submit a PR which will kick off automated CI builds on recent macOS; we can see whether it fixes the build on older macOS by checking the buildbot after the PR is merged. Be sure to mention in your commit message the two tickets it closes.

Please do submit PRs for any change you wish to make to your ports without asking for approval first. Several developers check the incoming PRs frequently and will review them, provide suggestions, and merge them. I only noticed today that you had left a patch here, though you left it here three months ago, so the port could have been fixed three months sooner if there had been a PR.

[Mark Jamsek <mark@…>]

In 83d43386d08576fa0f71e655cbcd96086c3353fa/macports-ports (master):

fnc: close tickets ​https://github.com/macports/macports-ports/pull/64175 and ​https://github.com/macports/macports-ports/pull/67290

(​https://github.com/macports/macports-ports/pull/64175) changes the Portfile to add a missing depends (ncurses), and patches
out an unwanted include (i.e,-I/usr/include/ncursesw) from upstream's makefile.
(​https://github.com/macports/macports-ports/pull/67290) patches out -Werror from the upstream's libfossil build to silence
warnings due to missing fallthrough attribute on older compilers.

Closes: #64175
Closes: #67290