parapluu / nifty

Erlang NIF Wrapper Generator
http://parapluu.github.io/nifty/
Other
142 stars 29 forks source link

more flexible rebar.config #34

Closed namjae closed 8 years ago

namjae commented 8 years ago

hi, I got following build error on Ubuntu 14.04LTS.

c_src/nifty_clangparse.c:10:27: fatal error: clang-c/Index.h: No such file or directory

this patch fixed above build error.

kostis commented 8 years ago

Thanks for your pull request. Is it possible to rebase this on the current master and squash both commits into one, so that we merge this by just clicking on github rather than manually?

TheGeorge commented 8 years ago

I am a bit reluctant to accept the pull request yet.

While I think we should absolutely use llvm-config to get the paths, we are using the paths in Travis to select the llvm version we are testing against.

I think we can select the version with an environment variable (like NIFTY_LLVM_VERSION) and set it in .travis.yml like this:

{port_specs, [
    {".*", "priv/nifty_clangparse.so", ["c_src/nifty_clangparse.c"], [{env,[
           {"CFLAGS", "`llvm-config$NIFTY_LLVM_VERION --cflags`"},
           {"LDFLAGS", "$LDFLAGS -lclang `llvm-config$NIFTY_LLVM_VERSION --ldflags`"}]}]},
    {"priv/nifty.so", ["c_src/nifty.c"]}
    ]}.

and also

--- a/.travis.yml
+++ b/.travis.yml
@@ -8,10 +8,10 @@ otp_release:
   - 17.4
   - 17.0
 env:
-  - CPATH=/usr/lib/llvm-3.4/include LIBRARY_PATH=/usr/lib/llvm-3.4/lib LD_LIBRARY_PATH=/usr/lib/llvm-3.4/lib
-  - CPATH=/usr/lib/gcc/x86_64-linux-gnu/4.6/include:/usr/lib/llvm-3.6/include LIBRARY_PATH=/usr/lib/llvm-3.6/lib LD_LIBRARY_PATH=/usr/lib/llvm-3.6/lib
-  - CPATH=/usr/lib/gcc/x86_64-linux-gnu/4.6/include:/usr/lib/llvm-3.7/include LIBRARY_PATH=/usr/lib/llvm-3.7/lib LD_LIBRARY_PATH=/usr/lib/llvm-3.7/lib
-  - CPATH=/usr/lib/gcc/x86_64-linux-gnu/4.6/include:/usr/lib/llvm-3.8/include LIBRARY_PATH=/usr/lib/llvm-3.8/lib LD_LIBRARY_PATH=/usr/lib/llvm-3.8/lib
+  - NIFTY_LLVM_VERSION=-3.4
+  - NIFTY_LLVM_VERSION=-3.6
+  - NIFTY_LLVM_VERSION=-3.7
+  - NIFTY_LLVM_VERSION=-3.8
namjae commented 8 years ago

hmm.. i don't know i just did was right or not.

kostis commented 8 years ago

@TheGeorge Yes, I think that your proposal kills two birds with one stone and we should adopt it.

Just make sure that the manual is updated to mention that nifty takes into account the environment variable NIFTY_LLVM_VERSION if it is set.

TheGeorge commented 8 years ago

this will be handled in #35