Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use GHC::filesystem for std::filesystem compatibility #1093

Merged
merged 1 commit into from
Jan 7, 2025

Conversation

dpogue
Copy link
Contributor

@dpogue dpogue commented Jan 5, 2025

This brings in https://github.com/gulrak/filesystem as a "polyfill" for the C++17 filesystem API, particularly on older macOS versions where it's not part of the system C++ library. This should resolve #1065.

If possible, it will use the std::filesystem API, but when targeting macOS versions without support it will use the GHC implementation instead.

This uses the latest master branch of gulrak/filesystem because the current tagged release doesn't work properly on macOS.

I opted to just drop this in the common folder for ease of use, since it's a single file and that's what was done with pffft, but I could try to put it in a top-level vendor folder similar to fmt if that's the preferred approach.

@dpogue dpogue force-pushed the filesystem-polyfill branch from dd5bc24 to 4f632c1 Compare January 5, 2025 22:28
@dpogue dpogue force-pushed the filesystem-polyfill branch from 4f632c1 to 8259335 Compare January 5, 2025 22:44
@kcat
Copy link
Owner

kcat commented Jan 7, 2025

I'm not very keen on adding a custom implementation of an API that's part of the required standard, for an OS that's over 7 years old and didn't update to improve support. Simple workarounds would be fine, but a full implementation seems much. Especially since the next (non-point) release will be increasing the standard requirement to C++20, so won't be useful just after being added.

Is this really all that's needed to make it work for the system?

@dpogue
Copy link
Contributor Author

dpogue commented Jan 7, 2025

Agree that a full filesystem implementation is probably overkill, particularly given how minimal the use of it is in openal-soft, but this seemed like the option that involved the least code changes by swapping in something with the same API.

The issue on macOS is that the C++ runtime is provided by the system and doesn't get updated. So even if you build with a newer compiler version, you can't use APIs that weren't part of the system C++ runtime of the target system. It's theoretically possible to do static linking, but that's very uncommon and potentially introduces incompatibilities with system frameworks like CoreAudio. Previous attempts at mixing C++ runtimes have resulted in issues like pointers being malloc'ed with one runtime and free'd in another and throwing errors.

Unfortunately it comes down to implementation details which parts of the C++ standard are compiled away and which parts involve additions to the runtime, but filesystem is definitely one that isn't available on older macOS versions.

@kcat kcat merged commit fb88224 into kcat:master Jan 7, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fatal error: 'filesystem' file not found
2 participants