nalgeon / sqlean

The ultimate set of SQLite extensions
MIT License
3.72k stars 118 forks source link

Build failure: `error: implicit declaration of function 'timespec_get'` #124

Closed barracuda156 closed 3 months ago

barracuda156 commented 3 months ago

timespec_get is not supported on macOS prior to Mojave:

/opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_databases_sqlean/sqlean/work/compwrap/cc/opt/local/bin/gcc-mp-14 -O3 -Os -std=c99 -arch ppc -Wall -Wsign-compare -fPIC -dynamiclib -Isrc -DSQLEAN_VERSION='"0.24.0"' src/sqlite3-uuid.c src/uuid/*.c -o dist/uuid.dylib
src/uuid/extension.c: In function 'uuid_v7_generate':
src/uuid/extension.c:193:5: error: implicit declaration of function 'timespec_get' [-Wimplicit-function-declaration]
  193 |     timespec_get(&ts, TIME_UTC);
      |     ^~~~~~~~~~~~
src/uuid/extension.c:193:23: error: 'TIME_UTC' undeclared (first use in this function)
  193 |     timespec_get(&ts, TIME_UTC);
      |                       ^~~~~~~~
src/uuid/extension.c:193:23: note: each undeclared identifier is reported only once for each function it appears in
make: *** [compile-macos] Error 1

See also: https://gitlab.com/sifoo/snigl/-/issues/3 https://github.com/zeam-vm/pelemay/issues/142

nalgeon commented 3 months ago

So maybe don't use Sqlean on pre-Mojave macOS? Mojave is 2018, it's been 6 years.

barracuda156 commented 3 months ago

@nalgeon If you could help with this, it would be awesome. Maybe gettimeofday could be used as a fallback?

We try to support legacy systems wherever possible, they are still used. (On powerpc and i386 nothing beyond 10.6 can be installed.)

nalgeon commented 3 months ago

I'm happy to accept a PR that disables the UUIDv7 functions (as they are disabled for Win32 ). Unfortunately, I can't implement and test the fix myself, as I don't have a pre-Mojave OS.

nalgeon commented 3 months ago

Come to think of it, since you are building from source, a check for C11+ should suffice (bcf79e44b949a816d34917b1469448fe11c06444)

barracuda156 commented 3 months ago

@nalgeon Unfortunately, this won’t help. The standard defines it, but the actual implementation is provided by the OS. So it may or may not be actually present. See, for example, a trickery Clang does: https://reviews.llvm.org/D108203

I have tried building now without passing a standard (gcc should default to 2017) and explicitly passing gnu11, both fail. I cannot check for Clang-using systems, but from info online it looks like it won’t work either.

barracuda156 commented 3 months ago

When standard is passed explicitly to the compiler:

/opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_databases_sqlean/sqlean/work/compwrap/cc/opt/local/bin/gcc-mp-14 -O3 -Os -std=gnu11 -arch ppc -Wall -Wsign-compare -fPIC -dynamiclib -Isrc -DSQLEAN_VERSION='"0.24.0"' src/sqlite3-uuid.c src/uuid/*.c -o dist/uuid.dylib
src/uuid/extension.c: In function 'uuid_v7_generate':
src/uuid/extension.c:193:5: error: implicit declaration of function 'timespec_get' [-Wimplicit-function-declaration]
  193 |     timespec_get(&ts, TIME_UTC);
      |     ^~~~~~~~~~~~
src/uuid/extension.c:193:23: error: 'TIME_UTC' undeclared (first use in this function)
  193 |     timespec_get(&ts, TIME_UTC);
      |                       ^~~~~~~~
src/uuid/extension.c:193:23: note: each undeclared identifier is reported only once for each function it appears in
make: *** [compile-macos] Error 1

UPD. Apparently we can hack around by lowering C standard below C11 – to C99 – to trick the build into thinking we cannot support C11 :) This does work – i.e. the build completes successfully.

Do you recommend doing this? If so, then adding a check works to fix the issue.

nalgeon commented 3 months ago

I'm confused. Look at the build command from your original description:

gcc-mp-14 -O3 -Os -std=c99 -arch ppc -Wall -Wsign-compare -fPIC -dynamiclib -Isrc -DSQLEAN_VERSION='"0.24.0"' src/sqlite3-uuid.c src/uuid/*.c -o dist/uuid.dylib

If this is a real command, then you are already setting the standard to C99, and the fix should work.

barracuda156 commented 3 months ago

@nalgeon Yes, it works if we add -std=c99. But that was a manually added flag (why I had it: initially I tried building with gcc-4.2, that failed on c99 issues, I added a flag to fix those, build failed further down on something else, I switched to gcc14, but kept the flag).

I rely on your opinion here. We can pass the flag to force the compiler into C99 standard on affected systems. I just wanted to know if this is preferable over disabling specific functions but building with C11+.

nalgeon commented 3 months ago

Could you please provide an actual build command you use in the MacPorts project to build the UUID extension?

barracuda156 commented 3 months ago

Default flags are passed, which do not include C standard. Currently the port does not add any custom flags.

So it will be, in my case:

/opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_databases_sqlean/sqlean/work/compwrap/cc/opt/local/bin/gcc-mp-14 -O3 -Os -arch ppc -Wall -Wsign-compare -fPIC -dynamiclib -Isrc -DSQLEAN_VERSION='"0.24.0"' src/sqlite3-uuid.c src/uuid/*.c -o dist/uuid.dylib

Portfile blacklists old Xcode gcc: https://github.com/macports/macports-ports/blob/0efb401d6e3b427c2a28992d110cc0db60917b68/databases/sqlean/Portfile#L35-L40 So on any macOS version the compiler used will support C11, and by default (unless we pass a flag manually) no compiler will use C99, AFAIK.

nalgeon commented 3 months ago

Okay, so then it's better to disable UUIDv7 with an explicit SQLEAN_OMIT_UUID7 flag, I think (da9a26dfecefd05fdd517c61a574ef2bbaba324f)

barracuda156 commented 3 months ago

@nalgeon Thank you, I will try that.

barracuda156 commented 3 months ago

@nalgeon Yes, from da9a26dfecefd05fdd517c61a574ef2bbaba324f and passing -DSQLEAN_OMIT_UUID7 it builds fine. Thanks for fixing it!

Would you mind to make a minor release?

nalgeon commented 3 months ago

Thanks for letting me know! A patch release won't be enough?

barracuda156 commented 3 months ago

Thanks for letting me know! A patch release won't be enough?

Yeah, I meant 0.24.1.

nalgeon commented 3 months ago

0.24.1

barracuda156 commented 3 months ago

Awesome, thank you! I will make a PR to MacPorts with the update soon.

barracuda156 commented 3 months ago

Closing as fixed.