crystal-lang / crystal_lib

Automatic binding generator for native libraries in Crystal
138 stars 30 forks source link

Support Crystal 0.26.0 #54

Closed Fryguy closed 6 years ago

Fryguy commented 6 years ago

https://github.com/crystal-lang/crystal/pull/6432 changed the signature of Crystal::Alias.

Fixes #53

Fryguy commented 6 years ago

While this passes, we also get the following weird output on a Mac: https://travis-ci.org/crystal-lang/crystal_lib/jobs/417424919#L1607-L1621 (link is for LLVM 6.0.1, but it happens with any LLVM version). I'm not sure what to make of this, as I didn't see it previously with Crystal 0.25.0. @ysbaddaden @olbat Thoughts?

EDIT: Apparently it also happens with Crystal 0.25.0, so sounds like an independent issue that can be solved separately: (Travis link from #52 which added the Crystal 0.25.0 support : https://travis-ci.org/crystal-lang/crystal_lib/jobs/405205288#L1541-L1556 )

olbat commented 6 years ago

You're right this output seems to have been introduced by #52 😞 I'll try to find some time to take a look ...

Fryguy commented 6 years ago

@olbat Commenting out https://github.com/crystal-lang/crystal_lib/blob/1122d5a6788f5aa12a4e4ecc0ef191e612d9f156/src/clang/index.cr#L12 and https://github.com/crystal-lang/crystal_lib/blob/1122d5a6788f5aa12a4e4ecc0ef191e612d9f156/src/clang/index.cr#L20 makes those go away so perhaps all those need is a darwin check?

ysbaddaden commented 6 years ago

Weird, that should be the other way around —including default directories should avoid these issues. Maybe they're not required on OS X with default compiler.

Fryguy commented 6 years ago

Narrowing down, this only happens with one particular spec:

LLVM_CONFIG=/usr/local/Cellar/llvm/6.0.1/bin/llvm-config crystal spec spec/lib_body_transformer_spec.cr:23

https://github.com/crystal-lang/crystal_lib/blob/1122d5a6788f5aa12a4e4ecc0ef191e612d9f156/spec/lib_body_transformer_spec.cr#L23-L25

Fryguy commented 6 years ago

The default_include_directories method, on my machine, gives

args # => ["-I/usr/local/include",
 "-I/Library/Developer/CommandLineTools/usr/lib/clang/9.1.0/include",
 "-I/Library/Developer/CommandLineTools/usr/include",
 "-I/usr/include",
 "-I/System/Library/Frameworks (framework directory)",
 "-I/Library/Frameworks (framework directory)"]

It turns out that second one, /Library/Developer/CommandLineTools/usr/lib/clang/9.1.0/include, is the one causing the output. If I remove that from the list, then the output goes away.

Fryguy commented 6 years ago

@olbat @ysbaddaden I added a commit to skip the default directory check on macOS, directly in the default_include_directories method.

olbat commented 6 years ago

Hi @Fryguy, I tried to reproduce on a mac and got the same issue. Thank you for the debugging 👍

I googled for the issue a little bit and found several persons having the same problem after migrating to macOS 10.13 1, 2, 3. It's probably related ... Did you try to remove /usr/local/include from your include path (or to rename the directory as suggested in the links, at least for testing purposes) ?

Also, I'm not a big fan of the _disable default_includedirectories on macOS fix since it may have some unwanted side-effects (i.e. for non-10.13 releases). What do you thing @ysbaddaden ?

BTW, I think that you pointed out one more issue here: some include paths are containing the "(framework directory)" string, see #55.

ysbaddaden commented 6 years ago

I don't know macOS so I can't tell what's good or not :(

ysbaddaden commented 6 years ago

Maybe just skip /usr/local/include for Darwin?

olbat commented 6 years ago

Even if it only affects a specific version of it ?

I'm not sure but I think that the widely used brew package manager save header files in this directory (that why I'm thinking that it may cause issues). @Fryguy can you confirm ?

Fryguy commented 6 years ago

@olbat That is correct. brew works by symlinking from /usr/local/include, bin, lib, etc. I have a ton of stuff in my /usr/local/include .

Fryguy commented 6 years ago

Tried a few more experiments...

  1. Removed the darwin gate and then coded in a line to remove /usr/local/include from the default_include_directories :x:
  2. Removed the darwin gate and just renamed /usr/local/include to /usr/local/include.bak, :x:
  3. combination of 1 and 2 :x:
  4. I've been running with LLVM_CONFIG env var, so instead of that I did a brew link llvm
  5. Removed the darwin gate and tried 4 again ✅

So, for me it seems using LLVM_CONFIG is the problem, however the Travis runs don't do that, so I'm not sure why Travis is also having the problem. The only difference I can think of between my local install and Travis is I don't have full xcode installed; I only have the xcode command line tools.

Some more information if it helps:

# When *not* linked with `brew link llvm`
> /usr/local/Cellar/llvm/6.0.1/bin/llvm-config --ldflags
-L/usr/local/Cellar/llvm/6.0.1/lib -Wl,-search_paths_first -Wl,-headerpad_max_install_names

Interestingly, there is a /usr/local/Cellar/llvm/6.0.1/lib/clang/6.0.1/ directory that has both a lib and include directory in there. As mentioned earlier, default_include_directories has /Library/Developer/CommandLineTools/usr/lib/clang/9.1.0/include (notice the clang version difference), since xcode command line tools comes with clang. Since removing the clang/9.1.0 path seemed to fix the issue, I think that difference is part of the problem.

Fryguy commented 6 years ago

I removed the darwin skip from this PR for now, as I'm thinking we should deal with this in a separate PR. The commit that actually gets Crystal 0.26.0 working is seemingly uncontroversial, and we shouldn't hold up merging it for this, IMO.

@ysbaddaden Are you ok with that?

olbat commented 6 years ago

Can someone merge it?