novelrt / NovelRT

A cross-platform 2D game engine accompanied by a strong toolset for visual novels.
MIT License
184 stars 42 forks source link

CMake: Potential Out of Order building causing errors depending on Generator/OS/etc #481

Open capnkenny opened 2 years ago

capnkenny commented 2 years ago

Self-titled - essentially, depending on a number of different factors (which are hard to isolate), there is a potential for CMake to try and build things out of order even though there are dependencies listed, causing numerous errors, such as:

It seems that the more threads a device has to build with, the less likely this is actually going to happen - it's prominent to happen on CI for Windows 2019/2022, but locally and in Ubuntu/macOS it's not reproducible at this time. We may want to revamp our CMakeLists soon to get everything in order and to stop this from occurring.

RubyNova commented 2 years ago

Is this CI only? Is it going to affect our release?

capnkenny commented 2 years ago

There's a potential. At the moment it's CI only though

RubyNova commented 2 years ago

What happens if we force single-threaded?

capnkenny commented 2 years ago

I'm not sure - to be clear, the main issue is how CMake is choosing the order to build the items, which is something internal to CMake and not really controlled by us. If CMake decides to do copy_build_products_to_* before Engine, for example, then effectively nothing will get DLLs because it thinks it has copied everything already.

Since this is more of a CI concern (it hasn't really been reported locally yet), I don't think we need to do something prior to release, but its something to certainly visit afterwards for sure.

RubyNova commented 2 years ago

I'm just worried this is going to suddenly break on us. It shouldn't be doing this.

capnkenny commented 2 years ago

I don't think you have to worry that this should suddenly break - however I will say that if we continue to further add dependencies the way we are currently doing so, it's going to become more difficult to manage and this may show itself more without additional changes.