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 CMake support on Win32 #789

Merged
merged 3 commits into from
Apr 22, 2019
Merged

Conversation

myd7349
Copy link
Contributor

@myd7349 myd7349 commented Apr 5, 2019

Hi! I am trying to add raylib to vcpkg: microsoft/vcpkg#5946.
And there are some problems with the CMake support on Win32:

  • find_package(PkgConfig) causes find_package(raylib CONFIG REQUIRED) failed on Win32
  • When built as static library, the library name is raylib_static.lib and find_library(raylib_LIBRARY NAMES raylib HINTS ${${XPREFIX}_LIBRARY_DIRS}) will not find it
  • When built as a shared library, BUILD_LIBTYPE_SHARED is necessary. Otherwise, no export library raylib.lib will be created since no symbols are exported
  • When built as a shared library, the dll file is generally installed to the bin dir: https://cmake.org/cmake/help/v3.0/command/install.html?highlight=install

@myd7349
Copy link
Contributor Author

myd7349 commented Apr 5, 2019

The CI failed because of missing decleration of LoadMesh.
LoadMesh is deleted in: d89d24c

@raysan5 raysan5 requested a review from a3f April 5, 2019 09:31
@raysan5
Copy link
Owner

raysan5 commented Apr 5, 2019

Thanks! That sounds great to me! I'm not a CMake expert but I'm sure @a3f could take a look. :)

@a3f a3f self-assigned this Apr 8, 2019
Copy link
Contributor

@a3f a3f left a comment

Choose a reason for hiding this comment

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

Otherwise, looks good to me.

cmake/raylib-config.cmake Outdated Show resolved Hide resolved
@myd7349 myd7349 force-pushed the fix-cmake-for-win32 branch from f7486a4 to 4e4f31d Compare April 8, 2019 09:45
a3f
a3f previously approved these changes Apr 9, 2019
@a3f
Copy link
Contributor

a3f commented Apr 9, 2019

I've rebased your commits on top of master and It looks like your PR breaks mingw 32 & 64 bit builds:

C:\mingw-w64\i686-6.3.0-posix-dwarf-rt_v5-rev1\mingw32\bin\gcc.exe -fno-strict-aliasing -Werror=implicit-function-declaration -Werror=pointer-arith -m32 -g  -m32 -Wl,--whole-archive CMakeFiles\models_rlgl_solar_system.dir/objects.a -Wl,--no-whole-archive  -o models_rlgl_solar_system.exe -Wl,--out-implib,libmodels_rlgl_solar_system.dll.a -Wl,--major-image-version,0,--minor-image-version,0 @CMakeFiles\models_rlgl_solar_system.dir\linklibs.rsp
CMakeFiles\models_rlgl_solar_system.dir/objects.a(models_rlgl_solar_system.c.obj): In function `main':
C:/projects/raylib/examples/models/models_rlgl_solar_system.c:81: undefined reference to `rlPushMatrix'
C:/projects/raylib/examples/models/models_rlgl_solar_system.c:82: undefined reference to `rlScalef'
C:/projects/raylib/examples/models/models_rlgl_solar_system.c:84: undefined reference to `rlPopMatrix'
C:/projects/raylib/examples/models/models_rlgl_solar_system.c:86: undefined reference to `rlPushMatrix'
C:/projects/raylib/examples/models/models_rlgl_solar_system.c:87: undefined reference to `rlRotatef'
C:/projects/raylib/examples/models/models_rlgl_solar_system.c:88: undefined reference to `rlTranslatef'
C:/projects/raylib/examples/models/models_rlgl_solar_system.c:89: undefined reference to `rlRotatef'
C:/projects/raylib/examples/models/models_rlgl_solar_system.c:91: undefined reference to `rlPushMatrix'
C:/projects/raylib/examples/models/models_rlgl_solar_system.c:92: undefined reference to `rlRotatef'
C:/projects/raylib/examples/models/models_rlgl_solar_system.c:93: undefined reference to `rlScalef'
C:/projects/raylib/examples/models/models_rlgl_solar_system.c:96: undefined reference to `rlPopMatrix'
C:/projects/raylib/examples/models/models_rlgl_solar_system.c:98: undefined reference to `rlRotatef'
C:/projects/raylib/examples/models/models_rlgl_solar_system.c:99: undefined reference to `rlTranslatef'
C:/projects/raylib/examples/models/models_rlgl_solar_system.c:100: undefined reference to `rlRotatef'
C:/projects/raylib/examples/models/models_rlgl_solar_system.c:101: undefined reference to `rlRotatef'
C:/projects/raylib/examples/models/models_rlgl_solar_system.c:102: undefined reference to `rlScalef'
C:/projects/raylib/examples/models/models_rlgl_solar_system.c:105: undefined reference to `rlPopMatrix'
CMakeFiles\models_rlgl_solar_system.dir/objects.a(models_rlgl_solar_system.c.obj): In function `DrawSphereBasic':
C:/projects/raylib/examples/models/models_rlgl_solar_system.c:139: undefined reference to `rlBegin'
C:/projects/raylib/examples/models/models_rlgl_solar_system.c:140: undefined reference to `rlColor4ub'
C:/projects/raylib/examples/models/models_rlgl_solar_system.c:146: undefined reference to `rlVertex3f'
C:/projects/raylib/examples/models/models_rlgl_solar_system.c:149: undefined reference to `rlVertex3f'
C:/projects/raylib/examples/models/models_rlgl_solar_system.c:152: undefined reference to `rlVertex3f'
C:/projects/raylib/examples/models/models_rlgl_solar_system.c:156: undefined reference to `rlVertex3f'
C:/projects/raylib/examples/models/models_rlgl_solar_system.c:159: undefined reference to `rlVertex3f'
CMakeFiles\models_rlgl_solar_system.dir/objects.a(models_rlgl_solar_system.c.obj):C:/projects/raylib/examples/models/models_rlgl_solar_system.c:162: more undefined references to `rlVertex3f' follow
CMakeFiles\models_rlgl_solar_system.dir/objects.a(models_rlgl_solar_system.c.obj): In function `DrawSphereBasic':
C:/projects/raylib/examples/models/models_rlgl_solar_system.c:167: undefined reference to `rlEnd'
collect2.exe: error: ld returned 1 exit status
examples\CMakeFiles\models_rlgl_solar_system.dir\build.make:87: recipe for target 'examples/models_rlgl_solar_system.exe' failed
mingw32-make.exe[2]: *** [examples/models_rlgl_solar_system.exe] Error 1
mingw32-make.exe[2]: Leaving directory 'C:/projects/raylib/build'
CMakeFiles\Makefile2:2246: recipe for target 'examples/CMakeFiles/models_rlgl_solar_system.dir/all' failed
mingw32-make.exe[1]: *** [examples/CMakeFiles/models_rlgl_solar_system.dir/all] Error 2
mingw32-make.exe[1]: Leaving directory 'C:/projects/raylib/build'
Makefile:161: recipe for target 'all' failed
mingw32-make.exe: *** [all] Error 2
Command exited with code 2

Can you take a look?

@a3f a3f self-requested a review April 9, 2019 06:00
@a3f a3f dismissed their stale review April 9, 2019 06:01

CI is fixed on master, PR still fails

@myd7349 myd7349 force-pushed the fix-cmake-for-win32 branch from 4e4f31d to 5134a3b Compare April 9, 2019 10:03
@myd7349
Copy link
Contributor Author

myd7349 commented Apr 9, 2019

@a3f This is strange, since rlPushMatrix is defined in a header file rlgl.h: https://github.com/raysan5/raylib/blob/master/src/rlgl.h#L988
I will rebase again to trigger a rebuild.

@myd7349
Copy link
Contributor Author

myd7349 commented Apr 9, 2019

CI failure on MinGW is caused by these lines:

  target_compile_definitions(raylib
      PRIVATE $<BUILD_INTERFACE:BUILD_LIBTYPE_SHARED>
      INTERFACE $<INSTALL_INTERFACE:USE_LIBTYPE_SHARED>
  )

Log file: log-fail.txt

A workaround is:

if (MSVC)
  target_compile_definitions(raylib
      PRIVATE $<BUILD_INTERFACE:BUILD_LIBTYPE_SHARED>
      INTERFACE $<INSTALL_INTERFACE:USE_LIBTYPE_SHARED>
  )
endif ()

Log file: log-ok.txt

Compare these two log files, I see the differences:
Picture

@myd7349
Copy link
Contributor Author

myd7349 commented Apr 9, 2019

Now I think the CI should pass. But I still do not know why it failed before.
Please do not merge yet, I want to dig it deeper.

@raysan5
Copy link
Owner

raysan5 commented Apr 13, 2019

@myd7349 did you dig it? could we merge this PR?

@myd7349
Copy link
Contributor Author

myd7349 commented Apr 13, 2019

Sorry for the delay, I'm in bed now. I will give it a test tomorrow.

@raysan5 raysan5 closed this Apr 13, 2019
@raysan5 raysan5 reopened this Apr 13, 2019
@raysan5
Copy link
Owner

raysan5 commented Apr 13, 2019

ok, no worries and no hurries! :)

@myd7349
Copy link
Contributor Author

myd7349 commented Apr 14, 2019

Finally, I figure out what happens.
I created a small project to reproduce the CI build error: https://github.com/myd7349/Ongoing-Study/tree/master/cpp/CMake/raylib_pr789_ci_failure
Notice that, I use an additional option REPRODUCE_CI_BUILD_ERROR to control whether or not to reproduce this CI build error:

    if (MSVC OR REPRODUCE_CI_BUILD_ERROR)
        target_compile_definitions(raylib
            PRIVATE $<BUILD_INTERFACE:BUILD_LIBTYPE_SHARED>
            INTERFACE $<INSTALL_INTERFACE:USE_LIBTYPE_SHARED>
        )
    endif ()

If I build this small project with:

cmake .. -G "MinGW Makefiles" -DSHARED=ON -DBUILD_EXAMPLES=ON -DREPRODUCE_CI_BUILD_ERROR=OFF

then MinGW will see a definition of RL_API like this:

#define RL_API

and all the symbols found in core.c.obj will be exported, including those inlined from rlgl.h(Since we define RLGL_IMPLEMENTATION just before including rlgl.h in core.c).
ok

If I build this small project with:

cmake .. -G "MinGW Makefiles" -DSHARED=ON -DBUILD_EXAMPLES=ON -DREPRODUCE_CI_BUILD_ERROR=ON

then MinGW will see a definition of RL_API like this:

#define RL_API __declspec(dllexport)

and this time, the target file doesn't contain rlMatrixMode:
fail

In fact, this workaround:

    if (MSVC)
        target_compile_definitions(raylib
            PRIVATE $<BUILD_INTERFACE:BUILD_LIBTYPE_SHARED>
            INTERFACE $<INSTALL_INTERFACE:USE_LIBTYPE_SHARED>
        )
    endif ()

just bypass the definition of BUILD_LIBTYPE_SHARED and USE_LIBTYPE_SHARED for MinGW. And all the symbols will be exported, so there will be no build errors with models_rlgl_solar_system.c.

However, these is still build errors with models_rlgl_solar_system.c if we specify SHARED=ON and use Visual Studio 2015 as the generator.

To solve this build error thoroughly, we have to think about this questions before:
Are functions like rlPushMatrix, rlMatrixMode a part of raylib's public API? That is, should them be exported when built as a shared library?
If the answer is YES, then we should prepend RL_API declaration before the original declarations of these functions in rlgl.h.
If these functions are intended to be used internally, then we should define RLGL_IMPLEMENTATION before including rlgl.h in models_rlgl_solar_system.c.

@raysan5
Copy link
Owner

raysan5 commented Apr 22, 2019

@myd7349 thank you very much for tracking this issue! I always use raylib as static library so I didn't though about it when adding models_rlgl_solar_system.c example...

When using raylib as shared library, probably rlgl functions should be included in the library, would it be enought just adding RLAPI definition same as raylib.h?

@raysan5
Copy link
Owner

raysan5 commented Apr 22, 2019

rlgl modified on commit 2249f12

@raysan5 raysan5 merged commit e41cb77 into raysan5:master Apr 22, 2019
@myd7349 myd7349 deleted the fix-cmake-for-win32 branch April 23, 2019 09:51
@myd7349
Copy link
Contributor Author

myd7349 commented Apr 23, 2019

@raysan5 Thanks! I will give it a test later.

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