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

Do not use CXX #18

Open
Lecrapouille opened this issue Nov 4, 2022 · 5 comments
Open

Do not use CXX #18

Lecrapouille opened this issue Nov 4, 2022 · 5 comments

Comments

@Lecrapouille
Copy link
Contributor

In

and in Makefile:

$(CXX) $(CXXFLAGS) -o $@ -c $<

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

This can conflict when doing make CXX=g++

@kbranigan
Copy link
Owner

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

@Lecrapouille
Copy link
Contributor Author

@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
Copy link
Contributor Author

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

@kbranigan
Copy link
Owner

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

@Lecrapouille
Copy link
Contributor Author

Lecrapouille commented Nov 9, 2022

@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 added a commit to Lecrapouille/SOIL that referenced this issue Nov 9, 2022
CXX = gcc is not a good idea because:
1/ We cannot override options like: make CC=clang-13 CFLAGS="-O2 -s
-Wall -fPIC" -j8
2/ CXX is for g++ and overriding will fail compiling the project.
3/ CC is by default equals to cc (gcc)
Lecrapouille added a commit to Lecrapouille/SOIL that referenced this issue Nov 9, 2022
CXX = gcc is not a good idea because:
1/ We cannot override options like: make CC=clang-13 CFLAGS="-O2 -s
-Wall -fPIC" -j8
2/ CXX is for g++ and overriding will fail compiling the project.
3/ CC is by default equals to cc (gcc)
kbranigan added a commit that referenced this issue Nov 16, 2022
Fix CXX and CXXFLAGS are for g++ and this is a C project #18
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

No branches or pull requests

2 participants