build fails on OSX / clang

[neomantra]

Building on OSX / clang fails with:

clang: warning: -lc: 'linker' input unused
clang: warning: -lm: 'linker' input unused
error: invalid integral value 'g' in '-Og'
error: invalid integral value 'g' in '-Og'

For the first two errors, linker options are getting passed to the compiler and it doesn't like that. I fixed by separating it into CFLAGS and LDFLAGS.

For the second error, clang doesn't support -Og. -O1 seems really close (there's some chatter about that on a clang bug to get parity with this gcc-feature). But really, why not build this library with -O2?

Here's a patch. I can make it a pull request if you want.

--- a/rmutil/Makefile
+++ b/rmutil/Makefile
@@ -3,7 +3,8 @@ ifndef RM_INCLUDE_DIR
        RM_INCLUDE_DIR=../
 endif

-CFLAGS = -g -fPIC -lc -lm -O3 -std=gnu99 -I$(RM_INCLUDE_DIR) -Wall -Wno-unused-function
+CFLAGS = -g -fPIC -O3 -std=gnu99 -I$(RM_INCLUDE_DIR) -Wall -Wno-unused-function
+LDFLAGS = -g -lc -lm
 CC=gcc

 OBJS=util.o strings.o sds.o vector.o heap.o priority_queue.o
diff --git a/src/Makefile b/src/Makefile
index a8eace3..d553795 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -19,7 +19,8 @@ else
         SHOBJ_LDFLAGS ?= -bundle -undefined dynamic_lookup
 endif

-CFLAGS = -I$(RM_INCLUDE_DIR) -g -fPIC -lc -lm -Og -std=gnu99 
+CFLAGS = -I$(RM_INCLUDE_DIR) -g -fPIC -O2 -std=gnu99
+LDFLAGS = -g -lc -lm
 CC=gcc
 .SUFFIXES: .c .so .o

[EDIT: formatting]

[itamarhaber]

Ugh, no mac here - will need to check how to do this. @dvirsky - we had a mac issue at RedisConf, do you remember what it was?

As for -Og, that's my bad - should actually be -O3 in this repo - changing, thanks @neomantra.

[neomantra]

Those changes work for me on OSX -- I didn't try them on Linux, but LDFLAGS should be effective there too.

On 5/25/16 7:14 PM, Itamar Haber wrote:

Ugh, no mac here - will need to check how to do this. @dvirsky https://github.com/dvirsky - we had a mac issue at RedisConf, do you remember what it was?

As for |-Og|, that's my bad - should actually be |-O3| in this repo - changing, thanks @neomantra https://github.com/neomantra.

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/RedisLabsModules/redex/issues/6#issuecomment-221735409

[itamarhaber]

I can confirm that the changes work on Linux.

@dvirsky makes sense to do the same in all modules & the SDK, no?

[itamarhaber]

@neomantra please verify that this can be closed with https://github.com/RedisLabsModules/redex/commit/f2d35edf438f1ac2279c312056589ddf83af6efc

[neomantra]

Similarly, sorry I missed this many moons ago. Just built on OSX 10.12 this with the latest master without issue.