Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix getxattr detection by properly calling CheckFunc #605

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

alexfanqi
Copy link

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
Copy link
Author

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

@alexfanqi alexfanqi changed the title add '\n' to multiline header in CheckFunc fix xattr detection by properly calling CheckFunc Jan 8, 2023
@alexfanqi alexfanqi changed the title fix xattr detection by properly calling CheckFunc fix getxattr detection by properly calling CheckFunc Jan 8, 2023
@cebtenzzre cebtenzzre added the topic-compiling Related to building rmlint from source label Feb 7, 2023
@cebtenzzre
Copy link
Collaborator

cebtenzzre commented Feb 7, 2023

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
Copy link
Author

alexfanqi commented Feb 9, 2023

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
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic-compiling Related to building rmlint from source
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants