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

configure.ac: Do s/-pthreads -lpthreads/-pthreads/ for protobuf #733

Merged
merged 1 commit into from
Mar 24, 2016

Conversation

cgull
Copy link
Member

@cgull cgull commented Mar 19, 2016

protobuf uses an obsolete automake pthreads detection macro,
which results in "pkgconfig --libs protobuf" returning
"-lprotobuf -pthread -lpthread" on Linux. Remove
the unnecessary and dangerous -lpthread in that case.

Fixes #727, mosh-server crash in libutempter on Debian Sid.

protobuf uses an obsolete automake pthreads detection macro,
which results in "pkgconfig --libs protobuf" returning
"-lprotobuf -pthread -lpthread" on Linux.  Remove
the unnecessary and dangerous -lpthread in that case.

Fixes mobile-shell#727, mosh-server crash in libutempter on Debian Sid.
@keithw
Copy link
Member

keithw commented Mar 19, 2016

This looks like the right fix, at least for now (although if the bug is in glibc, I hope we can get them to fix it!).

What did you mean by "an obsolete automake pthreads detection macro"? It's protobuf that is shipping that .pc file and specifying Libs: -L${libdir} -lprotobuf -pthread -lpthread. Or are you saying .pc files themselves are obsolete?

@keithw
Copy link
Member

keithw commented Mar 19, 2016

(And thanks for doing all that spelunking! Worthy of a KON and BAL column.)

@cgull
Copy link
Member Author

cgull commented Mar 19, 2016

The protobuf configure.ac uses an ancient ax_pthread.m4 macro, which Autoconf obsoleted and renamed to acx_pthread.m4 back in 2009. Google's made a few updates to their file, GNU Autoconf...a lot more. The variables that macro sets are used to substitute protobuf.pc.

Swapping in acx_pthread.m4 makes protobuf.pc more correct.

I will loudly kvetch that autoconf is a pile of accreted...stuff that seems to hinder portability as much as it helps; having the .m4 files be embedded by autoconf users rather than distributed with autoconf doesn't exactly help maintainability. It won't do any good, the OSS world is kind of stuck with it now.

@ghostbar
Copy link

Would love to see this merged; traveling without a working mosh sucks big time.

@cgull
Copy link
Member Author

cgull commented Mar 19, 2016

I'm going to wait till Monday for comment here and other places.

Since Sid is an unstable code base, I think this workaround is fine for now:
mosh --server="LD_PRELOAD=/usr/lib/x86_64-linux-gnu/libutempter.so.0 /usr/bin/mosh-server" localhost

@ghostbar
Copy link

@cgull thank you! I had forgotten about the --server flag :)

@cgull cgull merged commit a47917b into mobile-shell:master Mar 24, 2016
@cgull cgull deleted the protobuf-pthread-727 branch November 14, 2016 05:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants