Skip to content

Fix use of std::filesystem on older GCC and Clang#11012

Closed
phongn wants to merge 3 commits intoapache:masterfrom
phongn:fix-std-filesystem
Closed

Fix use of std::filesystem on older GCC and Clang#11012
phongn wants to merge 3 commits intoapache:masterfrom
phongn:fix-std-filesystem

Conversation

@phongn
Copy link
Copy Markdown
Collaborator

@phongn phongn commented Jan 25, 2024

Older versions of GCC (< 9.1) and Clang (< 9) required use of additional library linking flags in order to use std::filesystem. This PR instructs cmake to use those libstdc++/libc++ filesystem libraries when appropriate.

@phongn phongn requested review from JosiahWI and cmcfarlen January 25, 2024 17:56
@phongn phongn self-assigned this Jan 25, 2024
@phongn phongn added Build work related to build configuration or environment CMake work related to CMakes scripts or issues labels Jan 25, 2024
@phongn phongn requested a review from bneradt January 25, 2024 17:56
cmcfarlen
cmcfarlen previously approved these changes Jan 29, 2024
Copy link
Copy Markdown
Contributor

@cmcfarlen cmcfarlen left a comment

Choose a reason for hiding this comment

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

I don't have an older compiler to check with, but if this works for you and doesn't break the build then Im 👍

@ezelkow1
Copy link
Copy Markdown
Member

ezelkow1 commented Jan 31, 2024

This didnt seem to fix it for me (on gcc 8.5)

[ 52%] Building CXX object src/iocore/cache/CMakeFiles/Cache.dir/unit_tests/test_Cache.cc.o
[ 52%] Linking CXX executable Cache
libinkcache.a(Cache.cc.o): In function persist_bad_disks()': Cache.cc:(.text+0x56c1): undefined reference to std::filesystem::__cxx11::path::parent_path() const'
Cache.cc:(.text+0x56d7): undefined reference to std::filesystem::create_directories(std::filesystem::__cxx11::path const&, std::error_code&)' libinkcache.a(Cache.cc.o): In function std::filesystem::__cxx11::path::operator/=(std::filesystem::__cxx11::path const&)':
Cache.cc:(.text.ZNSt10filesystem7__cxx114pathdVERKS1[ZNSt10filesystem7__cxx114pathdVERKS1]+0x3d): undefined reference to std::filesystem::__cxx11::path::has_filename() const' Cache.cc:(.text._ZNSt10filesystem7__cxx114pathdVERKS1_[_ZNSt10filesystem7__cxx114pathdVERKS1_]+0x98): undefined reference to std::filesystem::__cxx11::path::_M_split_cmpts()'
libinkcache.a(Cache.cc.o): In function std::filesystem::__cxx11::path::is_absolute() const': Cache.cc:(.text._ZNKSt10filesystem7__cxx114path11is_absoluteEv[_ZNKSt10filesystem7__cxx114path11is_absoluteEv]+0x14): undefined reference to std::filesystem::__cxx11::path::has_root_directory() const'
libinkcache.a(Cache.cc.o): In function std::filesystem::__cxx11::path::path<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::filesystem::__cxx11::path>(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::filesystem::__cxx11::path::format)': Cache.cc:(.text._ZNSt10filesystem7__cxx114pathC2INSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEES1_EERKT_NS1_6formatE[_ZNSt10filesystem7__cxx114pathC5INSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEES1_EERKT_NS1_6formatE]+0x64): undefined reference to std::filesystem::__cxx11::path::_M_split_cmpts()'
libinkcache.a(Cache.cc.o): In function std::filesystem::__cxx11::path::path<char const*, std::filesystem::__cxx11::path>(char const* const&, std::filesystem::__cxx11::path::format)': Cache.cc:(.text._ZNSt10filesystem7__cxx114pathC2IPKcS1_EERKT_NS1_6formatE[_ZNSt10filesystem7__cxx114pathC5IPKcS1_EERKT_NS1_6formatE]+0x64): undefined reference to std::filesystem::__cxx11::path::_M_split_cmpts()'
collect2: error: ld returned 1 exit status
gmake[2]: *** [src/iocore/cache/CMakeFiles/Cache.dir/build.make:185: src/iocore/cache/Cache] Error 1
gmake[1]: *** [CMakeFiles/Makefile2:4225: src/iocore/cache/CMakeFiles/Cache.dir/all] Error 2
gmake: *** [Makefile:146: all] Error 2

just to double check I do see new cmake changes in my branch

Older LLVM and GCC compilers with C++17 support require explicit linking to their std::filesystem

libraries

if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" AND CMAKE_CXX_COMPILER_VERSION VERSION_LESS 9.1)
target_link_libraries(traffic_server PRIVATE stdc++fs)
endif()

if(CMAKE_CXX_COMPILER_ID STREQUAL "Clang" AND CMAKE_CXX_COMPILER_VERSION VERSION_LESS 9)
target_link_libraries(traffic_server PRIVATE c++fs)
endif()

@phongn
Copy link
Copy Markdown
Collaborator Author

phongn commented Feb 1, 2024

@ezelkow1 resolved in latest update to this PR, though we have a larger conversation on if we want to allow this.

@phongn phongn requested a review from ezelkow1 February 1, 2024 23:27
@ezelkow1
Copy link
Copy Markdown
Member

ezelkow1 commented Feb 2, 2024

Yes this does fix the issue in my testing now. However we do need that conversation if we want to allow the older compilers or not. If so then Im good with it

@bryancall
Copy link
Copy Markdown
Contributor

@phongn We are not officially supporting GCC compilers less than GCC 10 for the ATS 10 release. Do you need this PR or can we close it?

@phongn phongn closed this Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Build work related to build configuration or environment CMake work related to CMakes scripts or issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants