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

Tighten warning for 'Requires.private' in simple pkg-config parser #26008

Merged
merged 1 commit into from
Sep 30, 2024

Conversation

jabraham17
Copy link
Member

@jabraham17 jabraham17 commented Sep 26, 2024

Tightens up the logic for when we should warn about 'Requires.private' (and also 'Requires'). We only need to warn when the compiler/linker flags are not already handled by 'Cflags'/Libs.

Details:

Requires/Requires.private list what other pkg-config libraries are needed, so that pkg-config can pull in the need dependencies/flags. This ends up being a recursive operation (i.e. 'A' has 'Requires: B', so we must process 'B.pc'). But if we already have the information needed, this is unnecessary. So if we have 'Requires: B', but 'B' is already listed in 'Libs', then for our purposes we have satisfied the 'Requires.'

Our simple pkg-config parser is used only for bundled third-party libs, and we do not need to ability to recursively search other .pc files to satisfy the requirements. So this PR uses the above logic to avoid warning about Requires/Requires.private not being handled

This should resolve #26002

[Reviewed by @mppf and @jhh67]

Copy link
Contributor

@jhh67 jhh67 left a comment

Choose a reason for hiding this comment

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

Very nice. Thanks for diagnosing the problem and coming up with a solution.

static
and d.get("Requires.private")
and _pkgconfig_should_warn_for_requires(d, private=True)
):
Copy link
Member

Choose a reason for hiding this comment

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

Trivial, but seems like really weird formatting. I would expect this:

if (static
    and d.get("Requires.private")
    and _pkgconfig_should_warn_for_requires(d, private=True)):

(But if there is some Python style convention that applies in this case, I wouldn't know about it).

Copy link
Member Author

Choose a reason for hiding this comment

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

I used black to format the code, but I think both formattings match the PEP8 style guide

Copy link
Member

Choose a reason for hiding this comment

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

Alright! Thanks for checking.

@jabraham17 jabraham17 merged commit 8f702bf into chapel-lang:main Sep 30, 2024
7 checks passed
@jabraham17 jabraham17 deleted the tighten-warning branch September 30, 2024 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extra warning about Requires.private in builds using HWLOC_PCI=enable
3 participants