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

build: link libncurses statically #32959

Merged
merged 1 commit into from Dec 9, 2018

Conversation

Projects
None yet
4 participants
@knz
Copy link
Member

knz commented Dec 9, 2018

(this is a patch authored by @benesch - I am extracting this from #32623 which needs more work)

The Unix world is in the process of upgrading from libncurses ABI 5.0 to
libncurses ABI 6.0. The ABIs are incompatible in both directions, so we
can't support systems that ship libncurses 5.0 and systems that ship
libncurses 6.0 at the same time.

Link libncurses statically to sidestep the issue.

Note that we don't bother vendoring ncurses like we do for our other
C/C++ dependencies, because we don't particularly care about what
version of ncurses we link. Nearly all extant versions of ncurses
(5.0-6.1) are source compatible, and bug fixes in recent releases rarely
affect the low-level terminfo APIs that we depend upon.

Take the opportunity to upgrade to ncurses 6.1 as well. It doesn't fix
any bugs we know of, but it's good to stay up to date.

Fix #32281.

Release note (build change): ncurses is now linked statically so that
the cockroach binary no longer requires a particular version of the
ncurses shared library to be available on deployment machines.

build: link libncurses statically
The Unix world is in the process of upgrading from libncurses ABI 5.0 to
libncurses ABI 6.0. The ABIs are incompatible in both directions, so we
can't support systems that ship libncurses 5.0 and systems that ship
libncurses 6.0 at the same time.

Link libncurses statically to sidestep the issue.

Note that we don't bother vendoring ncurses like we do for our other
C/C++ dependencies, because we don't particularly care about what
version of ncurses we link. Nearly all extant versions of ncurses
(5.0-6.1) are source compatible, and bug fixes in recent releases rarely
affect the low-level terminfo APIs that we depend upon.

Take the opportunity to upgrade to ncurses 6.1 as well. It doesn't fix
any bugs we know of, but it's good to stay up to date.

Fix #32281.

Release note (build change): ncurses is now linked statically so that
the cockroach binary no longer requires a particular version of the
ncurses shared library to be available on deployment machines.

@knz knz requested a review from benesch Dec 9, 2018

@knz knz requested a review from cockroachdb/build-prs as a code owner Dec 9, 2018

@cockroach-teamcity

This comment has been minimized.

Copy link
Member

cockroach-teamcity commented Dec 9, 2018

This change is Reviewable

@knz

This comment has been minimized.

Copy link
Member

knz commented Dec 9, 2018

(forked off #32623)

@knz

This comment has been minimized.

Copy link
Member

knz commented Dec 9, 2018

LGTM (Nikhil is the author)

@knz knz self-assigned this Dec 9, 2018

@knz

This comment has been minimized.

Copy link
Member

knz commented Dec 9, 2018

I am running a smoke test on my laptop and will merge this once it passes.

@knz

This comment has been minimized.

Copy link
Member

knz commented Dec 9, 2018

bors r+

craig bot pushed a commit that referenced this pull request Dec 9, 2018

Merge #32959
32959: build: link libncurses statically r=knz a=knz

The Unix world is in the process of upgrading from libncurses ABI 5.0 to
libncurses ABI 6.0. The ABIs are incompatible in both directions, so we
can't support systems that ship libncurses 5.0 and systems that ship
libncurses 6.0 at the same time.

Link libncurses statically to sidestep the issue.

Note that we don't bother vendoring ncurses like we do for our other
C/C++ dependencies, because we don't particularly care about what
version of ncurses we link. Nearly all extant versions of ncurses
(5.0-6.1) are source compatible, and bug fixes in recent releases rarely
affect the low-level terminfo APIs that we depend upon.

Take the opportunity to upgrade to ncurses 6.1 as well. It doesn't fix
any bugs we know of, but it's good to stay up to date.

Fix #32281.

Release note (build change): ncurses is now linked statically so that
the cockroach binary no longer requires a particular version of the
ncurses shared library to be available on deployment machines.

Co-authored-by: Nikhil Benesch <nikhil.benesch@gmail.com>
@craig

This comment has been minimized.

Copy link

craig bot commented Dec 9, 2018

Build succeeded

@craig craig bot merged commit fd7ebc3 into cockroachdb:master Dec 9, 2018

3 checks passed

GitHub CI (Cockroach) TeamCity build finished
Details
bors Build succeeded
Details
license/cla Contributor License Agreement is signed.
Details

@knz knz deleted the knz:20181209-libedit-link branch Dec 9, 2018

@knz

This comment has been minimized.

Copy link
Member

knz commented Dec 9, 2018

OMG I am a doofus. I broked the builder image with this PR...

@knz

This comment has been minimized.

Copy link
Member

knz commented Dec 9, 2018

Or maybe not. There was redundancy between ncurses.conf and the build flags (--without-shared --without-dlsym). This PR keeps just one, not none as I had feared. All is good.

@vivekmenezes

This comment has been minimized.

Copy link
Contributor

vivekmenezes commented Dec 12, 2018

I'm seeing https://teamcity.cockroachdb.com/viewLog.html?buildId=1050805&tab=buildLog&_focus=8059

fail on inability to fetch a ubuntu resource. Any possible link here

@benesch

This comment has been minimized.

Copy link
Collaborator

benesch commented Dec 12, 2018

Doesn't look like it.

[09:35:23]	[Step 2/2] 		WARNING: Some requests generated warnings:
[09:35:23]	[Step 2/2] 		 - The resource 'projects/ubuntu-os-cloud/global/images/ubuntu-1604-xenial-v20181030' is deprecated. A suggested replacement is 'projects/ubuntu-os-cloud/global/images/ubuntu-1604-xenial-v20181204'.
[09:35:23]	[Step 2/2] 		
[09:35:23]	[Step 2/2] 		ERROR: (gcloud.compute.instances.create) Could not fetch resource:
[09:35:23]	[Step 2/2] 		 - Code: '57CCFDCDED9A1.AB01490.B40D7D07'
[09:35:23]	[Step 2/2] 		
[09:35:23]	[Step 2/2] 		: exit status 1
[09:35:23]	[Step 2/2] 		Cleaning up...
[09:35:23]	[Step 2/2] 		: exit status 1

The notice about the Ubuntu image is just a warning. The real error appears to be some inscrutable GCE problem that I'd chalk up as a transient, one-time thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment