Beep6581 / RawTherapee

A powerful cross-platform raw photo processing program
https://rawtherapee.com
GNU General Public License v3.0
2.79k stars 316 forks source link

Compilation failure in master - undefined reference to `Glib::ustring::ustring(std::string const&)' #3024

Closed Beep6581 closed 7 years ago

Beep6581 commented 8 years ago

Compilation of latest master fails for Partha in Windows 10/MSYS.

CMakeFiles/rth.dir/objects.a(editwindow.cc.obj):editwindow.cc:(.text+0x44f): undefined reference to 'Glib::ustring::ustring(std::string const&)'

Using: GCC-5.1.0 cmake - 2.86 glib - 2.47.4 glibmm-2.4 - 2.46.3 gtk-2.0 - 2.24.28

Forum: http://rawtherapee.com/forum/viewtopic.php?f=10&t=6340

adamreichold commented 8 years ago

Could this be -D_GLIBCXX_USE_CXX11_ABI=0 missing, i.e. GCC 5.1 will default to the C++11 ABI, but the dependencies might not be built using it? std::string is one of the parts of the standard library that changed.

mks9900 commented 8 years ago

I tried to compile RT (latest master) on gentoo with gcc-5.3 and it fails similary: http://pastebin.com/tyJqjmG6 Perhaps it can be of some help?

2015-12-18 20:57 GMT+01:00 adamreichold notifications@github.com:

Could this be -D_GLIBCXX_USE_CXX11_ABI=0 missing, i.e. GCC 5.1 will default to the C++11 ABI, but the dependencies might not be built using it? std::string is one of the parts of the standard library that changed.

— Reply to this email directly or view it on GitHub https://github.com/Beep6581/RawTherapee/issues/3024#issuecomment-165884467 .

adamreichold commented 8 years ago

As

undefined reference to `Glib::ustring::ustring(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)'

from the Gentoo build points out, this is definitely the C++11 ABI, i.e. RT is built using the C++11 ABI but glib is not. Please try adding

add_definitions (-D_GLIBCXX_USE_CXX11_ABI=0)

to CMakeLists.txt.

mks9900 commented 8 years ago

Adam, getting too advanced quickly here :-) Where in that file should I add that text? Simply adding it last in that file got me the same result.

2015-12-18 21:27 GMT+01:00 adamreichold notifications@github.com:

As

undefined reference to `Glib::ustring::ustring(std::__cxx11::basic_string<char, std::char_traits, std::allocator > const&)'

from the Gentoo build points out, this is definitely the C++11 ABI, i.e. RT is build using the C++11 ABI but glib is not. Please try adding

add_definitions (-D_GLIBCXX_USE_CXX11_ABI=0)

to CMakeLists.txt.

— Reply to this email directly or view it on GitHub https://github.com/Beep6581/RawTherapee/issues/3024#issuecomment-165891106 .

mks9900 commented 8 years ago

(And of course: welcome and very good to have you on board! /Best regards, Johan)

2015-12-18 21:40 GMT+01:00 Johan Thor johan@birkagatan.com:

Adam, getting too advanced quickly here :-) Where in that file should I add that text? Simply adding it last in that file got me the same result.

2015-12-18 21:27 GMT+01:00 adamreichold notifications@github.com:

As

undefined reference to `Glib::ustring::ustring(std::__cxx11::basic_string<char, std::char_traits, std::allocator > const&)'

from the Gentoo build points out, this is definitely the C++11 ABI, i.e. RT is build using the C++11 ABI but glib is not. Please try adding

add_definitions (-D_GLIBCXX_USE_CXX11_ABI=0)

to CMakeLists.txt.

— Reply to this email directly or view it on GitHub https://github.com/Beep6581/RawTherapee/issues/3024#issuecomment-165891106 .

adamreichold commented 8 years ago

Where in that file should I add that text? Simply adding it last in that file got me the same result.

You should already find that directive in line 21 of CMakeLists.txt, but within an if clause restricting it to Windows. It should be enough to just move it outside of the if clause, e.g. to line 24 like

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 6153484..f9c5567 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -18,10 +18,10 @@ if (WIN32 AND CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
   execute_process(COMMAND ${CMAKE_C_COMPILER} -dumpversion OUTPUT_VARIABLE GCC_VERSION)
   if (GCC_VERSION VERSION_GREATER 5.0 OR GCC_VERSION VERSION_EQUAL 5.0)
     message(STATUS "Gcc Version >= 5.0 ; adding -D_GLIBCXX_USE_CXX11_ABI=0 to build with Gtk2")
-    add_definitions (-D_GLIBCXX_USE_CXX11_ABI=0)
    # see here : https://gcc.gnu.org/gcc-5/changes.html#libstdcxx
   endif()
 endif()
+add_definitions (-D_GLIBCXX_USE_CXX11_ABI=0)

 if (UPPER_CMAKE_BUILD_TYPE STREQUAL "DEBUG")
   add_definitions (-D_DEBUG)
mks9900 commented 8 years ago

I made a guess before your answer and added it to the Openmp if clause, after line 279 since I use that, and it worked! So thanks again Adam! :-)

2015-12-18 21:43 GMT+01:00 adamreichold notifications@github.com:

Where in that file should I add that text? Simply adding it last in that file got me the same result.

You should already find that directive in line 21 of CMakeLists.txt, but within an if clause restricting it to Windows. It should be enough to just move it outside of the if clause, e.g. to line 24 like

diff --git a/CMakeLists.txt b/CMakeLists.txt index 6153484..f9c5567 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -18,10 +18,10 @@ if (WIN32 AND CMAKE_CXX_COMPILER_ID STREQUAL "GNU") execute_process(COMMAND ${CMAKE_C_COMPILER} -dumpversion OUTPUT_VARIABLE GCC_VERSION) if (GCC_VERSION VERSION_GREATER 5.0 OR GCC_VERSION VERSION_EQUAL 5.0) message(STATUS "Gcc Version >= 5.0 ; adding -D_GLIBCXX_USE_CXX11_ABI=0 to build with Gtk2")

  • add_definitions (-D_GLIBCXX_USE_CXX11_ABI=0)

    see here : https://gcc.gnu.org/gcc-5/changes.html#libstdcxx

    endif() endif() +add_definitions (-D_GLIBCXX_USE_CXX11_ABI=0)

    if (UPPER_CMAKE_BUILD_TYPE STREQUAL "DEBUG") add_definitions (-D_DEBUG)

— Reply to this email directly or view it on GitHub https://github.com/Beep6581/RawTherapee/issues/3024#issuecomment-165893929 .

Beep6581 commented 8 years ago

RT is built using the C++11 ABI but glib is not

Do we know, or @mks9900 do you know, which ABI the rest of your system was built against? Upgrading GCC from 4.* to 5.* specifically in Gentoo requires one to recompile "all libraries that use C++ code" , so if glib was built not using C++11 (glib is written in C), that leaves two possibilities: 1- the upgrade was not carried out correctly as some packages weren't rebuilt, or 2- you used an ABI higher than C++11, maybe C++14?

Case 1 seems most likely.

Adam if glib was compiled using C++11, would add_definitions (-D_GLIBCXX_USE_CXX11_ABI=0) still be needed?

adamreichold commented 8 years ago

I think from the linker error, we can infer that RT was built using the C++11 ABI and glibmm was not, because RT is trying to call a method using a C++11 std::string which glib does not provide. (There is also no ABI change coming with C++14, and C++11 can be used with old C++98 ABI, the standard library just is not fully conforming to the language specification then.)

If glibmm was built using the new ABI, then the extra definition is not necessary. Generally, if the whole system is built with the same compiler as is the usual approach on non-rolling Linux distributions, there shouldn't be such problems. This will usually occur only when newer compilers are combined with prebuilt libraries, i.e. rolling distributions which are not rebuilt completely or Windows. (But for example, using MSYS2 and its provided packages does not have that problem as they all use the new ABI.)

Beep6581 commented 8 years ago

I was trying to figure out whether _GLIBCXX_USE_CXX11_ABI=0 should be included in CMakeLists.txt. If glib was in C++ then the situation, according to the Gentoo docs, would be clear: all C++ libs should be rebuilt, therefore glib would be rebuilt too and it would use C++11 as would RT, so no need for that code. However glib is written in C so rebuilding it is not covered by that statement, though it might be covered as a result of this command: # revdep-rebuild --library 'libstdc\+\+\.so\.6' -- --exclude gcc

If rebuilding it is covered by either of these, then we could include it in CMakeLists.txt in the gtk3 branch. If it is not covered by either, then we leave it out and users will have to add it manually if needed.

adamreichold commented 8 years ago

@Beep6581 I think the culprit here is glibmm, the C++ wrapper around glib, which is written in C++. And this library links against libstdc++ as it uses std::string, so it should have been rebuilt.

But I don't think you can properly resolve this in one way or another by including _GLIBCXX_USE_CXX11_ABI=0 in CMakeLists.txt as it has to be consistent, but it does not matter in which way. It really is up to distribution resp. people building from source to decide which ABI is the correct one. (And even automatically detecting this can become problematic since different system libraries could use different ABI and we therefore would have to check all of them to be sure.)

IMHO, the best thing would be to make it simple to select this at build time, e.g. as in this branch, so that builders can enable the flag simply adding -DUSE_OLD_CXX_ABI=ON to the CMake call.

mks9900 commented 8 years ago

Adam: true, I didn't rebuild glibmm using the new compiler. Doing that and commenting out the line i CMakeLists.txt however resulted in this: http://pastebin.com/dnGu4ZYp

I did an emerge -eva @system though after the gcc upgrade.

BR, Johan

2015-12-19 9:40 GMT+01:00 adamreichold notifications@github.com:

@Beep6581 https://github.com/Beep6581 I think the culprit here is glibmm, the C++ wrapper around glib, which is written in C++. And this library links against libstdc++ as it uses std::string, so it should have been rebuilt.

But I don't think you can properly resolve this in one way or another by including _GLIBCXX_USE_CXX11_ABI=0 in CMakeLists.txt as it has to be consistent, but it does not matter in which way. It really is up to distribution resp. people building from source to decide which ABI is the correct one. (And even automatically detecting this can become problematic since different system libraries could use different ABI and we therefore would have to check all of them to be sure.)

IMHO, the best thing would be to make it simple to select this at build time, e.g. as in this branch https://github.com/adamreichold/RawTherapee/tree/add-cxx-abi-flag, so that builders can enable the flag simply adding -DUSE_OLD_CXX_ABI=ON to the CMake call.

— Reply to this email directly or view it on GitHub https://github.com/Beep6581/RawTherapee/issues/3024#issuecomment-165961102 .

adamreichold commented 8 years ago

@mks9900 I think this is the same problem but not for gtkmm itself, i.e.

/usr/lib/gcc/x86_64-pc-linux-gnu/5.3.0/../../../../lib64/libgtkmm-2.4.so: undefined reference to `Glib::ustring::ustring(std::string const&)'

indicates that gtkmm is still built using the old ABI, but glib using the new one.

adamreichold commented 8 years ago

Concerning the original report, I am now at the point of building master on win32 using MSYS2 and get the same errors because the RT build system always enables the old ABI when Windows, GCC 5 or later and Gtk 2 come together, i.e. the CMake log will show

-- Gcc Version >= 5.0 ; adding -D_GLIBCXX_USE_CXX11_ABI=0 to build with Gtk2

which will lead to the above problems since MSYS2's Gtkmm 2 and Glibmm 2 are also built against the new ABI. I think the only proper solution is the branch linked above, i.e. make it easy to select the correct ABI by the builder.

Btw., I think I now have the start for the Windows building instructions for RawPedia (It is still missing how to make a release build and copy out the necessary DLL for distribution. It also relies on the above branch with its manual ABI flag.):

# Install, update and run 'MinGW-w64 Win32 Shell':
https://msys2.github.io/ 

# Install build tools and dependencies:
$ pacman -S tar gzip make intltool git
$ pacman -S mingw-w64-i686-gcc mingw-w64-i686-make  mingw-w64-i686-pkg-config mingw-w64-i686-cmake
$ pacman -S mingw-w64-i686-gtkmm mingw-w64-i686-gtkmm3 mingw-w64-i686-lcms2 mingw-w64-i686-fftw

# Download and install libiptcdata:
$ curl -LO http://downloads.sourceforge.net/project/libiptcdata/libiptcdata/1.0.4/libiptcdata-1.0.4.tar.gz
$ tar xzf libiptcdata-1.0.4.tar.gz
$ cd libiptcdata-1.0.4

$ ./configure --prefix=/mingw32
# Remove 'iptc' and 'doc' from 'SUBDIRS' and 'DIST_SUBDIRS'...
$ vim Makefile

$ make
$ make install

# Download and install clearlooks theme engine:
$ curl -LO http://ftp.gnome.org/pub/GNOME/sources/gtk-engines/2.20/gtk-engines-2.20.2.tar.gz
$ tar xzf gtk-engines-2.20.2.tar.gz
$ cd gtk-engines-2.20.2
$ ./configure --prefix=/mingw32 --disable-all --enable-clearlooks
$ make
$ make install

# Clone and build RawTherapee:
$ git clone http://github.com/Beep6581/RawTherapee.git
$ cd RawTherapee

# Only if you want to build the Gtk+ 3 version...
$ git checkout gtk3

$ mkdir build
$ cd build
# Add -DUSE_OLD_CXX_ABI=ON if necessary...
$ cmake -G "Unix Makefiles" -DCMAKE_MAKE_PROGRAM=mingw32-make -DCMAKE_BUILD_TYPE="debug" -DPROC_TARGET_NUMBER="2" -DBUILD_BUNDLE="ON" -DCMAKE_INSTALL_PREFIX="." -DBINDIR="." -DDATADIR="." -DCACHE_NAME_SUFFIX=4 ..
$ make
$ make install

# Run RawTherapee:
$ cd debug
$ ./rawtherapee.exe
Beep6581 commented 8 years ago

@mks9900 see: https://wiki.gentoo.org/wiki/Upgrading_GCC and cat /usr/portage/metadata/news/2015-10-22-gcc-5-new-c++11-abi/2015-10-22-gcc-5-new-c++11-abi.en.txt

@adamreichold your branch looks good, merged to master.

mks9900 commented 8 years ago

Ah, thanks for that last text file! A massive 108 packages are now being rebuilt! Will get back to you with results later. :-)

2015-12-19 13:38 GMT+01:00 Beep6581 notifications@github.com:

@mks9900 https://github.com/mks9900 see: https://wiki.gentoo.org/wiki/Upgrading_GCC and cat /usr/portage/metadata/news/2015-10-22-gcc-5-new-c++11-abi/2015-10-22-gcc-5-new-c++11-abi.en.txt

— Reply to this email directly or view it on GitHub https://github.com/Beep6581/RawTherapee/issues/3024#issuecomment-165981857 .

Beep6581 commented 8 years ago

mks9000 unknowingly you have become my guinea pig... please let me know whether your system still runs after this, as I am still on GCC-4.9.3 and considering updating to GCC-5.3 but don't want to risk it :D

Beep6581 commented 8 years ago

This issue is to be closed once PR #3028 is merged. RawPedia has been updated: http://rawpedia.rawtherapee.com/Linux http://rawpedia.rawtherapee.com/Windows http://rawpedia.rawtherapee.com/OS_X

mks9900 commented 8 years ago

Ok, now both me and my computer are ready to report the following:

  1. After the magical revdep-rebuild, RT builds fine again without any tricks in CMakelists.txt
  2. My computer seems to be alive and kicking using the new compiler. No errors reported by any package installed. Your call, DrSlony.

Over and out from the guinea pig department. ;-)

2015-12-19 18:44 GMT+01:00 Beep6581 notifications@github.com:

This issue is to be closed once PR #3028 https://github.com/Beep6581/RawTherapee/pull/3028 is merged. RawPedia has been updated: http://rawpedia.rawtherapee.com/Linux

— Reply to this email directly or view it on GitHub https://github.com/Beep6581/RawTherapee/issues/3024#issuecomment-166009738 .

Beep6581 commented 8 years ago

Thank you :)