1.16.5: build fails with `-Wimplicit-function-declaration`

[kloczek]

Looks like something is wrong and build fails with:

libtool: compile:  /usr/bin/gcc -DPACKAGE_NAME=\"libdqlite\" -DPACKAGE_TARNAME=\"libdqlite\" -DPACKAGE_VERSION=\"1.16.5\" "-DPACKAGE_STRING=\"libdqlite 1.16.5\"" -DPACKAGE_BUGREPORT=\"https://github.com/canonical/dqlite\" -DPACKAGE_URL=\"\" -DPACKAGE=\"libdqlite\" -DVERSION=\"1.16.5\" -DHAVE_STDIO_H=1 -DHAVE_STDLIB_H=1 -DHAVE_STRING_H=1 -DHAVE_INTTYPES_H=1 -DHAVE_STDINT_H=1 -DHAVE_STRINGS_H=1 -DHAVE_SYS_STAT_H=1 -DHAVE_SYS_TYPES_H=1 -DHAVE_UNISTD_H=1 -DHAVE_WCHAR_H=1 -DSTDC_HEADERS=1 -D_ALL_SOURCE=1 -D_DARWIN_C_SOURCE=1 -D_GNU_SOURCE=1 -D_HPUX_ALT_XOPEN_SOCKET_API=1 -D_NETBSD_SOURCE=1 -D_OPENBSD_SOURCE=1 -D_POSIX_PTHREAD_SEMANTICS=1 -D__STDC_WANT_IEC_60559_ATTRIBS_EXT__=1 -D__STDC_WANT_IEC_60559_BFP_EXT__=1 -D__STDC_WANT_IEC_60559_DFP_EXT__=1 -D__STDC_WANT_IEC_60559_EXT__=1 -D__STDC_WANT_IEC_60559_FUNCS_EXT__=1 -D__STDC_WANT_IEC_60559_TYPES_EXT__=1 -D__STDC_WANT_LIB_EXT2__=1 -D__STDC_WANT_MATH_SPEC_FUNCS__=1 -D_TANDEM_SOURCE=1 -D__EXTENSIONS__=1 -DHAVE_PTHREAD_PRIO_INHERIT=1 -DHAVE_PTHREAD=1 -DHAVE_DLFCN_H=1 -DLT_OBJDIR=\".libs/\" -DHAVE_LINUX_IO_URING_H=1 -DHAVE_LINUX_AIO_ABI_H=1 -DHAVE_DECL_RWF_NOWAIT=1 -I. -std=c11 -g3 -fcf-protection --param=ssp-buffer-size=4 -pipe -fno-strict-aliasing -fdiagnostics-color -fexceptions -fstack-clash-protection -fstack-protector-strong -fasynchronous-unwind-tables -fdiagnostics-show-option -Wall -Wextra -Wimplicit-fallthrough=5 -Wcast-align -Wstrict-prototypes -Wlogical-op -Wmissing-include-dirs -Wold-style-definition -Winit-self -Wfloat-equal -Wsuggest-attribute=noreturn -Wformat=2 -Wshadow -Wendif-labels -Wdate-time -Wnested-externs -Wconversion -Werror -pthread -DUSE_SYSTEM_RAFT -O2 -fvisibility=hidden -DRAFT_API= -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -O2 -g -grecord-gcc-switches -pipe -mtls-dialect=gnu2 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -fdata-sections -ffunction-sections -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -flto=auto -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=3 -Wp,-D_GLIBCXX_ASSERTIONS -Wall -Werror=format-security -fno-strict-aliasing -c src/server.c  -fPIC -DPIC -o src/.libs/libdqlite_la-server.o
src/server.c: In function 'dqlite__init':
src/server.c:140:9: error: implicit declaration of function 'raft_register_state_cb' [-Wimplicit-function-declaration]
  140 |         raft_register_state_cb(&d->raft, state_cb);
      |         ^~~~~~~~~~~~~~~~~~~~~~
src/server.c:140:9: error: nested extern declaration of 'raft_register_state_cb' [-Werror=nested-externs]
cc1: all warnings being treated as errors

[cole-miller]

Duplicate of #650, #594, and #549, and the answer is the same. Please don't open additional issues about this.

[kloczek]

Two of those tickets are about to the same issue.

I'm using raft from https://github.com/cowsql/raft/ (0.22.1).

Just checked raft 0.22.1 source code.

[tkloczko@pers-jacek raft-0.22.1]$ grep -r raft_register_state_cb
[tkloczko@pers-jacek raft-0.22.1]$

[kloczek]

https://github.com/canonical/raft/ is at the moment archived so it is no longer maintained. Q: where is raft which I should use? 🤔

[kloczek]

From https://github.com/canonical/raft/

The dqlite team is no longer maintaining our raft implementation as an independent project. Instead, the raft source code has been incorporated into canonical/dqlite as a private implementation detail

and looks like just released new version wants to use system installed raft

checking pkg-config is at least version 0.9.0... yes
checking for SQLITE... yes
checking for UV... yes
checking for RAFT... no
configure: error: Package requirements (raft >= 0.18.1) were not met:

Package 'raft', required by 'virtual:world', not found

Consider adjusting the PKG_CONFIG_PATH environment variable if you
installed software in a non-standard prefix.

So I'm now puzzled a bit 🤔

[freeekanayaka]

What about dropping support for external linking to libraft altogether? Not sure what it is needed for, but it seems to mostly just bring confusion.

[cole-miller]

@kloczek Apologize for the confusion, I thought I'd explained this fully elsewhere but maybe I left some key details out. The following are all true:

• --enable-build-raft is now the preferred way to build dqlite.
• canonical/raft is archived and not receiving new releases (past v0.18.1).
• The dqlite build system continues to support linking to a separately-build libraft.so. This is the default (if you don't pass --enable-build-raft) to maintain backward compatibility.
• When using the "separately-built libraft" configuration, dqlite depends on canonical/raft, not cowsql/raft. You should use the last release of canonical/raft, v0.18.1. If you install cowsql/raft and try to build dqlite against it, our configure script will find the library (since it's installed under the same pkg-config name as canonical/raft), but then the actual build will fail because cowsql/raft doesn't implement all features of canonical/raft v0.18.1. The dqlite team can't do anything about this.
• Eventually, the "separately-built libraft" build configuration for dqlite will go away, and --enable-build-raft will be the default and only choice. This is why you should switch to --enable-build-raft as soon as possible.

I hope that covers everything!

[cole-miller]

@freeekanayaka

What about dropping support for external linking to libraft altogether? Not sure what it is needed for, but it seems to mostly just bring confusion.

I wanted to give users of dqlite a transition period to switch over to the new build configuration. We will definititely remove the option to link a libraft eventually.

[freeekanayaka]

@freeekanayaka

What about dropping support for external linking to libraft altogether? Not sure what it is needed for, but it seems to mostly just bring confusion.

I wanted to give users of dqlite a transition period to switch over to the new build configuration. We will definititely remove the option to link a libraft eventually.

Yeah, I gathered that, but I don't quite see why a user would want to stick to the old configuration.

If you just make the new configuration the default, that will be virtually transparent to users, because you're removing a dependency, not adding one. And on the plus side people won't be confused, it should just work afaict.

[cole-miller]

If you just make the new configuration the default, that will be virtually transparent to users, because you're removing a dependency, not adding one.

I agree this is convenient, but it seems like a potential violation of expectations that code from libraft.so no longer runs after an upgrade in this case. What if someone has installed a patched libraft and is relying on dqlite to call into that patched code? The transition period gives people a chance to explicitly turn on --enable-build-raft and simultaneously update how they build and install dqlite to reflect that libraft is no longer in the loop.

That said, we've had three releases now featuring --enable-build-raft, so it's probably a good idea to retire it with the next release. Thanks to you both for pointing out the potential confusion.