tomassedovic / tcod-rs

Rust bindings for libtcod 1.6.3 (the Doryen library/roguelike toolkit)
Do What The F*ck You Want To Public License
229 stars 45 forks source link

Upgrade libtcod to 1.12.3 #290

Closed abesto closed 3 years ago

abesto commented 5 years ago

Things to note:

tomassedovic commented 5 years ago

Thanks! And sorry for the conflicts!

Updating Cargo.toml should be trivial and feel free to disregard any changes to sys_sdl2_c.c and use your version for now.

The CI issues are real. I've rebased this and got to the same issues. The Linux verison is using ANSI C, but this relies on C++ 14.

We need to replace this line with config.flag("-std=c++14");:

https://github.com/tomassedovic/tcod-rs/blob/718d8d95f70d9f8556e017d4d2143082f2ec78d4/tcod_sys/build.rs#L65

(and similar in build_linux_dynamic, mingw and darwin)

That got me to build tcod-sys but tcod itself (the safe bindings) failed with a bunch of errors.

From the PR message, I'm not sure whether that's where you are (i.e. you're using tcod-sys directly) or whether you can actually build tcot itself and it just doesn't have the new libtcod stuff.

abesto commented 5 years ago

We need to replace this line with config.flag("-std=c++14")

Seems straightforward enough. I'll do it once I pick this up again, I'd guess in a couple of days.

That got me to build tcod-sys but tcod itself (the safe bindings) failed with a bunch of errors.

From the PR message, I'm not sure whether that's where you are (i.e. you're using tcod-sys directly) or whether you can actually build tcot itself and it just doesn't have the new libtcod stuff.

Oops, I'm starting to suspect I'd need to update the root-level Cargo.toml for tcod to pick up the new tcod-sys? I didn't touch the top-level Cargo.toml, and tcod compiled fine; I was surprised, but happy. Guess it's not that simple after all :)

tomassedovic commented 5 years ago

I think tcod should be picking up the new tcod-sys version. If building tcod works for you, it's possibly another linking issue somewhere.

I'm seeing the same issue when I do update the version to 5.1.0 locally.

abesto commented 5 years ago

I've just added c++14, and updated the top-level Cargo.toml as well for good measure. Could compile cleanly under Ubuntu (FWIW, in the Linux subsystem of Windows 10):

Compiling tcod-sys v5.3.0 (/mnt/c/Users/abesto/Documents/GitHub/tcod-rs/tcod_sys)
Compiling tcod v0.14.0 (/mnt/c/Users/abesto/Documents/GitHub/tcod-rs)
Finished dev [unoptimized + debuginfo] target(s) in 55.86s   

Let's see what CI thinks. If it's sad, I'll then try to reproduce on a proper Linux system.

Meanwhile, if it's easy for you to reproduce, it might be good to have a full build output with the errors from your env.

abesto commented 5 years ago

Huh, rebased to tomassedovic:master, getting build errors now. Lemme try to resolve them.

abesto commented 5 years ago

Bah, never mind, just messed up the conflict resolution of tcod_sys/libtcod/src/libtcod/sys_sdl2_c.cpp.

abesto commented 5 years ago

Meh, Travis fails because ... it seems to not know about C++14? I'm having trouble fully believing that, but then https://github.com/libtcod/libtcod/blob/master/.travis.yml is also doing some magic around selecting the compiler. I'm out of time to spend on tracking that down for today, help is appreciated; I will be coming back to this, but no promises on the timeline.

On Appveyor I'm seeing one success so far, second build is in progress, so it's starting to smell like a Win / Linux env difference.

abesto commented 5 years ago

Alright, so all this work got us a working cargo build on Travis Linux, but cargo test is failing, because ... one of the examples are ... not linking against stdlib? Either way, that's all for today.

What we learned:

abesto commented 5 years ago

We have a goodnews-badnews situation.

Good news: I managed to reproduce the issue locally, and found some leads to fixing it. Bad news: I couldn't fix it. Halp!

Details! I was almost right: the failure is due to not linking against stdc++, during static compilation specifically (see https://stackoverflow.com/questions/10906275/undefined-reference-to-stdios-baseinitinit). Accordingly, I tried adding config.flag("-lstdc++"); to relevant-looking places, but this does nothing (probably obvious, if I dug deeper into how the full build process here works).

However! It seems that linking against C++ libraries is not very easy in Rust today. The conversation at https://users.rust-lang.org/t/linking-against-c-dependencies/25099/4 is trying to solve a very similar problem. I've tried println!("cargo:rustc-link-lib=static=stdc++"); and received the same message as noted in that thread (error: could not find native static librarystdc++, perhaps an -L flag is missing?). They then mention static-nobundle (which only works on nightly Rust); unfortunately in our case this brings back the original compiler errors. The resolution in the thread seemed to be a combination of static-nobundle and ... moving from gcc to clang? I'm not 100% clear on that.

So at this point I think the problem is well identified, and I've showed a few "solutions" that don't work; I'm escalating to you all, hopefully at this point someone will go "right, you just need to X".

HexDecimal commented 4 years ago

Current maintainer of libtcod here.

The latest versions of libtcod had their C++ dependencies removed and can now be compiled without any C++ sources. You'll only need C99 support from the tool-chain.

abesto commented 3 years ago

Closing this in favor of #309 - there's more progress there, some great simplifications, and it also targets a newer libtcod release.