search

crystal-lang / crystal_lib

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

Add Crystal 0.25+ support #52

Closed olbat closed 6 years ago

olbat commented 6 years ago

The CI is failing because SizeT is not binded on the same type when running on Linux and MacOS.

I guess the difference can be explained by the different libc (glibc/clang) ...

Any idea on how it should be fixed ?

bcardiff commented 6 years ago

But they are binding to the same value in linux and darwin (64 bits). From the CI message

       Expected: "lib LibSome\nfun just_size_t : LibC::Int\nend"
            got: "lib LibSome\nfun just_size_t : LibC::SizeT\nend"

It might be the case that the expectation is wrongly translated. The goal is to have that LibC::SizeT in the output so the output is portable.

Disclaimer: I am not really familiar with crystal_lib though.

olbat commented 6 years ago

But they are binding to the same value in linux and darwin (64 bits).

Are you sure of that ? because on the CI this test is either failing on Linux or macOS ...

I think the problem on Linux is maybe related to this warning ... I'm trying to find a way to fix it.

olbat commented 6 years ago

I pushed a fix however I'm not sure it's the best way to do it.

Fryguy commented 6 years ago

Great work @olbat ...looking forward to this getting merged!

Fryguy commented 6 years ago

Ugh I guess you do need that fix 😕 ☝️ https://github.com/crystal-lang/crystal_lib/pull/52#discussion_r202545832

olbat commented 6 years ago

I tried to improve the fix a little bit ... Now I'm looking for the builtin headers' directory in directories of clang's default search path.

Does it look OK to you ?

RX14 commented 6 years ago

It looks good to me, but i'd like a review from someone who's worked on this codebase before.

ysbaddaden commented 6 years ago

@olbat yes, this project could be based off clang.cr if needed.

olbat commented 6 years ago

@ysbaddaden okay, nice :)

I think it could be nice centralize the efforts if it's okay to add an external dependency to this lib ... If the maintainers (@bcardiff @RX14 ?) are ok with that, I'll try to find some time to take a look (in another PR ofc).

RX14 commented 6 years ago

I'm unfamiliar with this codebase. Judging by the graphs, it's mainly @asterite and @ysbaddaden who should decide

ysbaddaden commented 6 years ago

Thanks for your efforts!

olbat commented 6 years ago

No problem, thank you for the help and the review ! :)

olbat commented 6 years ago

@olbat yes, this project could be based off clang.cr if needed.

see #56