icculus / physfs

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

rename src/ to physfs/ #71

Open duarm opened 1 year ago

duarm commented 1 year ago

just a minor inconvenience, but when opting to just drop physfs into our project (as advised in CMakeLists.txt), it's inconvenient to just add -Ipath/to/physfs/, because we would then we would have to include #include <src/physfs.h>. If we use -Ipath/to/physfs/src/ we would then include #include <physfs.h>. Because I prefer prefixed headers, I would like to include like this <physfs/physfs.h>, but there's no way without renaming or symlinking or creating a new file.

We also don't have an include/, so one less reason for the src/ folder.

madebr commented 9 months ago

When you drop physfs in your project, you ideally only need to do the following and don't need to worry about include paths:

add_subdirectory(third_party/physfs)
target_link_libraries(mygame PRIVATE PhysFS::PhysFS)

Thanks to these lines: https://github.com/icculus/physfs/blob/741c4be358cd87366af1387d1e04e3f7d4f50c1e/CMakeLists.txt#L186 https://github.com/icculus/physfs/blob/741c4be358cd87366af1387d1e04e3f7d4f50c1e/CMakeLists.txt#L206

duarm commented 9 months ago

I'm not using cmake, thanks for the tip anyway. This issue is just a suggestion, because I much prefer prefixed includes like <physfs/physfs.h>. If you're dropping physfs onto your project, you can just rename src/ to physfs/, or just symlink src to physfs/ if you don't want to mess with the git tree

madebr commented 9 months ago

Your issue/nuisance is valid though. For SDL3, the headers were moved to include/SDL3. Projects now can include SDL3/SDL.h, no matter how they get SDL3. A PhysicsFS install prefix currently looks as below, so we would need to move physfs.h to an include folder (and no physfs subfolder).

prefix
├── bin
│   └── test_physfs
├── include
│   └── physfs.h
└── lib
    ├── cmake
    │   └── PhysFS
    │       ├── PhysFSConfig.cmake
    │       └── PhysFSConfig-debug.cmake
    ├── libphysfs.a
    ├── libphysfs.so -> libphysfs.so.1
    ├── libphysfs.so.1 -> libphysfs.so.3.3.0
    ├── libphysfs.so.3.3.0
    └── pkgconfig
        └── physfs.pc
madebr commented 9 months ago

See https://github.com/icculus/physfs/pull/76

icculus commented 9 months ago

Let's do this, but for PhysicsFS 4. Changing it now would be a surprising change to most people using the code that probably breaks their builds.

In the meantime, a hack would be to make a physfs.h file in whatever directory makes sense for your project, that simply contains #include "../somewhere_else/physfs/src/physfs.h" and maybe a link to this issue in a comment.

And maybe an extra comment cursing my name for being difficult. :)

madebr commented 9 months ago

In the meantime, a hack would be to make a physfs.h file in whatever directory makes sense for your project, that simply contains #include "../somewhere_else/physfs/src/physfs.h" and maybe a link to this issue in a comment.

I've removed the header move in #41. Perhaps there's no issue at all here.

In the SDL2 branch, headers are stored in a include subfolder. Installed headers in <prefix>/include/SDL2. So you couldn't do #include <SDL2/SDL.h>. This is no problem with PhysicsFS, because you can only do #include <physfs.h>. Doing #include <physfs/physfs.h> is "not supported" by this project's build scripts.