Simple-OpenGL-Image-Library icon indicating copy to clipboard operation
Simple-OpenGL-Image-Library copied to clipboard

Do not use CXX

Open Lecrapouille opened this issue 3 years ago • 5 comments

In https://github.com/kbranigan/Simple-OpenGL-Image-Library/blob/4fff135429c4c7996791c019e6cc20a1e9fde568/Makefile#L16

and in Makefile: https://github.com/kbranigan/Simple-OpenGL-Image-Library/blob/4fff135429c4c7996791c019e6cc20a1e9fde568/Makefile#L42

CXX is for g++ should be CC and CFLAGS and better to use ?=

This can conflict when doing make CXX=g++

Lecrapouille avatar Nov 04 '22 09:11 Lecrapouille

That seems reasonable, do you have a reference of best practices I can refer to?

kbranigan avatar Nov 04 '22 15:11 kbranigan

@kbranigan of course https://stackoverflow.com/questions/5541946/cflags-ccflags-cxxflags-what-exactly-do-these-variables-control the goal is to let you do stuff like make CC=clang CFLAGS=-Wdesired-options (in my case my main Makefile calls other Makefile (third parts) passing my CXX and CC and in our case we compile SOIL with g++ which ends badly :)

Lecrapouille avatar Nov 04 '22 16:11 Lecrapouille

@kbranigan While not perfect, I adapt mine https://github.com/Lecrapouille/SOIL/blob/master/Makefile

Lecrapouille avatar Nov 05 '22 14:11 Lecrapouille

Are you interested in doing a pull request? I'm up for incorporating your changes

kbranigan avatar Nov 05 '22 14:11 kbranigan

@kbranigan here is the PR. Sorry for the long delay: I messed up myself I've already forked this project with different name but could not fork it again, so I used my organization name (but I should have used a branch) ... anyway.

The src/stb_image_aug.c is suffering of -Wmaybe-uninitialized and I'm not sure what if -soption is important (stripping) for a lib (a binary yes) and clang does not like it. I think -fPIC should be used by default for static libraries (this can be done in a second PR). Finally, https://github.com/SpartanJ/SOIL2 seems to be a fresher version to this repo, so I'm not sure if efforts has to be made for this current repo ?

Lecrapouille avatar Nov 09 '22 02:11 Lecrapouille