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

meson: make backtrace dependency on execinfo #3276

Merged
merged 1 commit into from
Dec 14, 2022
Merged

Conversation

neheb
Copy link
Contributor

@neheb neheb commented Sep 28, 2022

musl libc for example has no such header.

Signed-off-by: Rosen Penev rosenp@gmail.com

musl libc for example has no such header.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
Copy link
Contributor

@terrelln terrelln left a comment

Choose a reason for hiding this comment

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

This could enable backtraces when use_backtrace is false. Otherwise it looks good to me.

build/meson/programs/meson.build Show resolved Hide resolved
@eli-schwartz
Copy link
Contributor

use_backtrace cannot be false anymore, it was changed away from a boolean.

It's now a feature option with the default value of "disabled" which means that when used in the required: kwarg, it logs that it is skipping the check "because feature backtrace is disabled".

@terrelln
Copy link
Contributor

@eli-schwartz Thanks for the info! I checked out the source code and it looks like you're correct.

I previously looked at the documentation for has_header(), and I saw that:

required: If set to true, Meson will halt if the header check fails.

This seems to me that if required = disabled then execinfo.found() could still return true. So it seems the documentation is out of date. Likely it wasn't updated when required was changed to accept a feature.

Copy link
Contributor

@terrelln terrelln left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

@terrelln terrelln merged commit 031de3c into facebook:dev Dec 14, 2022
@neheb neheb deleted the mess branch December 14, 2022 23:44
@eli-schwartz
Copy link
Contributor

So it seems the documentation is out of date. Likely it wasn't updated when required was changed to accept a feature.

Actually, it's not described anywhere else it is accepted either.

I've reworded the docs slightly, a new version of the website is in progress.

@Cyan4973 Cyan4973 mentioned this pull request Feb 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants