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.

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

[3.6] bpo-35257: Avoid leaking LTO linker flags into distutils (GH-10900) #11265

Merged
merged 3 commits into from
Dec 20, 2018
Merged

[3.6] bpo-35257: Avoid leaking LTO linker flags into distutils (GH-10900) #11265

merged 3 commits into from
Dec 20, 2018

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Dec 20, 2018 •
edited by bedevere-bot
Loading

vstinner and others added 2 commits December 20, 2018 15:10
@vstinner
bpo-35499: make profile-opt don't override CFLAGS_NODIST (GH-11164)
4af6fe7 
"make profile-opt" no longer replaces CFLAGS_NODIST with CFLAGS. It
now adds profile-guided optimization (PGO) flags to CFLAGS_NODIST,
existing CFLAGS_NODIST flags are kept.

(cherry picked from commit 640ed52)
@stratakis @vstinner
bpo-35257: Avoid leaking LTO linker flags into distutils (GH-10900)
f6a10d4 
When compiling 3rd party C extensions, the linker flags used by the
compiler for the interpreter and the stdlib modules, will get
leaked into distutils. In order to avoid that, the PY_CORE_LDFLAGS
and PY_LDFLAGS_NODIST are introduced to keep those flags separated.

(cherry picked from commit cf10a75)
@vstinner
Copy link
Member Author

I tested manually this PR: it passed all my tests, https://bugs.python.org/issue35257#msg332234

@ned-deily: This PR fixes a regression introduced in the 3.6 branch after 3.6.8. I would like to merge it.

@vstinner
Copy link
Member Author

cc @stratakis

@ned-deily
Copy link
Member

Why did you combine the PRs from two separate issues into one? It would make things much easier to track if you kept them separate, no?

@vstinner
Copy link
Member Author

Why did you combine the PRs from two separate issues into one? It would make things much easier to track if you kept them separate, no?

The first commit isn't important, it's a minor change. But the second commit depends on the first commit. Otherwise, I would get even more conflicts on the backport. So I decided to combine both commits.

@ned-deily
Copy link
Member

Please separate them. I spent a couple of hours earlier trying to decipher and document the tangled mess of the several bpo issues and PRs involved in this sequence over the various branches. Combining these just makes the mess more complicated.

@vstinner
Copy link
Member Author

I created PR #11267 for the first change.

@ned-deily ned-deily merged commit a21bedf into python:3.6 Dec 20, 2018
@vstinner vstinner deleted the ldflags_nodist36 branch December 20, 2018 20:45
@vstinner
Copy link
Member Author

@ned-deily: Oh you succeeded to merge it, thanks.

I was trying to create a PR to the second change, but there was a conflict since you already merged this PR :-)

Cool, I'm happy to see this bug fixed in 3.6 as well ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@vstinner @ned-deily @the-knights-who-say-ni @bedevere-bot @stratakis