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

MacOS: -Dcocoa=ON -Dopengl=OFF pass cmake but fail compilation #7160

Closed
pcanal opened this issue Feb 9, 2021 · 4 comments · Fixed by #15185
Closed

MacOS: -Dcocoa=ON -Dopengl=OFF pass cmake but fail compilation #7160

pcanal opened this issue Feb 9, 2021 · 4 comments · Fixed by #15185

Comments

@pcanal
Copy link
Member

pcanal commented Feb 9, 2021

On MacOS passing to cmake:

`-Dcocoa=ON -Dopengl=OFF`

leads to

/Users/pcanal/root_working/code/rootcling/graf2d/cocoa/src/TGCocoa.mm:20:10: fatal error: 'TGLIncludes.h' file not found
#include "TGLIncludes.h"
         ^~~~~~~~~~~~~~~
1 error generated.

In graf2d/cocoa/CMakeLists.txt the code:

if(opengl) # special case when cocoa is enabled but not opengl (i.e. gminimal=ON)
  target_include_directories(GCocoa PRIVATE ${OPENGL_INCLUDE_DIR})
else()
  target_include_directories(GCocoa PRIVATE
    ${CMAKE_SOURCE_DIR}/graf3d/gl/inc
  )
endif(opengl)

acknowledge the challenge but fail to address it.

And indeed in TGCocoa.mm the file TGLIncludes.h is always included.

Maybe this should fail at configuration time.

@pcanal pcanal added the bug label Feb 9, 2021
@github-actions github-actions bot added this to Needs triage in Triage Feb 9, 2021
@eguiraud eguiraud removed this from Needs triage in Triage Feb 10, 2021
@couet
Copy link
Member

couet commented Feb 16, 2021

I asked Timur about that. He told me that Cocca can be installed without OpenGl and a few ifdefs should make it. Indeed I just tried and found that in TGCocoa.mm, TGLIncludes.h should be under a "opengl ifdef"... in that same file glFlush(); should be also. Then in CMakeFiles/GCocoa.dir/link.txt , -lRGlew should not be generated.
So that mainly defining a new GL flag (or using an existing one) and changing the cmake procedure to avoid generating -lRGlew. May be @oshadura can help ?

@oshadura oshadura assigned bellenot and unassigned oshadura Jul 14, 2021
olemorud added a commit to olemorud/root that referenced this issue Mar 22, 2023
olemorud added a commit to olemorud/root that referenced this issue Mar 22, 2023
olemorud added a commit to olemorud/root that referenced this issue Mar 22, 2023
olemorud added a commit to olemorud/root that referenced this issue Mar 22, 2023
olemorud added a commit to olemorud/root that referenced this issue Mar 22, 2023
olemorud added a commit to olemorud/root that referenced this issue Mar 22, 2023
olemorud added a commit to olemorud/root that referenced this issue Mar 23, 2023
olemorud added a commit to olemorud/root that referenced this issue Mar 23, 2023
olemorud added a commit to olemorud/root that referenced this issue Mar 23, 2023
olemorud added a commit to olemorud/root that referenced this issue Mar 23, 2023
olemorud added a commit to olemorud/root that referenced this issue Mar 23, 2023
olemorud added a commit to olemorud/root that referenced this issue Mar 23, 2023
olemorud added a commit to olemorud/root that referenced this issue Mar 23, 2023
olemorud added a commit that referenced this issue Mar 27, 2023
omazapa pushed a commit to omazapa/root that referenced this issue Apr 13, 2023
@dpiparo dpiparo self-assigned this Apr 1, 2024
@bellenot
Copy link
Member

bellenot commented Apr 5, 2024

@pcanal before I look deeply into that, can you explain the specific use case for such combination? Is it really worth the effort?

@pcanal
Copy link
Member Author

pcanal commented Apr 5, 2024

If i remember correctly I encountered this when starting from a minimal build (-Dminimal=ON) and then adding just -Dcocoa=ON and it fails.

I see 2 possible resolutions
(a) Implement what is hinted at in #7160 (comment)
(b) Have CMake fail for -Dcocoa=ON -Dopengl=OFF

@dpiparo
Copy link
Member

dpiparo commented Apr 7, 2024

I would go for (b) it looks simpler .

guitargeek added a commit to guitargeek/root that referenced this issue Apr 9, 2024
guitargeek added a commit to guitargeek/root that referenced this issue Apr 9, 2024
Closes root-project#7160, as discussed in the issue thread.
guitargeek added a commit to guitargeek/root that referenced this issue Apr 9, 2024
Closes root-project#7160, as discussed in the issue thread.
@guitargeek guitargeek added this to Issues in Fixed in 6.34.00 via automation Apr 10, 2024
guitargeek added a commit that referenced this issue Apr 10, 2024
Closes #7160, as discussed in the issue thread.
kristupaspranc pushed a commit to kristupaspranc/root that referenced this issue May 21, 2024
Closes root-project#7160, as discussed in the issue thread.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging a pull request may close this issue.

7 participants