search

gulrak / filesystem

An implementation of C++17 std::filesystem for C++11 /C++14/C++17/C++20 on Windows, macOS, Linux and FreeBSD.
MIT License
1.34k stars 173 forks source link

Minimum deployment target check fails in fs_std.hpp, unless Availability.h is included #114

Closed stas2 closed 3 years ago

stas2 commented 3 years ago

I want to make use of the dynamic detection feature by only including ghc/fs_std.hpp. However if only ghc/fs_std.hpp is included, then detection fails when building on Mac for 10.14 target.

Apparently it fails, because it uses __MAC_OS_X_VERSION_MIN_REQUIRED macro definition, which is defined by Availability.h from MacOSX SDK, which is not included prior to fs_std.hpp.

To make the detection work, I need to explicitly include before including fs_std.hpp.

Should it also be included in ghc/fs_std.hpp for the detection to work properly, without additional includes by the user?

This also appears to be the case for other headers. Availability.h is also later included in filesystem.hpp, but it is included after the system detection.

It looks like this platform detection code could be extracted from filesystem.hpp (lines 54-94) into a separate internal header file and shared between public header files. What do you think? I am willing to make this change and submit a PR, however I will not be able to test on all platforms.

gulrak commented 3 years ago

Thanks for the report, I'll look into the issue. :+1:

I still want filesystem.hpp to be a complete single-file implementation usable without the rest of the headers. It is used by quite some projects in that form and that is the reason it is an additional separate artifact of every release. This could be offered by some amalgamation tool, but it would be to much hassle for the platform detection only. Without the single-file feature, I for sure would prefer separating it out too, so thanks for the suggestion.

gulrak commented 3 years ago

This is now part of release v1.5.6