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

Merged
merged 1 commit into from
Dec 9, 2024

Conversation

alexfanqi
Copy link
Contributor

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
Contributor 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
Contributor

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
Contributor 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
Contributor

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.

Copy link
Collaborator

@fermino fermino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, thank you!

@fermino
Copy link
Collaborator

fermino commented Dec 9, 2024

Given that it was already approved by @cebtenzzre and it fixes the compilation issues in RHEL 10, ArchLinux and Debian 24+ (as found out while working on #677), I will go ahead and merge it.

Thanks again @alexfanqi for the fixes!

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.

3 participants