icculus / physfs

A portable, flexible file i/o abstraction.
https://icculus.org/physfs/
zlib License
559 stars 98 forks source link

Static and Shared build #72

Open ntropy83 opened 1 year ago

ntropy83 commented 1 year ago

Hi there,

my physfs-static lib build fails on Ubuntu in Github Actions, works on windows tho. It works on my Arch Linux desktop and Ubuntu desktop without problems, its only github Actions that fails. What could I be missing, don't know the whole idea behind the lib yet.

https://github.com/ntropy83/tinyGameEngine/actions/runs/6870720894

The Cmake configuration for my physfs implementation is in the folder engine.

Thanks for your help in advance ! Greetings ent

icculus commented 1 year ago

The windows build appears to have compiled PhysicsFS and the Linux build doesn't appear to be doing so.

At least, I don't see any physfs source files in the build output on Linux...?

ntropy83 commented 1 year ago

The cmake for the windows and Linux built is basically the same, there is only one little difference with Linux where I link against X11 libraries.

I managed to get it compiled on both now, I have added a conditional check in the cmake so it builds physfs as shared lib on Linux and as static lib for Windows that way it compiled for both OS'es.

If you have the time, there is only little code around physfs in my program. In the main cmake, I have the conditional check and link against the target. And then in the submodule cmake I configure physfs, maybe you can have a quick look at that and see if I have messed up something: https://github.com/ntropy83/tinyGameEngine/blob/main/engine/CMakeLists.txt

Thank you in advance and have a nice weekend, Greetings

Epixu commented 1 year ago

Hi there, sorry for interrupting, @ntropy83, but I noticed that you use modern CMake version, so here's how I managed to add physfs to my engine's plugin module, might give you some ideas. (here's the fetch_external_module bit)

It builds fine on both Ubuntu/Windows

I suspect it might have something to do with project(LANGUAGES CXX) and picking files based on that - it kind of acts as a filter. I vaguely remember having problems with that, but can't be exactly sure. I would advice against specifying project language there either way, if you have proper file extensions.

Here's a related problem I had: https://discourse.cmake.org/t/c-file-is-only-compiled-when-it-is-used-by-add-subdirectory/754

ntropy83 commented 1 year ago

@Epixu thanks for your idea, I will have a deeper look at your solution.

The "LANGUAGES CXX" definition in the cmake project command is really unnecessary, I have deleted it. Yet it still didn't fix the lib linking error. At the moment its building the static lib on Windows but then tries to link against the shared one, it may seem: https://github.com/ntropy83/tinyGameEngine/actions/runs/6916342603

Nevertheless my conditional linker check solution is working so far and I can go on from here. :)

Epixu commented 1 year ago

I believe you should link against the target physfs-static here, you are currently linking against the shared library target physfs @ntropy83

ntropy83 commented 1 year ago

@Epixu actually the Github Actions Workflow target for Ubuntu builds, when I use physfs and for windows when I use physfs-static. Thats what I did with my actual CmakeLists.txt: https://github.com/ntropy83/tinyGameEngine/blob/40efa509f419a663dbbcb4b362204116678a2542/CMakeLists.txt#L93C1-L93C1

This problem tho only persits with github actions workflows OS images, on a local build on Arch Linux and a crosscompile to Windows it never gives a problem with physfs.

madebr commented 9 months ago

Does this still reproduce? The github actions logs have been purged.

You need to use (current) PhysicsFS using PhysFS::PhysFS* target(s).

if(TARGET PhysFS::PhysFS)
  target_link_libraries(mygame PRIVATE PhysFS::PhysFS)
else()
  target_link_libraries(mygame PRIVATE PhysFS::PhysFS-static)
endif()