Another tiny patch for Linux GNUmakefile.in

Hi there. Thanks for the work you're doing for reworking the Makefiles. I've provided the patch I wanted to add for compiling under Linux.

This is for the build-rework branch, NOT the trunk, and is the output directly from the fnc program, as I can't figure out how to get fossil diff or f-vdiff to give me the hash for the current repository revision.

Fossil revision 181490a1c (branch build-rework):

diff 181490a1c091d9e8a4a25a63c7a1fe07325145ea /home/viking/src/c/libfossil/libfossil           5.26%[~] GNUmakefile.in  |  3  3  0 +++ 
1 file changed, 3 insertions(+), 0 deletions(-)

Index: GNUmakefile.in
=======================================================================
hash - 587a115444ede43fb966537ca9b61ab60eab5f26f3ba3cf62b8e4eab18ed8010
hash + 71c27026597745f7c357d5425d6f0f4ccb3bf2029848929a17f22d9e28669ee9
--- GNUmakefile.in
+++ GNUmakefile.in
@@ -518,4 +518,7 @@ ifeq (1,$(BUILD_CLIENT_FNC))
   ifneq ($(filter $(PLATFORM_UNAME),Darwin),)
     $(_fnc).BIN.LDFLAGS += -liconv
+  endif
+  ifneq ($(filter $(PLATFORM_UNAME),Linux),)
+    $(_fnc).BIN.LDFLAGS += -lbsd
   endif
   $(eval $(call ShakeNMake.CALL.RULES.BIN,$(_fnc)))

Feel free to amend if needed.

I've also noticed I can't build the f-apps on OpenBSD any more, as it claims that sqlite3.h is missing, though it is in extsrc/include and is in my /usr/local/include as well. The f-apps build fine over on Fedora and Ubuntu, so I'm scratching my head a bit about that, as everything built fine before the recent shuffle-about aside from the missing include statement and the linker correction above.

Cheers, brickviking
(Post 4 of some)

I can't figure out how to get fossil diff or f-vdiff to give me the hash for the current repository revision.

f-status will show you that. Also: f-resolve current

The version name "current" works only in checkouts and resolves to the current hash. Fossil has that feature, too.

I've also noticed I can't build the f-apps on OpenBSD any more, as it claims that sqlite3.h is missing,

That was fixed just before i went to bed, i thought. Speaking of...

i'm still in bed but will look at this, and your libbsd patch, as soon as i'm up and around.

Thank you for the continued feedback!

When I last posted, it was at revision 181490a1c, and definitely showing up, but seems to have been fixed since. As of repo revision 08f28aa79f3f, compiling seems to work on Fedora, Ubuntu and OpenBSD with or without libbsd present in the case of the Linux machines. Thanks for that - I've been trying to fight with that one for about five hours and not really winning. I'll look and see what I missed.

Here's some points I discovered along the way.

• As for compiling, at least on Ubuntu and Fedora, libbsd is not required. The changes you made render my libbsd patch redundant.

• I tested this quickly by renaming both the /usr/include/bsd directory and /usr/lib/x86_64-gnu-linux/libbsd.so to something not obviously a library. I then reran the configure, and then the make. Both completed with a resulting fnc binary and f-apps. This was true for Ubuntu 22.04 and Fedora 36, and is likely to hold true for future releases of both.

• Compiling with libbsd and the relevant includes present also runs to completion for both Linuxes.

• Compiling for OpenBSD 7.5 runs to completion as well.

I'll also poke about and see what few grammar/spelling errors I can find, though I doubt I'll find (m)any.

Cheers, brickviking
(Post 6)

tested this quickly by renaming both the /usr/include/bsd directory and /usr/lib/x86_64-gnu-linux/libbsd.so to something not obviously a library. I then reran the configure, and then the make.

The configure script now simply attempts to link some dummy app against -lbsd. If it works, that flag is added for fnc, else it doesn't add that flag. If fnc doesn't need it, but has the flag, no harm done.

I'll also poke about and see what few grammar/spelling errors I can find, though I doubt I'll find (m)any.

Oh, don't be so certain!

Thank you for the tests! i'll merge this into trunk sometime today.

Thank you for the tests! i'll merge this into trunk sometime today.

FYI: the build-rework branch has been merged into the trunk. Please report any new problems here.

complaints about alloca from libc in NetBSD being used in sqlite3.c

That part's fixed in the current trunk, i believe. We had the -DSQLITE_USE_ALLOCA flag there for reasons lost to history (probably copy/pasted from fossil 10 years ago). The other ones are for fnc so i'll wait for Mark to respond how he'd like to handle them (defining _OPENBSD_SOURCE directly in fnc or require it as a build flag - either works for me).

Thanks, brickviking!

The tolower(3) fix is definitely correct. And I agree with Stephan
inasmuch as -DSQLITE_USE_ALLOCA should get zapped. And we'll just
conditionally define _OPENBSD_SOURCE on NetBSD builds.

diff 717276940c7c9ce6415c29802498cef432a72712 26adf1707c1a075f599a02fe04a5be08666c6104
[~] fnc.bld.mk            |  1  0  1 -
[~] include/fnc_compat.h  |  4  4  0 ++++
[~] src/fnc.c             |  2  1  1 +-

3 files changed, 5 insertions(+), 2 deletions(-)

Index: fnc.bld.mk
=======================================================================
hash - 196ddef3e7fefbacf7ae2733d247033a6b4135d792f9e8ff48504e46416cf7ff
hash + 70b4dfbe66a793778712a71a89d3f133c9d4b9ade5b585bf936b38bd322bd2f7
--- fnc.bld.mk
+++ fnc.bld.mk
@@ -24,7 +24,6 @@ SQLITE_CFLAGS =	${C
 		-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 \

Index: include/fnc_compat.h
=======================================================================
hash - 85556f09ac604e1ae599d2095fbc1d047f767aa86c873558ff09aa38c5014d4e
hash + bb6f5631690c77372e00e84a5d5113d6c3acc42eca457cdea3de3646d9ba4092
--- include/fnc_compat.h
+++ include/fnc_compat.h
@@ -47,6 +47,10 @@
 
 #endif  /* __has_include */
 #endif  /* __linux__ */
+
+#ifdef __NetBSD__
+#define _OPENBSD_SOURCE		/* strtonum(3) */
+#endif
 
 #if !defined(__linux__) && !defined(__APPLE__)
 #define HAVE_REALLOCARRAY

Index: src/fnc.c
=======================================================================
hash - 703732d0398e7369f86e5ecd3771ce737ba4faa78ec9f6c21d485ccb896fd863
hash + adc4e9b42289738c496671e596680be094b7f2f7bb3ddeba1fde4a87c3fecb66
--- src/fnc.c
+++ src/fnc.c
@@ -13822,7 +13822,7 @@ fnc_conf_getopt(char **ret, enum fnc_opt_id id, bool ls
 				return RC_ERRNO("strdup");
 			}
 			for (t = lower; *t != '\0'; ++t)
-				*t = tolower(*t);
+				*t = tolower((unsigned char)*t);
 			envval = getenv(lower);
 			free(lower);
 		}

Thanks for that. I guess I can expect those in the next time I update my BSD's copies of libfossil. It just puzzles me that everything but NetBSD shrugs and say "Well, you must know what you're doing" whereas NetBSD seems to think "Hey, you really should make sure this is only an unsigned char", and because of the -Werror, doesn't go any further.

Anyhow, cheers again.
(Post 8, very short one)

Yes, I just ran a sync now: https://fossil.wanderinghorse.net/r/libfossil/info/8e0da47996461a2b

Actually, I usually do explicitly cast arguments to the character
handling functions but this one slipped by. Probably because my hard-
wired heuristic looks for isXXXXX(3) functions; so I can appreciate
NetBSD being vocal here. It's because char can be signed on some arches,
which makes tolower(c) undefined behaviour in such cases. So I very much
appreciate your report and patch :)

$(_fnc).BIN.LDFLAGS += -lbsd

As of the current build-rework tip, this is determined via the configure process. If configure can link -lbsd to an app then it's used, otherwise it's not. On my system, e.g., -lbsd fails to link (cannot find, despite libbsd clearly being in the system-level library path).

Please try that out:

$ ./configure
$ grep lbsd GNUmakefile

that should show two lines of output on your system, one of them is a comment and the other is LDFLAGS_LIBBSD .... If it doesn't, the new test in auto.def is broken.

This change was tested on my main workstation (Linux, but doesn't like -lbsd) and OpenBSD 7.5 on a Raspberry Pi4 (which is dog slow but works, and doesn't need -lbsd). What i'm missing is a system which actually needs -lbsd, so i need your feedback on whether this change actually works.

FYI, you can use the -o diff option (e.g., fnc diff -o) to write the
diff directly to stdout so you can pipe it to xclip(1) rather than copy
pasta the curses diff view. Alternatively, you can use the diff view P
keymap to write the currently viewed diff to file. See fnc diff -h or
the fnc(1) manual page (which is admittedly long but all the commands are
tagged so you can enter :t diff when viewing the manual page to jump
straight to the diff section).

Come to think of it, I did notice that :t diff didn't work, as less(1) complained there was no tags file at my present location, and less (at least on my Linux) wants a ctags file, and for it to be at my current location. Clearly the TAGS file (probably from etags) that's made on my system doesn't seem to be put into the right place for less to use. And of course, tags don't work with more(1), and I don't know about any other PAGER program - I can only remember those two - more or less ;-)

Cheers, brickviking
(Post 9 of more)

Clearly the TAGS file (probably from etags) that's made on my system doesn't seem to be put into the right place for less to use.

It's generated (if etags is found) in the top build directory. Is there a better/preferred option for that?

The first time i try to hit a tag in emacs, it asks me where the tags file is, i point it to the top dir, tap enter, and then it remembers that for the rest of the session. i only use that for jumping around files with esc-., and don't get any other use out of tags.

That's not quite what I meant. less(1) (at least on Linux) can't use an etags-created file, only a ctags-created one (or a very close output relative). And yes, when I have a tags file (not TAGS), hitting :t diff will take me to the place in the source code where it thinks it first found "diff".

Hope that clears up things. Yes, I've used emacs with tags, though I'm still not sure how to use it exactly. I also know vim has support for tags in whichever format it supports.

You can probably tell I don't use tags or cscope much.

Cheers, brickviking
(Post 11)

That's not quite what I meant. less(1) (at least on Linux) can't use an etags-created file, only a ctags-created one

Aha! i've always assumed (without ever having looked into it) that they were competing, but essentially compatible, products doing the same thing. Apparently my system has ctags (who knew?) so i will tinker with adding ctags support to the build, storing them as tags. If emacs can handle a ctags file, i'll default to using ctags, otherwise i'll possibly go ahead and generate both tags (ctags) and TAGS (etags), with the caveat that that won't work well on Windows, but we can skip generating those altogether on Windows.

I've used emacs with tags, though I'm still not sure how to use it exactly.

Put your cursor on some C symbol and tap (esc-.). That will, the first time, as where the tags file is (if it's not in the current dir), then will jump to the definition of that symbol. There's probably a way to jump to the decl but i've never looked into it.

You can probably tell I don't use tags or cscope much.

i tinkered with cscope once, and it's nice, but i just don't get enough use out of it to bother with it. i'm almost always working on my own trees, and know where everything is, and it's less useful in that case. If i were working on foreign trees, cscope and a "language server" would be a great help.