Skip to content

fix(compat): Add drop-in std::filesystem header for older systems#952

Merged
stv0g merged 2 commits into
masterfrom
compat-std-filesystem
Aug 26, 2025
Merged

fix(compat): Add drop-in std::filesystem header for older systems#952
stv0g merged 2 commits into
masterfrom
compat-std-filesystem

Conversation

@stv0g
Copy link
Copy Markdown
Contributor

@stv0g stv0g commented Aug 21, 2025

Needed for some older compilers which do not include std::filesystem by default.

@stv0g stv0g requested a review from n-eiling as a code owner August 21, 2025 08:56
@n-eiling
Copy link
Copy Markdown
Member

Looks good.
Looks in the CI as if ghc_filesystem is the default? Shouldn't we prefer std::filesystem?
Also, do you think it's a good idea to include the third party code as a copy in this repo? I always prefer to download it from the original repository. This reduces license issues and nobody can get the idea of modifying the file. Also updating the dependency should be cleaner.

@stv0g
Copy link
Copy Markdown
Contributor Author

stv0g commented Aug 21, 2025

Looks in the CI as if ghc_filesystem is the default? Shouldn't we prefer std::filesystem?

Dang, I've implement a CMake check to see if the library is available.. However, that does not seem to work.. I will check.

Also, do you think it's a good idea to include the third party code as a copy in this repo?

I agree. But I am not sure how we want to implement this? Git submodules? CMake's ExternalProject? Or via out deps.sh shell script?

@stv0g
Copy link
Copy Markdown
Contributor Author

stv0g commented Aug 21, 2025

I've removed the external header file. Instead we now rely on CMake's find_package() for finding the header-only library (ghc::filesystem has good CMake support for it to work flawlessly).

@stv0g
Copy link
Copy Markdown
Contributor Author

stv0g commented Aug 21, 2025

And the deps.sh script has been adjusted to also install the library.

@stv0g stv0g force-pushed the compat-std-filesystem branch from 6fe4742 to 209601c Compare August 21, 2025 13:51
n-eiling
n-eiling previously approved these changes Aug 21, 2025
Copy link
Copy Markdown
Member

@n-eiling n-eiling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great :)

@stv0g stv0g force-pushed the compat-std-filesystem branch 5 times, most recently from 3f06fd0 to b332c13 Compare August 26, 2025 06:32
Signed-off-by: Steffen Vogel <steffen.vogel@opal-rt.com>
Signed-off-by: Steffen Vogel <steffen.vogel@opal-rt.com>
@stv0g stv0g force-pushed the compat-std-filesystem branch from b332c13 to f2b265d Compare August 26, 2025 06:44
@stv0g stv0g merged commit 186e880 into master Aug 26, 2025
3 checks passed
@stv0g stv0g deleted the compat-std-filesystem branch August 26, 2025 07:01
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.

3 participants