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

fix: Add stdc++fs to link path to compatible with gcc #88

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

winest
Copy link

@winest winest commented Jan 28, 2020

gcc need to specify libstdc++fs to support when specifying CMAKE_CXX_STANDARD=17
Refer to https://gitlab.kitware.com/cmake/cmake/issues/17834

@tammela
Copy link
Contributor

tammela commented Jan 28, 2020

It's not necessary for our use case.
In gcc < 9.0 the library was separate from libstdc++, but even then the portion we use here was not implemented properly and was throwing a segfault. We added a try_run() clause to CMakeLists.txt that runs a small program and checks for the correctness of the APIs we use from fs, in other words, we don't support gcc < 9.0.

@winest
Copy link
Author

winest commented Jan 29, 2020

hmm..that's interesting. I thought it works because I use gcc 8.3(latest on official Ubuntu 1804) with this fix to format 10,000+ files correctly. Anyway, thanks for your check and good code.

@tammela
Copy link
Contributor

tammela commented Jan 29, 2020

@winest you could run the test suite.
If it passes, please let me know!

@winest
Copy link
Author

winest commented Jan 29, 2020

@tammela
It seems fine.

./lua-format-test
line 2:0 mismatched input '#!/bin/lua5.3' expecting
/home/winest/Apps/LuaFormatter/test/testdata/literals/doublequote.config exist
0 dd
/home/winest/Apps/LuaFormatter/test/testdata/literals/singlequote.config exist
0 dd
/home/winest/Apps/LuaFormatter/test/testdata/expression/function_2.config exist
0 dd
/home/winest/Apps/LuaFormatter/test/testdata/issues/issue-36.config exist
1 dd
/home/winest/Apps/LuaFormatter/test/testdata/issues/issue-62_1.config exist
0 dd
/home/winest/Apps/LuaFormatter/test/testdata/issues/issue-62_2.config exist
0 dd
/home/winest/Apps/LuaFormatter/test/testdata/issues/issue-62_3.config exist
0 dd
/home/winest/Apps/LuaFormatter/test/testdata/keep_simple_block_one_line/keep_simple_function_one_line_false.config exist
0 dd
/home/winest/Apps/LuaFormatter/test/testdata/keep_simple_block_one_line/keep_simple_control_block_one_line_false.config exist
0 dd
===============================================================================
All tests passed (121 assertions in 52 test cases)

My environment:

cmake --version
cmake version 3.15.3

gcc --version
gcc (Ubuntu 8.3.0-6ubuntu1~18.04.1) 8.3.0

g++ --version
g++ (Ubuntu 8.3.0-6ubuntu1~18.04.1) 8.3.0

@tammela
Copy link
Contributor

tammela commented Feb 2, 2020

Confirmed that apparently they fixed this issue (segfault on fs lib) in 8.3.

I did a PR that addressed the code issues here and made it work in all platforms.

Thank you!

Koihik added a commit that referenced this pull request Feb 4, 2020
fix: link to stdc++fs when needed as discussed in #88
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.

None yet

2 participants