unzip: fix configure script when using clang 16 #235121
Conversation
| +typedef struct _dir DIR; | ||
| +DIR* opendir(const char*); | ||
| +int closedir(DIR*); |
There was a problem hiding this comment.
I think the correct fix is to include the headers that define it:
#include <sys/types.h>
#include <dirent.h>
Maybe we could borrow the fix from other distributions?
There was a problem hiding this comment.
My reasoning for not using the headers here is that this particular test is determining which library has those symbols. It checks which headers to include later. I was trying to preserve that original separation of checks.
In practice, it shouldn’t matter for the platforms nixpkgs supports. If you’d rather I not bother, I can push an updated patch that just includes those headers.
There was a problem hiding this comment.
I don't have a strong opinion for this case.
Generally it's easy to cause unintended breakage by defining slightly incompatible prototypes (macros vs symbols, struct mismatch, struct fs integral typedef, -static -flto seeing through struct definitions). Would be unfortunate if it comes back and bites us later on more exotic target or toolchain.
There was a problem hiding this comment.
I agree it’s a bad test. It just checks for the first library that happens not to have missing symbols when you try to link it. I went ahead and pushed a commit to use the headers.
| +typedef struct _dir DIR; | ||
| +DIR* opendir(const char*); | ||
| +int closedir(DIR*); |
There was a problem hiding this comment.
I don't have a strong opinion for this case.
Generally it's easy to cause unintended breakage by defining slightly incompatible prototypes (macros vs symbols, struct mismatch, struct fs integral typedef, -static -flto seeing through struct definitions). Would be unfortunate if it comes back and bites us later on more exotic target or toolchain.
Clang 16 makes implicit int and function declarations an error by default in any mode except for C89. The configure script has two of these that cause it to misdetect features when using Clang 16. * Implicit `int main` in the errno check. This is fixed by adding the missing `int`. * Implicit definitions of `opendir` and `closedir`. These are fixed by including the required headers.
reckenrode
force-pushed
the
unzip-clang-fix
branch
from
a9e6027 to
1364837
Compare
Description of changes
Clang 16 makes implicit int and function declarations an error by default in any mode except for C89. The configure script has two of these that cause it to misdetect features when using Clang 16.
int mainin the errno check. This is fixed by adding the missingint.opendirandclosedir. These are fixed by declaring the function definitions directly inconftest.c.For
opendirandclosedir, I opted not to includedirent.hbecause the test is checking which library to link. Whetherdirent.hcan be used is checked separately. Using this approach preserves the intent of the check while fixing the incompatibility.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/)