search

sahib / rmlint

Extremely fast tool to remove duplicates and other lint from your filesystem
http://rmlint.rtfd.org
GNU General Public License v3.0
1.86k stars 128 forks source link

fix getxattr detection by properly calling CheckFunc #605

Open alexfanqi opened 1 year ago

alexfanqi commented 1 year ago

with

header=
    '#include<a>'
    '#include<b>'

scons will generate it in one single line '#include<a>#include<b>, causing wrong config and bug

ref: https://bugs.gentoo.org/870850

alexfanqi commented 1 year ago

oh, the proper way is actually not having a header because CheckFunc is meant to check linking the function

ref: https://pairlist4.pair.net/pipermail/scons-users/2023-January/009150.html

cebtenzzre commented 1 year ago

I believe that Gentoo bug is different, as IIRC that's what happens if you do manage to the disable the xattr feature - the #ifs are inconsistent and it doesn't build. The issue fixed by this PR causes the xattr feature to be falsely enabled, which would result in a complaint about a missing header, function declaration, or symbol.

I'll make this a higher priority if you can give an example of an OS rmlint should support that does not provide said xattr functions. As far as I know compilation on current macOS and Linux systems is not affected by this issue.

(edit: This PR fixes a compiler warning.)

alexfanqi commented 1 year ago

Thanks for looking into my pr. I still think this issue is not relevant to whether xattr is enabled or not on the system. I should have provided more context.

The issue is if header is manually specified, scons won't put a default dummy declaration like char getxattr(void); in conftest.c. https://github.com/SCons/scons/blob/440728dd1d9fee6a8e010b4d9871737686cb3afb/SCons/Conftest.py#L265

This dummy declaration cannot be omited. It worked previously because of a bad c89 legacy, which the compiler implicitly add a declaration if there is none. This legacy has been deemed bad, but not until now, clang-16 and future gcc decided to remove it and issue an implicit-function-declaration error. https://discourse.llvm.org/t/configure-script-breakage-with-the-new-werror-implicit-function-declaration/65213

The error will let scons think getxattr is not present. So the solution would be we don't specify header and let scons add it by default.

cebtenzzre commented 1 year ago

Oh, I didn't know that implicit function declarations are going to become an error in the future. I've only tested with released versions of compilers (clang 15 and GCC 12.2). Thanks for letting me know, I will make sure this gets included in the next release.