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 builds without GL #255

Closed
wants to merge 2 commits into from
Closed

Conversation

Squareys
Copy link
Contributor

@Squareys Squareys commented Jun 23, 2018

Hi @mosra !

I applied your patch for the fix for building WITH_GL=OFF. If building with BUILD_DEPRECATED=ON, it would not find External/OpenGL, which I made sure that it gets installed anyway now (untested!).

Cheers, Jonathan

The issue you get when trying to build a project using magnum that was build accordingly:

-- The C compiler identification is GNU 4.8.5
-- The CXX compiler identification is GNU 4.8.5
-- Check for working C compiler using: Ninja
-- Check for working C compiler using: Ninja -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Detecting C compile features
-- Detecting C compile features - done
-- Check for working CXX compiler using: Ninja
-- Check for working CXX compiler using: Ninja -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Found Corrade: /usr/local/include  found components:  Utility Containers 
-- Found Corrade: /usr/local/include  found components:  Utility Containers rc 
-- Found Magnum: /usr/local/include   
-- LIB_SUFFIX variable is not defined. It will be autodetected now.
-- You can set it manually with -DLIB_SUFFIX=<value> (64 for example)
-- LIB_SUFFIX autodetected as '64', libraries will be installed into /builds/vhiterabbit/webvr-pong/deploy/lib64
-- Found Magnum: /usr/local/include  found components:  Magnum 
-- Configuring done
CMake Error in src/server/CMakeLists.txt:
  Imported target "Magnum::Magnum" includes non-existent path

    "/usr/local/include/MagnumExternal/OpenGL"

  in its INTERFACE_INCLUDE_DIRECTORIES.  Possible reasons include:

  * The path was deleted, renamed, or moved to another location.

  * An install or uninstall procedure did not complete successfully.

  * The installation package was faulty and references files it does not
  provide.

mosra and others added 2 commits June 23, 2018 21:53
Some deprecated APIs use headers (but not externally defined symbols)
from the GL library, link those includes as well


Signed-off-by: Squareys <squareys@googlemail.com>
find_package(OpenGLES2 REQUIRED)
else()
find_package(OpenGLES3 REQUIRED)
if(WITH_GL OR TARGET_GL)
Copy link
Owner

Choose a reason for hiding this comment

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

After thinking about this a bit more, this should be just if(WITH_GL). TARGET_GL can be only enabled if WITH_GL is enabled.

if(BUILD_DEPRECATED AND TARGET_GL) # TODO: remove once compat gets dropped
if(BUILD_DEPRECATED) # TODO: remove once compat gets dropped
# Some deprecated APIs use headers (but not externally defined symbols)
# from the GL library, link those includes as well
Copy link
Owner

Choose a reason for hiding this comment

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

In many places I used the following preprocessor statement:

#if defined(MAGNUM_BUILD_DEPRECATED) && defined(MAGNUM_TARGET_GL)

but there are apparently cases that I missed, having just #ifdef MAGNUM_BUILD_DEPRECATED. Sorry.

... so, instead of installing the GL stuff even when the user explicitly doesn't want to, the deprecated stuff depending on GL should not be compiled at all when TARGET_GL is OFF. So all these CMake patches that add GL stuff should be removed and replaced by preprocessor patches that remove GL stuff.

I have no easy way of testing this myself without creating a barebones VM from scratch, so I'm kinda depending on you to find and fix all those cases :) Thank you 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright, that will require time that I currently don't have, though. I will have the time to work through this as of the 9th of July, I hope that is okay.

@mosra mosra added this to the 2018.0c milestone Sep 14, 2018
@mosra mosra added this to TODO in GL via automation Sep 14, 2018
@mosra
Copy link
Owner

mosra commented Sep 15, 2018

I fixed most of the issues affecting core library in 89d9883 while integrating the Vulkan Triangle example. There might be some more left, but since none of the changes of this PR would get used anyway, I'm closing it.

@mosra mosra closed this Sep 15, 2018
GL automation moved this from TODO to Done Sep 15, 2018
@mosra mosra mentioned this pull request Sep 16, 2018
56 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
GL
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants