Code Monkey home page Code Monkey logo

Comments (17)

freeekanayaka avatar freeekanayaka commented on September 21, 2024

Probably this is due to libraft being compiled without -fno-strict-aliasing.

from dqlite.

kloczek avatar kloczek commented on September 21, 2024

Indeed it is the case.
Code should not rely on optimisation flags.

from dqlite.

kloczek avatar kloczek commented on September 21, 2024

Strange is that compiler do not warns that some code may be incorrectly optimised because aliasing use 🤔

from dqlite.

kloczek avatar kloczek commented on September 21, 2024

@freeekanayaka could please have look on above issue? 🤔

from dqlite.

cole-miller avatar cole-miller commented on September 21, 2024

from dqlite.

kloczek avatar kloczek commented on September 21, 2024

As for the issue at hand, if the current raft code depends on strict aliasing being disabled for correctness, that's arguably unfortunate but not a priority for us to fix.

Depending on strict aliasing to be disabled is always nothing more than asking for trouble.
it would be really good to modify the code to remove that dependency.

Just checked and dqlite test suite still fails if raft is compiled with -fno-strict-aliasing

from dqlite.

freeekanayaka avatar freeekanayaka commented on September 21, 2024

@kloczek FWIW this has been fixed in cowsql/raft:

cowsql/raft#127

from dqlite.

kloczek avatar kloczek commented on September 21, 2024

One sec please let me check that 😋

from dqlite.

kloczek avatar kloczek commented on September 21, 2024

Just tested and now build fails

[tkloczko@pers-jacek dqlite-1.16.2]$ make -k
  CC       src/libdqlite_la-server.lo
src/server.c: In function 'dqlite__init':
src/server.c:123:9: error: implicit declaration of function 'raft_register_state_cb' [-Wimplicit-function-declaration]
  123 |         raft_register_state_cb(&d->raft, state_cb);
      |         ^~~~~~~~~~~~~~~~~~~~~~
src/server.c:123:9: error: nested extern declaration of 'raft_register_state_cb' [-Werror=nested-externs]
cc1: all warnings being treated as errors
make: *** [Makefile:1611: src/libdqlite_la-server.lo] Error 1
make: Target 'all' not remade because of errors.
[tkloczko@pers-jacek dqlite-1.16.2]$ make -k V=1
/bin/sh ./libtool  --tag=CC   --mode=compile /usr/bin/gcc -DPACKAGE_NAME=\"libdqlite\" -DPACKAGE_TARNAME=\"libdqlite\" -DPACKAGE_VERSION=\"1.16.2\" -DPACKAGE_STRING=\"libdqlite\ 1.16.2\" -DPACKAGE_BUGREPORT=\"https://github.com/canonical/dqlite\" -DPACKAGE_URL=\"\" -DPACKAGE=\"libdqlite\" -DVERSION=\"1.16.2\" -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_ARPA_INET_H=1 -DHAVE_FCNTL_H=1 -DHAVE_STDINT_H=1 -DHAVE_STDLIB_H=1 -DHAVE_STRING_H=1 -DHAVE_SYS_SOCKET_H=1 -DHAVE_UNISTD_H=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  -O2   -fvisibility=hidden -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -O2 -g -grecord-gcc-switches -pipe -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 -o src/libdqlite_la-server.lo `test -f 'src/server.c' || echo './'`src/server.c
libtool: compile:  /usr/bin/gcc -DPACKAGE_NAME=\"libdqlite\" -DPACKAGE_TARNAME=\"libdqlite\" -DPACKAGE_VERSION=\"1.16.2\" "-DPACKAGE_STRING=\"libdqlite 1.16.2\"" -DPACKAGE_BUGREPORT=\"https://github.com/canonical/dqlite\" -DPACKAGE_URL=\"\" -DPACKAGE=\"libdqlite\" -DVERSION=\"1.16.2\" -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_ARPA_INET_H=1 -DHAVE_FCNTL_H=1 -DHAVE_STDINT_H=1 -DHAVE_STDLIB_H=1 -DHAVE_STRING_H=1 -DHAVE_SYS_SOCKET_H=1 -DHAVE_UNISTD_H=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 -O2 -fvisibility=hidden -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -O2 -g -grecord-gcc-switches -pipe -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:123:9: error: implicit declaration of function 'raft_register_state_cb' [-Wimplicit-function-declaration]
  123 |         raft_register_state_cb(&d->raft, state_cb);
      |         ^~~~~~~~~~~~~~~~~~~~~~
src/server.c:123:9: error: nested extern declaration of 'raft_register_state_cb' [-Werror=nested-externs]
cc1: all warnings being treated as errors

and ..

[tkloczko@pers-jacek dqlite-1.16.2]$ rpm -ql raft-devel | grep include\*.h$ | xargs grep raft_register_state_cb
[tkloczko@pers-jacek dqlite-1.16.2]$

from dqlite.

freeekanayaka avatar freeekanayaka commented on September 21, 2024

Probably there was some confusion, when I wrote that this was fixed I meant the strict aliasing issue, not this missing function.

I could add that function to cowsql/raft for compatibility, but does not make much sense since dqlite is now going to ship with the raft source included.

I'd suggest that you just wait for #568 to be merged. After that you won't need any external raft library to build dqlite.

from dqlite.

kloczek avatar kloczek commented on September 21, 2024

I could add that function to cowsql/raft for compatibility, but does not make much sense since dqlite is now going to ship with the raft source included.

But why? 🤔
Why dqlite cannot be build against system installed raft? 🤔
Budling other projects code is always nothing more tan asking for troubles ..

from dqlite.

freeekanayaka avatar freeekanayaka commented on September 21, 2024

I could add that function to cowsql/raft for compatibility, but does not make much sense since dqlite is now going to ship with the raft source included.

But why? 🤔 Why dqlite cannot be build against system installed raft? 🤔 Budling other projects code is always nothing more tan asking for troubles ..

Dqlite 1.16.0 is compatible with cowsql/raft (which is a fork of canonical/raft), starting from dqlite version 1.16.2 you need to use canonical/raft because new interfaces were added to it that are not present in cowsql/raft. Since canonical/raft is going to be merged into dqlite directly (see #568) I will not fix cowsql/raft to work with dqlite 1.16.2. Just wait for dqlite 1.16.3 and you will not need neither canonical/raft or cowsql/raft.

from dqlite.

kloczek avatar kloczek commented on September 21, 2024

Dqlite 1.16.0 is compatible with cowsql/raft (which is a fork of canonical/raft), starting from dqlite version 1.16.2 you need to use canonical/raft because new interfaces were added to it that are not present in cowsql/raft. Since canonical/raft is going to be merged into dqlite directly (see #568) I will not fix cowsql/raft to work with dqlite 1.16.2. Just wait for dqlite 1.16.3 and you will not need neither canonical/raft or cowsql/raft.

Does it mean that raft project will be no longer maintained and will be archived? 🤔

from dqlite.

freeekanayaka avatar freeekanayaka commented on September 21, 2024

Dqlite 1.16.0 is compatible with cowsql/raft (which is a fork of canonical/raft), starting from dqlite version 1.16.2 you need to use canonical/raft because new interfaces were added to it that are not present in cowsql/raft. Since canonical/raft is going to be merged into dqlite directly (see #568) I will not fix cowsql/raft to work with dqlite 1.16.2. Just wait for dqlite 1.16.3 and you will not need neither canonical/raft or cowsql/raft.

Does it mean that raft project will be no longer maintained and will be archived? 🤔

canonical/raft will be merged into canonical/dqlite, it will be a single repository with both source codes and a single shared library.

cowsql/raft will continue to be a standalone repository and library.

from dqlite.

kloczek avatar kloczek commented on September 21, 2024

canonical/raft will be merged into canonical/dqlite, it will be a single repository with both source codes and a single shared library.

cowsql/raft will continue to be a standalone repository and library.

So what is the sense use forked version and why not port dqlite to cowsql/raft? 🤔

from dqlite.

freeekanayaka avatar freeekanayaka commented on September 21, 2024

canonical/raft will be merged into canonical/dqlite, it will be a single repository with both source codes and a single shared library.
cowsql/raft will continue to be a standalone repository and library.

So what is the sense use forked version and why not port dqlite to cowsql/raft? 🤔

dqlite has been forked too: https://github.com/cowsql/cowsql and that fork uses cowsql/raft. Cowsql is currently being used by Incus.

EDIT: As said, my plan was to keep cowsql/raft compatible with dqlite, and in fact it is compatible up to dqlite 1.16.0. However there's now no reason to maintain compatibility, since dqlite is going to embed raft in its own source code.

from dqlite.

kloczek avatar kloczek commented on September 21, 2024

dqlite has been forked too: https://github.com/cowsql/cowsql and that fork uses cowsql/raft. Cowsql is currently being used by Incus.

OK thank you to let me know.
(I'm switching my build procedures to use those two repos)

EDIT: As said, my plan was to keep cowsql/raft compatible with dqlite, and in fact it is compatible up to dqlite 1.16.0. However there's now no reason to maintain compatibility, since dqlite is going to embed raft in its own source code.

Which is as always nothing more than asking for troubles ..

from dqlite.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.