Skip to content

bpo-44394: Ensure libexpat is linked against libm #28617

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.

Sign up for GitHub

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

Jump to bottom
Merged
pablogsal merged 2 commits into from
Sep 29, 2021
Merged

bpo-44394: Ensure libexpat is linked against libm #28617

pablogsal merged 2 commits into from
Sep 29, 2021

Conversation

@pablogsal
Copy link
Member

@pablogsal pablogsal commented Sep 29, 2021 •
edited
Loading

https://bugs.python.org/issue44394

We need to link against libm to have isnan. See libexpat/libexpat#510

vstinner
vstinner reviewed Sep 29, 2021
View reviewed changes
setup.py
]
extra_compile_args = []
expat_lib = []
expat_lib = ['m']
Copy link
Member

@vstinner vstinner Sep 29, 2021

Choose a reason for hiding this comment

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

Would you mind to add a comment mentioning "bpo-44394: " and explaining that libexpact uses isnan() of math.h?

Copy link
Member Author

@pablogsal pablogsal Sep 29, 2021

Choose a reason for hiding this comment

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

Done

vstinner
vstinner approved these changes Sep 29, 2021
View reviewed changes
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM.

vstinner
vstinner reviewed Sep 29, 2021
View reviewed changes
setup.py
extra_compile_args = []
expat_lib = []
# bpo-44394: libexpact uses isnan() of math.h and needs linkage
# against the libm
Copy link
Member

@vstinner vstinner Sep 29, 2021

Choose a reason for hiding this comment

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

It seems like it's a recent libexpact change, in version 2.4.0: libexpat/libexpat#511

I don't know if it's worth mentioning it.

Copy link
Member Author

@pablogsal pablogsal Sep 29, 2021

Choose a reason for hiding this comment

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

Nah, I think is good since is the one we use currently in all branches

@pablogsal pablogsal merged commit 6c1154b into python:main Sep 29, 2021
@pablogsal pablogsal deleted the bpo-44394 branch September 29, 2021 13:18
@miss-islington
Copy link
Contributor

miss-islington commented Sep 29, 2021

Thanks @pablogsal for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8, 3.9, 3.10.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

miss-islington commented Sep 29, 2021

Sorry @pablogsal, I had trouble checking out the 3.10 backport branch.
Please backport using cherry_picker on command line.
cherry_picker 6c1154b9de29e1c9cd3d05f5289543e5cff73895 3.10

@bedevere-bot
Copy link

bedevere-bot commented Sep 29, 2021

GH-28619 is a backport of this pull request to the 3.9 branch.

@bedevere-bot
Copy link

bedevere-bot commented Sep 29, 2021

GH-28620 is a backport of this pull request to the 3.8 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 29, 2021
@pablogsal @miss-islington
bpo-44394: Ensure libexpat is linked against libm (pythonGH-28617)
545f5d5 
(cherry picked from commit 6c1154b)

Co-authored-by: Pablo Galindo Salgado <[email protected]>
@pablogsal pablogsal added needs backport to 3.10 only security fixes and removed needs backport to 3.10 only security fixes labels Sep 29, 2021
@miss-islington
Copy link
Contributor

miss-islington commented Sep 29, 2021

Thanks @pablogsal for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖

@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Sep 29, 2021
@bedevere-bot
Copy link

bedevere-bot commented Sep 29, 2021

GH-28621 is a backport of this pull request to the 3.10 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 29, 2021
@pablogsal @miss-islington
bpo-44394: Ensure libexpat is linked against libm (pythonGH-28617)
0c68674 
(cherry picked from commit 6c1154b)

Co-authored-by: Pablo Galindo Salgado <[email protected]>
miss-islington added a commit that referenced this pull request Sep 29, 2021
@miss-islington @pablogsal
bpo-44394: Ensure libexpat is linked against libm (GH-28617)
fafa213 
(cherry picked from commit 6c1154b)

Co-authored-by: Pablo Galindo Salgado <[email protected]>
@ambv ambv mentioned this pull request Sep 29, 2021
Merged
ambv pushed a commit to miss-islington/cpython that referenced this pull request Sep 29, 2021
@pablogsal @ambv
bpo-44394: Ensure libexpat is linked against libm (pythonGH-28617)
f9230e9 
(cherry picked from commit 6c1154b)

Co-authored-by: Pablo Galindo Salgado <[email protected]>
ambv added a commit that referenced this pull request Sep 29, 2021
@miss-islington @pablogsal @ambv
[3.10] bpo-44394: Ensure libexpat is linked against libm (GH-28617) (G…
412ae8a 
…H-28621)

* bpo-44394: Ensure libexpat is linked against libm (GH-28617)
(cherry picked from commit 6c1154b)

Co-authored-by: Pablo Galindo Salgado <[email protected]>
Co-authored-by: Łukasz Langa <[email protected]>
ambv added a commit that referenced this pull request Sep 29, 2021
@miss-islington @pablogsal @ambv
[3.8] bpo-44394: Ensure libexpat is linked against libm (GH-28617) (G…
90004fc 
…H-28620)

(cherry picked from commit 6c1154b)

Co-authored-by: Pablo Galindo Salgado <[email protected]>
Co-authored-by: Łukasz Langa <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinner vstinner vstinner approved these changes

Assignees

@pablogsal pablogsal

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@pablogsal @miss-islington @bedevere-bot @vstinner @the-knights-who-say-ni