Use std filesystem instead of Boost.Filesystem if available.#7
Use std filesystem instead of Boost.Filesystem if available.#7Mike-Devel wants to merge 3 commits into
Conversation
Mike-Devel
force-pushed
the
std_filesystem
branch
from
f5a455b to
a7196a5
Compare
|
What special flags are needed on g++ 8? |
|
Afaik you have to explicitly link the filesystem library with '-lstdc++fs'. |
|
If that's true, these changes will break |
|
You mean because the cmake file will link to boost, but main.cpp will compile against std::filesystem? |
|
Same when using Is this worth the trouble? :-) |
|
The fix seems straight forward: Don't detect std::filesystem in source, but ifdef on a flag set by the build system. I'd consider it worth the trouble to propose the PR, because other than b2, cmake can't trigger the build of boost filesystem on demand (yet). Whether it is worth the trouble for you to review and maintain it I don't know. |
Mike-Devel
force-pushed
the
std_filesystem
branch
from
a7196a5 to
3f23585
Compare
| else() | ||
|
|
||
| message(STATUS "Using Boost::filesystem as filesystem library") | ||
| if( NOT TARGET Boost::filesystem ) |
There was a problem hiding this comment.
I'm not fond of if TARGET because it's order-dependent.
There was a problem hiding this comment.
True. Shall I remove it?
Mike-Devel
force-pushed
the
std_filesystem
branch
2 times, most recently
from
5f3aba3 to
88ac1c4
Compare
|
So, I removed the check for the boost::filesystem target and added a test to travis to make sure my changes don't break gcc 8. Do you think it is worth it to integrate into the main repo? If so, any other changes you want me to make? |
|
The fact that Apart from that, were I to integrate this, I'd probably drop all autodetections and just let the user choose |
Last time I checked, develop and master had the same problem, so I ignored it. It seems to be fixed on develop - will investigate why it still fails on this PR. |
|
No, they don't have this problem. The job fails because the exceptions differ, but it doesn't fail - like here - because the output is completely missing. |
Mike-Devel
force-pushed
the
std_filesystem
branch
2 times, most recently
from
2ce5c52 to
82b7665
Compare
Mike-Devel
force-pushed
the
std_filesystem
branch
from
82b7665 to
ed35ff1
Compare
Mike-Devel
force-pushed
the
std_filesystem
branch
from
bbd2879 to
0157181
Compare
|
OK, so I found the error: I accidentially inverted an The main remaining problem with this PR - as far as I can tell - is that no CI check actually covers the |
|
Ping. |
|
Keep it open. Hasn't the CMake community figured out how to link to stdc++fs or c++fs automatically as required? |
|
Not that I know of. Here is some information on the topic: https://gitlab.kitware.com/cmake/cmake/issues/17834. Seems it is less a question of how to do it, but more if they should/want to do it. |
Sounds like the GCC problem will be going away, but libc++ meanwhile decided to add libc++fs as well. |
|
Or maybe not. https://wandbox.org/permlink/tlXI44j9XzL7XUqr |
|
Considering that gcc still requires |
|
std::filesystem seems to work with latest trunk versions of clang and gcc (according to the wandbox link you posted). So I guess there is hope. |
2 participants
Makes the cmake based boostdep build independent of an installed boost library when you have a recent c++17 toolchain.
Afaik this currently only works with MSVC2017, as gcc-8 needs special linker flags for now and I don't think it is worth supporting that. I tested it locally, but unfortunately I'm not sure how to hook this up with appveyor CI.