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

cli: bump the libedit dep and fix linking isues #24513

Merged
merged 1 commit into from
Apr 5, 2018

Conversation

knz
Copy link
Contributor

@knz knz commented Apr 5, 2018

Prior to this patch, the underlying go-libedit dependency would set
LDFLAGS to link on linux against libtinfo, mistakenly assuming that
this would cause the resulting Go binary to only use the subset of
whatever tinfo implementation on linux system dedicated to terminal
access.

The mistake is that there is no such thing -- there is no standad
standalone libtinfo on linux, and on most linux system it is either
a symlink to libncurses, or doesn't exist at all!

This patch corrects the issue by upgrading to the newest go-libedit
code which links against libncurses instead, which is guaranteed to
exist (it's been part of the Linux Standard Base forever).

The builder docker image is upgraded accordingly.

Release note (general change): Prevent execution errors reporting a
missing libtinfo.so.5 on linux systems.

Fixes #24492.

@knz knz requested review from benesch and a team April 5, 2018 16:47
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@knz
Copy link
Contributor Author

knz commented Apr 5, 2018

I am in the process of updating the builder image. Will update the PR when that completes.

@bdarnell
Copy link
Contributor

bdarnell commented Apr 5, 2018

:lgtm:


Review status: 0 of 3 files reviewed at latest revision, all discussions resolved, some commit checks pending.


Comments from Reviewable

Copy link
Contributor

@a-robinson a-robinson left a comment

Choose a reason for hiding this comment

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

Thanks @knz! I assume you verified this works on coreos?

@knz
Copy link
Contributor Author

knz commented Apr 5, 2018

I need to complete the builder image before I complete the coreos testing. Builder image still building. It takes a while.

Prior to this patch, the underlying `go-libedit` dependency would set
LDFLAGS to link on linux against libtinfo, mistakenly assuming that
this would cause the resulting Go binary to only use the subset of
whatever tinfo implementation on linux system dedicated to terminal
access.

The mistake is that there is no such thing -- there is no standad
standalone libtinfo on linux, and on most linux system it is either
a symlink to libncurses, or doesn't exist at all!

This patch corrects the issue by upgrading to the newest go-libedit
code which links against libncurses instead, which is guaranteed to
exist (it's been part of the Linux Standard Base forever).

The builder docker image is upgraded accordingly.

Release note (general change): Prevent execution errors reporting a
missing `libtinfo.so.5` on linux systems.
@knz
Copy link
Contributor Author

knz commented Apr 5, 2018

Updated builder image. Will iterate until CI is green then test coreos.

@knz
Copy link
Contributor Author

knz commented Apr 5, 2018

core@kena-coreos ~ $ ldd cockroach-linux-2.6.32-gnu-amd64
        linux-vdso.so.1 (0x00007ffe1bbe7000)
        librt.so.1 => /lib64/librt.so.1 (0x00007f8bc43cd000)
        libpthread.so.0 => /lib64/libpthread.so.0 (0x00007f8bc41b0000)
        libm.so.6 => /lib64/libm.so.6 (0x00007f8bc3eac000)
        libncurses.so.5 => /lib64/libncurses.so.5 (0x00007f8bc3c51000)
        libc.so.6 => /lib64/libc.so.6 (0x00007f8bc38aa000)
        /lib64/ld-linux-x86-64.so.2 (0x00007f8bc45d5000)

It works all right on coreos -- was able to start server, client, use in docker etc.

@knz
Copy link
Contributor Author

knz commented Apr 5, 2018

bors r+

craig bot added a commit that referenced this pull request Apr 5, 2018
24513: cli: bump the libedit dep and fix linking isues r=knz a=knz

Prior to this patch, the underlying `go-libedit` dependency would set
LDFLAGS to link on linux against libtinfo, mistakenly assuming that
this would cause the resulting Go binary to only use the subset of
whatever tinfo implementation on linux system dedicated to terminal
access.

The mistake is that there is no such thing -- there is no standad
standalone libtinfo on linux, and on most linux system it is either
a symlink to libncurses, or doesn't exist at all!

This patch corrects the issue by upgrading to the newest go-libedit
code which links against libncurses instead, which is guaranteed to
exist (it's been part of the Linux Standard Base forever).

The builder docker image is upgraded accordingly.

Release note (general change): Prevent execution errors reporting a
missing `libtinfo.so.5` on linux systems.

Fixes #24492.
@craig
Copy link
Contributor

craig bot commented Apr 5, 2018

Build succeeded

craig bot added a commit that referenced this pull request Apr 5, 2018
24531: cherry-pick 2.0: cli: bump the libedit dep and fix linking issues r=knz a=knz

Picks #24513.

cc @cockroachdb/release
@craig craig bot merged commit 63dfbd8 into cockroachdb:master Apr 5, 2018
@knz knz deleted the 20180405-libedit branch April 5, 2018 22:16
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.

None yet

5 participants