db: fix build on Darwin and clang 16 #241178
Conversation
The configure scripts frequently use `main` returning an implicit `int`, which causes spurious failures when CC is clang 16+. This is fixed by patching the provided macros and regenerating the scripts with autoreconfHook, though it requires some manual processing (see below). The upstream `configure.ac` is written in such a way that it requires fixups and post-processing. * Fixups are required because the original build process just cats the macros together into one file. When `aclocal` is run, it does not pick up all of them. This is worked around by catting the missing macros to a file that is picked up by autoreconfHook. * Post-processing is required to populate the version information. This is done in a subshell to avoid polluting the environment with the contents of `RELEASE`. Otherwise, the build will fail because the version C macros it expects will not be defined.
Both `_spin_lock_try` and `_spin_unlock` are private and deprecated APIs, which are not exported by any headers in the SDK. The build fails because the configure script does not define the functions before calling them, which is treated as error by clang 16. This patch replaces use of those APIs with `os_unfair_lock`, which is the recommended replacement per the deprecation messages.
|
@ofborg eval |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/prs-ready-for-review/3032/2390 |
|
Would it be possible to remove |
|
xcbuild creates a compatible “toolchain” that includes ctags, which uses This PR is not required for the stdenv bootstrap because nothing in the final stdenv depends on Berkeley DB. However, anything post-bootstrap that does will need these patches. That’s why this is one of the related (but not prerequisite) PRs in the tracking issue. I’m trying to address problems before the actual update is merged to minimize the amount of breakage. |
| extraPatches = [ ./clang-4.8.patch ./CVE-2017-10140-4.8-cwd-db_config.patch ] | ||
| ++ lib.optionals stdenv.isDarwin [ ./darwin-mutexes-4.8.patch ]; |
There was a problem hiding this comment.
We can't make the patch unconditional?
| @@ -19,10 +22,48 @@ stdenv.mkDerivation (rec { | |||
| sha256 = sha256; | |||
| }; | |||
|
|
|||
| # The provided configure script features `main` returning implicit `int`, which causes | |||
| # configure checks to work incorrectly with clang 16. | |||
| nativeBuildInputs = lib.optionals stdenv.cc.isClang [ autoreconfHook ]; | |||
There was a problem hiding this comment.
Can't we just reconfigure on all platforms? It would make things more uniform and bugs would be easier to spot on linux.
There was a problem hiding this comment.
I can see if it works on Linux and open a PR to make it unconditional if there are no issues.
There was a problem hiding this comment.
I opened #242060 to make them unconditional.
Description of changes
This PR updates Berkley DB’s configure scripts to build with clang 16. These changes were done to db5 then back and forward-ported to db4 and db6 as necessary.
inttomainin configure tests.signaland the signal handler’s definition in the mmap growth test to fix a false negative result with clang 16.In addition, it replaces the mutex implementation with
os_unfair_lockon Darwin._spin_lock_tryand_spin_unlockare private APIs and deprecated. The recommended replacement according to the deprecation message isos_unfair_lock. This also changes the db4 mutex implementation from pthreads toos_unfair_lock(even though db4 supported the other implementation, it wasn’t being used).Things done
sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)