Lots of build fixes, minor cleanups#6
Conversation
ivucica
left a comment
ivucica
left a comment
There was a problem hiding this comment.
Thank you for sending this in! I truly do appreciate this being looked at.
I think I would mostly like to understand why we need to add a dependency on an extra library (glew).
I understand it's useful, but it's still just a convenience (and mostly only needed for shaders). If it's strongly improving the experience, we can do that, but should document why and how, and do it in minimal amount of places where required, not across the board.
In short, gl.h is still correct on almost all platforms, so replacing it with glew is not a build fix.
I may have missed a big fix done. Please document in the PR text what build errors you've fixed and on what platforms and configurations (e.g. debian 12 aarch64, error "unable to XYZ") and whether this was fixable in another way (https://packages.debian.org/file:gl.h perhaps). If gl.h and glext.h are really not available except via glew.h, we can see how to best apply these changes.
If there was no big build fix done, maybe just PR and/or commit messages need adjustment?
| #import <AppKit/NSApplication.h> | ||
| #import <AppKit/NSMenu.h> | ||
| #import <GL/gl.h> | ||
| #import <GL/glew.h> |
There was a problem hiding this comment.
But, GLEW is different than base OpenGL headers?
I'd like to learn more about why this additional dependency is being added. Which system is missing gl.h but has glew.h?
| #import <GL/gl.h> | ||
| #import <GL/glew.h> | ||
| #import <GL/glu.h> | ||
| #import <GL/glext.h> |
There was a problem hiding this comment.
This should still be a standard header; I have GL/glext.h on my hardware, which machine is this missing on?
(This applies across the board where this change was made)
| such as clang. gcc-4.6+ will probably work, too. For requirements of | ||
| each individual library as well as installation procedure, refer to their | ||
| documentation (or refer to online search engines). | ||
| 1. Take a look at the README for requirements. |
There was a problem hiding this comment.
- INSTALL file is correct to list dependencies, if anything README could refer back to INSTALL.
- You've added dependency on glew, but have not documented it
- You've removed reference to optal-nsfonthacks.patch from README, but not here
- It would be better to add gnustep-corebase if it's actively needed (I don't recall if it is)
|
|
||
| * **Foundation**. You can use GNUstep Base or Apple Cocoa to get Foundation. | ||
| * **AppKit**. You can use GNUstep GUI or Apple Cocoa to get AppKit. | ||
| * **CoreFoundation**. You can use GNUstep CoreBase or Apple Cocoa to get CoreFoundation. |
| due to its use of `NSOpenGLContext` -- otherwise it doesn't care about the | ||
| UI framework used -- an experimental patch is provided. See | ||
| `opal-nsfonthacks.patch`. | ||
| * Note (2025) - this seems to be fixed. |
There was a problem hiding this comment.
Good change, does this mean you simply did not apply it and all demo programs (incl those based on bridge) worked?
|
Unfortunately it's been a while since I last tried to build this, and I currently am focusing on a lot of other projects. So I might not be able to improve this PR for a while. |
2 participants
This lets it build on modern GNU/Linux systems.