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

perl plugin: fix Clang compilation by ignoring a warning #4309

Merged
merged 1 commit into from May 21, 2024

Conversation

jvsg
Copy link
Contributor

@jvsg jvsg commented May 8, 2024

ChangeLog: perl plugin: fix Clang compilation by ignoring a warning

Fixes: #4308

@jvsg jvsg requested a review from a team as a code owner May 8, 2024 01:16
@eero-t eero-t changed the title clang: fix compilation by ignoring a warning perl plugin: fix Clang compilation by ignoring a warning May 8, 2024
@eero-t eero-t added the Fix A pull request fixing a bug label May 8, 2024
@eero-t
Copy link
Contributor

eero-t commented May 8, 2024

Changed title to better describe what is being fixed, and added the required (Fix) label & "ChangeLog" entry.

Copy link
Contributor

@eero-t eero-t left a comment

Choose a reason for hiding this comment

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

If the warned macro issue cannot be fixed (I know very little of Perl, so I have no idea how hard that would be), at least the #if guard should be fixed to check for correct compiler version.

src/perl.c Outdated Show resolved Hide resolved
@eero-t
Copy link
Contributor

eero-t commented May 8, 2024

@mrunge, do you have Linux setup for checking whether Perl plugin has same issue with current Linux Clang / or any other comments on this?

(I don't have any Perl dev stuff on my setup, and I'd rather keep it out of it, as I'm rather a Python user. :-))

@eero-t
Copy link
Contributor

eero-t commented May 8, 2024

Commit title should be also be fixed, to indicate that it's the perl plugin that is being fixed (not clang one).

A new warning type introduced since clang 12 causes a build failure.
Ignore it to fix.

https://reviews.llvm.org/D86751

Fixes: collectd#4308
@jvsg
Copy link
Contributor Author

jvsg commented May 20, 2024

@eero-t please re-review

Copy link
Contributor

@eero-t eero-t left a comment

Choose a reason for hiding this comment

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

Thanks, this looks fine now! => Triggered checks & approved.

@eero-t
Copy link
Contributor

eero-t commented May 20, 2024

The only failure is unrelated (#4298).

Before merging, I would a comment from somebody who uses (or at least builds) Perl plugin. @mrunge?

@mrunge
Copy link
Member

mrunge commented May 21, 2024

I am not using the perl plugin, and I'm also not building collectd on a regular basis anymore, so can't add test cases or feedback to this.

@mrunge
Copy link
Member

mrunge commented May 21, 2024

@jvsg thank you for your PR, it looks good to me.

@mrunge mrunge merged commit 84284d5 into collectd:main May 21, 2024
24 of 25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fix A pull request fixing a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Perl.c fails to compile with clang
3 participants