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

bpo-38634: Allow non-apple build to cope with libedit #16986

Merged
merged 1 commit into from
Dec 4, 2019
Merged

bpo-38634: Allow non-apple build to cope with libedit #16986

merged 1 commit into from
Dec 4, 2019

Conversation

serge-sans-paille
Copy link
Contributor

@serge-sans-paille serge-sans-paille commented Oct 29, 2019 •
edited by bedevere-bot
Loading

If Python is used as a library by a binary linking to libedit, the linker
resolves the rl_initialize symbol required by the readline module against
libedit instead of libreadline, which leads to a segfault.

Take advantage of the existing supporting code to have readline module being
compatible with both situations.

https://bugs.python.org/issue38634

@serge-sans-paille
Copy link
Contributor Author

cc @vstinner

@ned-deily
Copy link
Member

ned-deily commented Oct 29, 2019 •
edited by bedevere-bot
Loading

Thanks for this PR. There is a long standing open issue (bpo-13501) to generalize libedit support in Python including a PR (#12076) that is more involved than just removing the APPLE conditional code as proposed here. libedit itself has changed over the years so perhaps not all of the changes in #12076 need to be made. But we should probably start from there. Assigning to @gpshead who last looked at bpo-13501.

vstinner
vstinner reviewed Dec 4, 2019
View reviewed changes
Modules/readline.c Outdated Show resolved Hide resolved
@serge-sans-paille
Allow non-apple build to cope with libedit
b547667 
If Python is used as a library by a binary linking to libedit, the linker
resolves the rl_initialize symbol required by the readline module against
libedit instead of libreadline, which leads to a segfault.

Take advantage of the existing supporting code to have readline module being
compatible with both situations.
vstinner
vstinner approved these changes Dec 4, 2019
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. I didn't check manually, but I trust @serge-sans-paille for testing it properly :-)

@vstinner vstinner merged commit 7105319 into python:master Dec 4, 2019
@miss-islington
Copy link
Contributor

Thanks @serge-sans-paille for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Thanks @serge-sans-paille for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Sorry @serge-sans-paille and @vstinner, I had trouble checking out the 3.7 backport branch.
Please backport using cherry_picker on command line.
cherry_picker 7105319ada2e663659020cbe9fdf7ff38f421ab2 3.7

@miss-islington miss-islington assigned vstinner and unassigned gpshead Dec 4, 2019
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 4, 2019
@serge-sans-paille @miss-islington
bpo-38634: Allow non-apple build to cope with libedit (pythonGH-16986)
95b22f7 
The readline module now detects if Python is linked to libedit at runtime
on all platforms.  Previously, the check was only done on macOS.

If Python is used as a library by a binary linking to libedit, the linker
resolves the rl_initialize symbol required by the readline module against
libedit instead of libreadline, which leads to a segfault.

Take advantage of the existing supporting code to have readline module being
compatible with both situations.
(cherry picked from commit 7105319)

Co-authored-by: serge-sans-paille <[email protected]>
@bedevere-bot
Copy link

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

@vstinner
Copy link
Member

vstinner commented Dec 4, 2019

I did the 3.8 since it was possible to automate it. I'm not interested to backport it to 3.7, since it's too much effort for a rare use case.

miss-islington added a commit that referenced this pull request Dec 4, 2019
@miss-islington @serge-sans-paille
bpo-38634: Allow non-apple build to cope with libedit (GH-16986)
68669ef 
The readline module now detects if Python is linked to libedit at runtime
on all platforms.  Previously, the check was only done on macOS.

If Python is used as a library by a binary linking to libedit, the linker
resolves the rl_initialize symbol required by the readline module against
libedit instead of libreadline, which leads to a segfault.

Take advantage of the existing supporting code to have readline module being
compatible with both situations.
(cherry picked from commit 7105319)

Co-authored-by: serge-sans-paille <[email protected]>
jacobneiltaylor pushed a commit to jacobneiltaylor/cpython that referenced this pull request Dec 5, 2019
@serge-sans-paille
bpo-38634: Allow non-apple build to cope with libedit (pythonGH-16986)
bdc86ca 
The readline module now detects if Python is linked to libedit at runtime
on all platforms.  Previously, the check was only done on macOS.

If Python is used as a library by a binary linking to libedit, the linker
resolves the rl_initialize symbol required by the readline module against
libedit instead of libreadline, which leads to a segfault.

Take advantage of the existing supporting code to have readline module being
compatible with both situations.
shihai1991 pushed a commit to shihai1991/cpython that referenced this pull request Jan 31, 2020
@serge-sans-paille @shihai1991
bpo-38634: Allow non-apple build to cope with libedit (pythonGH-16986)
7757d2e 
The readline module now detects if Python is linked to libedit at runtime
on all platforms.  Previously, the check was only done on macOS.

If Python is used as a library by a binary linking to libedit, the linker
resolves the rl_initialize symbol required by the readline module against
libedit instead of libreadline, which leads to a segfault.

Take advantage of the existing supporting code to have readline module being
compatible with both situations.
rohieb added a commit to rohieb/cpython that referenced this pull request Jan 28, 2021
@rohieb
bpo-13501: allow choosing between readline and libedit
394d3a7 
In contrast to macOS, libedit is available as its own include file and
library on Linux systems to prevent file name clashes. So if both
libraries are available on the system, readline is currently chosen by
default; and if only libedit is available, it is not found at all. This
patch adds a way to link against libedit by adding the following
arguments to configure:

  --with-readline           link against libreadline (the default)
  --with-readline=editline  link against libeditline
  --with-readline=no        disable building the readline module
  --without-readline        (same)

The runtime detection of libedit vs. readline was already done in commit
7105319 (2019-12-04, serge-sans-paille: "bpo-38634: Allow
non-apple build to cope with libedit (pythonGH-16986)").

Fixes: pythonGH-12076 ("bpo-13501 Build or disable readline with Editline")
Fixes: bpo-13501 ("Make libedit support more generic; port readline / libedit to FreeBSD")
Co-authored-by: Enji Cooper (ngie-eign)
Co-authored-by: Martin Panter (vadmium)
Co-authored-by: Robert Marshall (kellinm)
gpshead pushed a commit that referenced this pull request Feb 9, 2021
@rohieb
bpo-13501: allow choosing between readline and libedit (GH-24189)
e1f7769 
In contrast to macOS, libedit is available as its own include file and
library on Linux systems to prevent file name clashes. So if both
libraries are available on the system, readline is currently chosen by
default; and if only libedit is available, it is not found at all. This
patch adds a way to link against libedit by adding the following
arguments to configure:

  --with-readline           link against libreadline (the default)
  --with-readline=editline  link against libeditline
  --with-readline=no        disable building the readline module
  --without-readline        (same)

The runtime detection of libedit vs. readline was already done in commit
7105319 (2019-12-04, serge-sans-paille: "bpo-38634: Allow
non-apple build to cope with libedit (GH-16986)").

Fixes: GH-12076 ("bpo-13501 Build or disable readline with Editline")
Fixes: bpo-13501 ("Make libedit support more generic; port readline / libedit to FreeBSD")
Co-authored-by: Enji Cooper (ngie-eign)
Co-authored-by: Martin Panter (vadmium)
Co-authored-by: Robert Marshall (kellinm)
adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 13, 2021
@rohieb @adorilson
bpo-13501: allow choosing between readline and libedit (pythonGH-24189)
a050fcd 
In contrast to macOS, libedit is available as its own include file and
library on Linux systems to prevent file name clashes. So if both
libraries are available on the system, readline is currently chosen by
default; and if only libedit is available, it is not found at all. This
patch adds a way to link against libedit by adding the following
arguments to configure:

  --with-readline           link against libreadline (the default)
  --with-readline=editline  link against libeditline
  --with-readline=no        disable building the readline module
  --without-readline        (same)

The runtime detection of libedit vs. readline was already done in commit
7105319 (2019-12-04, serge-sans-paille: "bpo-38634: Allow
non-apple build to cope with libedit (pythonGH-16986)").

Fixes: pythonGH-12076 ("bpo-13501 Build or disable readline with Editline")
Fixes: bpo-13501 ("Make libedit support more generic; port readline / libedit to FreeBSD")
Co-authored-by: Enji Cooper (ngie-eign)
Co-authored-by: Martin Panter (vadmium)
Co-authored-by: Robert Marshall (kellinm)
tnir pushed a commit to tnir/cpython that referenced this pull request Apr 15, 2021
@rohieb @tnir
bpo-13501: allow choosing between readline and libedit
0b989c6 
In contrast to macOS, libedit is available as its own include file and
library on Linux systems to prevent file name clashes. So if both
libraries are available on the system, readline is currently chosen by
default; and if only libedit is available, it is not found at all. This
patch adds a way to link against libedit by adding the following
arguments to configure:

  --with-readline           link against libreadline (the default)
  --with-readline=editline  link against libeditline
  --with-readline=no        disable building the readline module
  --without-readline        (same)

The runtime detection of libedit vs. readline was already done in commit
7105319 (2019-12-04, serge-sans-paille: "bpo-38634: Allow
non-apple build to cope with libedit (pythonGH-16986)").

Fixes: pythonGH-12076 ("bpo-13501 Build or disable readline with Editline")
Fixes: bpo-13501 ("Make libedit support more generic; port readline / libedit to FreeBSD")
Co-authored-by: Enji Cooper (ngie-eign)
Co-authored-by: Martin Panter (vadmium)
Co-authored-by: Robert Marshall (kellinm)
@tnir tnir mentioned this pull request Apr 15, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@labath labath labath left review comments

@vstinner vstinner vstinner approved these changes

Assignees

@vstinner vstinner

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@serge-sans-paille @ned-deily @miss-islington @bedevere-bot @vstinner @labath @gpshead @the-knights-who-say-ni