Project

General

Profile

Actions

Emulator Issues #3507

closed

CMake system fails

Added by Christian.Morales.Vega over 13 years ago.

Status:
Fixed
Priority:
Normal
% Done:

0%

Operating system:
N/A
Issue type:
Bug
Milestone:
Regression:
No
Relates to usability:
No
Relates to performance:
No
Easy:
No
Relates to maintainability:
No
Regression start:
Fixed in:

Description

The CMake system in r6415 fails to work at all because it search for libglew instead of libGLEW. Also:

  • The VISIBILITY_INLINES_HIDDEN is set in a way that a semicolon is added to the CXX_FLAGS
  • Plugins are compiled as shared libraries, not as modules
  • Plugins fail to link against some used libraries

The attached patch fixes it.

Actions #1

Updated by Christian.Morales.Vega over 13 years ago

Obviously I attached the wrong file.

Actions #2

Updated by hrydgard over 13 years ago

  • Status changed from New to Accepted

You got commit access.

Actions #3

Updated by NeoBrainX over 13 years ago

fwiw, with your patch I'm getting numerous of these warnings:
cc1: warning: command line option "-fvisibility-inlines-hidden" is valid for C++/ObjC++ but not for C

Actions #4

Updated by Christian.Morales.Vega over 13 years ago

Ok, the problem was that "set(CMAKE_CXX_FLAGS ${CMAKE_CXX_FLAGS} -fvisibility-inlines-hidden)" failed if CMAKE_CXX_FLAGS was already set (export CXXFLAGS="-foo" before the cmake call).

Instead of "add_definitions(-fvisibility-inlines-hidden)", that also adds the flag to C, it seems the best thing to do would be

set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fvisibility-inlines-hidden")

Any problem with that?

Actions #5

Updated by glennricster over 13 years ago

Do not apply that patch. There are several issues with it. I don't have time to elaborate now, but will later.

Actions #6

Updated by Christian.Morales.Vega over 13 years ago

Ok, don't worry. I already waited since, to start with, I'm sure someone has a system where glew is libglew. Otherwise that would have been detected before.

Take your time.

Actions #7

Updated by glennricster over 13 years ago

Ok, so first of all the add_definitions adds the given definition to both c and c++ flags, and -fvisibility-inlines-hidden is not a valid c flag. It is only valid as a c++/objc++ flag as noted by NeoBrainX. So leave that line the way it is. It is not adding a semicolon as you are claiming. Look in the various flags.txt files that are generated and you will see that I am correct.

Second of all the glew in all lower case is correct as it is. The reason it is lower case is because that is what pkg-config uses. Run "pkg-config --libs glew" and you will see that it correctly returns the linker flag "-lGLEW". The check_lib macro that I defined in CMakeTests/CheckLib.cmake first uses pkg-config and then falls back to the built in macro find_library. The pkg-config method is better when it is available for a library, as it is for the GLEW library. It checks for development header files as well as checking for the system shared library. If this fails to build for you then you must not have the proper development files installed on your system and the build will fail no matter how you do it.

Some other notes: Whether you define the plugins to be SHARED or a MODULE doesn't really matter at this point. Cmake treats them the same. If we start setting linker flags via the CMAKE_MODULE_LINKER_FLAGS and CMAKE_SHARED_LINKER_FLAGS then it will be more important. The rest of the changes you have are fine, but mostly unnecessary.

So to summarize much of your patch is incorrect and the rest is unnecessary.

Actions #8

Updated by Christian.Morales.Vega over 13 years ago

I didn't the patch just because I have too much free time ;-)
I attach the output of the cmake run on an openSUSE 11.3 system, cmake_output.txt, and the CMakeError.log file.

The patch changes the output so:

  • The VISIBILITY_HIDDEN test succeeds
  • PortAudio is detected (yes, it's installed)
  • glew is detected

See the semicolon in CMakeError.log: "-fasynchronous-unwind-tables -g ;-fvisibility-inlines-hidden". Sorry, I don't see any flags.txt file (find -iname "flags" returns nothing). This is with cmake 2.8.1.
I already accepted the change to add_definitions() has a problem. I asked about quoting "${CMAKE_CXX_FLAGS} -fvisibility-inlines-hidden" instead.

About glew, the pkg-config file is only there since version 1.5.3 (from early this year), so there is a problem with pre-2010 versions. I have no problem making this patch openSUSE specific, I let you decide how to handle it.

About shared vs module, I was expecting it to stop assigning a soname to the plugins. But it seems cmake doesn't agrees with me here. After finding this I left the change for correctness, but if you prefer to keep SHARED instead of MODULE I have no problem with this.

Actions #9

Updated by glennricster over 13 years ago

The reason you are getting the message about glew not found is because you don't have the proper development files for glew installed and configured correctly on your system. Try running the pkg-config from my last post from a command line and see what you get. I.e. "pkg-config --libs glew".

Actions #10

Updated by Christian.Morales.Vega over 13 years ago

I have the development files for glew installed. Your pkg-config command fails because as I said openSUSE 11.3 comes with a pre-1.5.3 version of glew that doesn't installs a /usr/lib64/pkgconfig/glew.pc file.

$ rpm -ql glew-devel
/usr/include/GL/glew.h
/usr/include/GL/glxew.h
/usr/include/GL/wglew.h
/usr/lib64/libGLEW.so

Actions #11

Updated by glennricster over 13 years ago

The reason that you can't find the flags.txt files that I referred to is because the configuration does not succeed and it doesn't write those files. When the cmake configuration succeeds it will write the flags.txt files in each of the source directories, as well as the link.txt and Makefile files.

How are you doing this build? What distribution are you using?

Actions #12

Updated by Christian.Morales.Vega over 13 years ago

It's openSUSE 11.3. And the spec file I used to create the RPMs is the first file attached (by error).
That would be translated to:

export CFLAGS='-fmessage-length=0 -O2 -Wall -D_FORTIFY_SOURCE=2 -fstack-protector -funwind-tables -fasynchronous-unwind-tables -g'
export CXXFLAGS='-fmessage-length=0 -O2 -Wall -D_FORTIFY_SOURCE=2 -fstack-protector -funwind-tables -fasynchronous-unwind-tables -g'
mkdir build
cd build
cmake -DCMAKE_INSTALL_PREFIX=/usr -DLIB_SUFFIX=64 ..

Actions #13

Updated by glennricster over 13 years ago

The glew.pc file was included with glew before version 1.5.3. What version of glew do yyou have? It sounds to me like this is a packaging issue. The OpenSUSE developers might have forgotten to include it.

Actions #14

Updated by Christian.Morales.Vega over 13 years ago

Perhaps other distros patched its packages to add a glew.pc file, but that file is in the glew SVN only since "Thu Jan 7 17:03:40 2010 UTC (10 months, 1 week ago)": http://glew.svn.sourceforge.net/viewvc/glew/trunk/glew/glew.pc.in?view=log

Actions #15

Updated by glennricster over 13 years ago

Ahh, I see. The correct way to do it is to add quotes around the parameters. In other words it should be
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fvisibility-inlines-hidden")

Actions #16

Updated by glennricster over 13 years ago

Hmm, so it seems the best way to remedy the glew issue is to modify the check_lib macro to first check with pkg-config with glew and then check for the library with GLEW. I still would like to know what version of glew is installed on OpenSUSE though. The version installed on Ubuntu is 1.5.2 and it has the glew.pc file. Of course the most recent Ubuntu distribution is only about one month old.

Actions #17

Updated by glennricster over 13 years ago

Do you know if glu.pc is installed on openSUSE?

Actions #18

Updated by Christian.Morales.Vega over 13 years ago

openSUSE comes with glew 1.5.0, probably more old than it should. But the Ubuntu Maverick 1.5.2 package only has the glew.pc file because of this patch:

--- glew-1.5.2.orig/Makefile
+++ glew-1.5.2/Makefile
@@ -43,6 +43,8 @@ endif
GLEW_DEST ?= /usr
BINDIR ?= $(GLEW_DEST)/bin
LIBDIR ?= $(GLEW_DEST)/lib
+PKGCFGDIR ?= $(LIBDIR)/pkgconfig
+PKGCFGFILE ?= glew.pc
INCDIR ?= $(GLEW_DEST)/include/GL
SHARED_OBJ_EXT ?= o
TARDIR = ../glew-$(GLEW_VERSION)
@@ -121,7 +123,7 @@ ifeq ($(patsubst mingw%,mingw,$(SYSTEM))
$(STRIP) -x lib/$(LIB.SHARED)
$(INSTALL) -m 0644 lib/$(LIB.SHARED) $(BINDIR)/
else

  • $(STRIP) -x lib/$(LIB.SHARED)
    +# $(STRIP) -x lib/$(LIB.SHARED)
    $(INSTALL) -m 0644 lib/$(LIB.SHARED) $(LIBDIR)/
    $(LN) $(LIB.SHARED) $(LIBDIR)/$(LIB.SONAME)
    endif
    @@ -132,12 +134,26 @@ endif
    ifeq ($(patsubst mingw%,mingw,$(SYSTEM)), mingw)
    $(INSTALL) -m 0644 lib/$(LIB.DEVLNK) $(LIBDIR)/
    else
  • $(STRIP) -x lib/$(LIB.STATIC)
    +# $(STRIP) -x lib/$(LIB.STATIC)
    $(INSTALL) -m 0644 lib/$(LIB.STATIC) $(LIBDIR)/
    $(LN) $(LIB.SHARED) $(LIBDIR)/$(LIB.DEVLNK)
    endif

utilities

  • $(INSTALL) -s -m 0755 bin/$(GLEWINFO.BIN) bin/$(VISUALINFO.BIN) $(BINDIR)/
  • -$(INSTALL) -m 0755 bin/$(GLEWINFO.BIN) bin/$(VISUALINFO.BIN) $(BINDIR)/

+pkgconfig:

  • $(INSTALL) -d -m 0755 $(PKGCFGDIR)
  • echo prefix=$(TRG_DEST) > $(PKGCFGFILE)
  • echo exec_prefix=$${prefix} >> $(PKGCFGFILE)
  • echo libdir=$${prefix}/lib >> $(PKGCFGFILE)
  • echo includedir=$${prefix}/include >> $(PKGCFGFILE)
  • echo >> $(PKGCFGFILE)
  • echo Name: The OpenGL Extension Wrangler Library >> $(PKGCFGFILE)
  • echo Description: cross-platform C/C++ extension loading library >> $(PKGCFGFILE)
  • echo Version: $(GLEW_VERSION) >> $(PKGCFGFILE)
  • echo Libs: -L$${libdir} -l$(NAME) >> $(PKGCFGFILE)
  • echo Cflags: -I$${includedir} >> $(PKGCFGFILE)
  • $(INSTALL) -m 0644 $(PKGCFGFILE) $(PKGCFGDIR)/

uninstall:
$(RM) $(INCDIR)/wglew.h

Go to http://packages.ubuntu.com/source/maverick/glew, download the file glew_1.5.2-0ubuntu1.debian.tar.gz and see by yourself.

Actions #19

Updated by glennricster over 13 years ago

Also it seems that you need to update to the latest revision of dolphin. Your patch does not apply against the current revision.

Actions #20

Updated by Christian.Morales.Vega over 13 years ago

Yes, Mesa from openSUSE comes with a glu.pc file.

Actions #21

Updated by glennricster over 13 years ago

Try out this modified patch. You will need to update to the latest svn revision 6430.

Actions #22

Updated by glennricster over 13 years ago

Actually test this patch. I did a little clean up.

Actions #23

Updated by glennricster over 13 years ago

Christian: Have you tested that patch? Does it work for you? If so you can commit it whenever you get a chance.

Actions #24

Updated by Christian.Morales.Vega over 13 years ago

Tested with r6439. The only change I needed is to also link to X11/GL the video software plugin that was added in r6431 (I expect this to not cause problems in Windows, true?).

Give some minutes to be sure I don't break anything. SVN is not my first language... And perhaps the server is down? (500 Internal Server Error)

Actions #25

Updated by Christian.Morales.Vega over 13 years ago

  • Status changed from Accepted to Fixed

Commited.
Closing...

Actions #26

Updated by glennricster over 13 years ago

The cmake build doesn't work on windows yet, but we want it to eventually.

Actions #27

Updated by nitro322 over 13 years ago

Just to confirm, this fixed my issue building on Gentoo as well. Thanks!

Actions

Also available in: Atom PDF