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

fix #251: adjust ostringstream constructor for recent libstdc++ #252

Merged
merged 2 commits into from
Apr 11, 2020

Conversation

benlorenz
Copy link
Member

@benlorenz benlorenz commented Apr 10, 2020

Better fix this time, just 4 characters. As it turns out, only the default constructor for ostringstream requires GLIBCXX_3.4.26, we can just use an empty string instead.

---- obsolete first try:
@fingolfin @kalmarek

Unfortunately just dlopening a new libstdc++.so.6.0.26 doesn't help because the DT_NEEDED of libpolymake.so is just libstdc++.so.6 which will still resolve to the old one from julia which is also in memory.

But since we love having more and more dependencies we can patch the library with patchelf to use the full filename of the correct libstdc++. I tried looking for options to gcc to do something similar at link time but that seems impossible.

PS: all of the above applies only to Sys.islinux()

@benlorenz
Copy link
Member Author

benlorenz commented Apr 10, 2020

edit: please ignore

Unfortunately adding this new libstdc++ broke exactly the show method mentioned in #251

Polymake.Vector{Int64}: Test Failed at /home/runner/work/Polymake.jl/Polymake.jl/test/vectors.jl:88
  Expression: string(V) == "pm::Vector<long>\n5 10 3"
   Evaluated: "pm::Vector<long>\n" == "pm::Vector<long>\n5 10 3"

it turns out only the default constructor for ostringstream requires GLIBCXX_3.4.26 but not the one for the empty string
@benlorenz benlorenz changed the title fix #251: add CompilerSupportLibraries for recent libstdc++ fix #251: adjust ostringstream constructor for recent libstdc++ Apr 10, 2020
fingolfin
fingolfin previously approved these changes Apr 11, 2020
Copy link
Member

@fingolfin fingolfin left a comment

Choose a reason for hiding this comment

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

nice!

@kalmarek
Copy link
Contributor

@benlorenz I tried this and see no adverse effects:

  • strings libpolymake.so | fgrep GLIBCXX_3.4.26 | c++filt is empty
  • Polymake.Vector([1,2,3]) prints correctly (and all the tests pass).

kalmarek
kalmarek previously approved these changes Apr 11, 2020
@benlorenz benlorenz dismissed stale reviews from kalmarek and fingolfin via 1cc1205 April 11, 2020 10:39
@benlorenz
Copy link
Member Author

The print bug was with my previous try where I pulled in the new libstdc++.

I added a version bump so we can directly make a new release if needed, feel free to merge this as soon as the tests are done.

@kalmarek kalmarek merged commit f84f4db into master Apr 11, 2020
@benlorenz benlorenz deleted the issue/251/libstdcxx branch April 14, 2020 10:13
kalmarek added a commit to kalmarek/libcxxwrap-julia that referenced this pull request Jun 30, 2020
barche pushed a commit to JuliaInterop/libcxxwrap-julia that referenced this pull request Sep 4, 2020
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