search

rpav / c2ffi

Clang-based FFI wrapper generator
GNU Lesser General Public License v2.1
234 stars 38 forks source link

some changes/fixes that i needed to make it build on NixOS 21.05 #97

Closed attila-lendvai closed 3 years ago

attila-lendvai commented 3 years ago

let me know if i should split this into multiple parts, or if i should change anything.

i'm no expert of cmake, i just struggled through the obstacles.

attila-lendvai commented 3 years ago

@rpav i would appreciate if you could take a look at it at your earliest convenience, because the nix c2ffi PR depends on it (https://github.com/NixOS/nixpkgs/pull/119564).

rpav commented 3 years ago

Sorry I'll try and look at this today.

attila-lendvai commented 3 years ago

Sorry I'll try and look at this today.

much appreciated! but don't break your neck over it.

rpav commented 3 years ago

Do we really need the change in CMakeLists.txt if the package file does this anyway? This doesn't seem to be an issue on other platforms and while it's not an issue in this particular version, I'm not sure depending on RTTI being disabled in the long term is a good idea. (Some recent C++ features use rtti indirectly as I recall.)

attila-lendvai commented 3 years ago

the reason i moved it there is what i have found in the comment: LLVM releases are compiled without RTTI. IOW, one cannot be sure whether RTTI is compiled into the LLVM binaries he is linking to.

i'm not well versed in C++ at all, especially in the latest developments, so, if you think it's a bad idea, then i can move it back into the nix file.

attila-lendvai commented 3 years ago

@rpav any opinion on this? i'm willing to update it to whichever solution you think is better.

rpav commented 3 years ago

Crap sorry fell off my radar again .. let's do this in the package file for now, I'll try and take a look at the necessity for everything else.

attila-lendvai commented 3 years ago

@rpav ok, i removed the commit that moved the rtti stuff from the nix file into the cmake file.

please note that Arch latest has moved on to llvm v12, and it breaks the CI. i don't know how to fix it, i don't know how to force arch to install llvm v11.

rpav commented 3 years ago

Sorry I'll merge this soon.

rpav commented 3 years ago

Sorry for the delay; should be looking at clang-12 shortly too.