Skip to content

Do not include libcxx library for C#43166

Merged
alexey-milovidov merged 1 commit intoClickHouse:masterfrom
azat:build/libcxx-only-for-cxx
Nov 22, 2022
Merged

Do not include libcxx library for C#43166
alexey-milovidov merged 1 commit intoClickHouse:masterfrom
azat:build/libcxx-only-for-cxx

Conversation

@azat
Copy link
Copy Markdown
Member

@azat azat commented Nov 11, 2022

If you will have the same header in both libraries you may have some tricky failures.

And there is at least one header that exists in both: stdatomic.h

Right now there is a workaround for this header, that allows to use C++ version of this header for C - the workaround is called "set _LIBCPP_COMPILER_CLANG_BASED not only for CXX" 1 and use include_next 2.

However #42730 (cc @rschu1ze) changes this, and now it fails to compile because of this.

Changelog category (leave one):

  • Not for changelog (changelog entry is not required)

@robot-clickhouse robot-clickhouse added the pr-not-for-changelog This PR should not be mentioned in the changelog label Nov 11, 2022
@azat azat force-pushed the build/libcxx-only-for-cxx branch from d05cf74 to 3735630 Compare November 11, 2022 12:40
@azat azat marked this pull request as draft November 11, 2022 13:24
If you will have the same header in both libraries you may have some
tricky failures.

And there is at least one header that exists in both: stdatomic.h

Right now there is a workaround for this header, that allows to use C++
version of this header for C - the workaround is called "set
_LIBCPP_COMPILER_CLANG_BASED not only for CXX" [1] and use include_next
[2].

  [1]: https://github.com/ClickHouse/libcxx/blob/9a457ab3c64a533a06922b386b284215c17ce627/include/__config#L39
  [2]: https://github.com/ClickHouse/libcxx/blob/9a457ab3c64a533a06922b386b284215c17ce627/include/stdatomic.h#L223-L231

However ClickHouse#42730 changes this, and now it fails to compile because of this.

Signed-off-by: Azat Khuzhin <[email protected]>
@azat azat force-pushed the build/libcxx-only-for-cxx branch from 3735630 to ddfea23 Compare November 11, 2022 13:43
@azat azat marked this pull request as ready for review November 11, 2022 13:43
@robot-ch-test-poll1 robot-ch-test-poll1 added the submodule changed At least one submodule changed in this PR. label Nov 11, 2022
@azat
Copy link
Copy Markdown
Member Author

azat commented Nov 18, 2022

Failures unrelated:

Stress test (ubsan) — Backward compatibility check: Error message in clickhouse-server.log (see bc_check_error_messages.txt)

2022.11.11 20:56:38.838217 [ 610150 ] {} <Error> test_1.concurrent_mutate_mt_2: while loading part all_46_46_0_106 on path data/test_1/concurrent_mutate_mt_2/all_46_46_0_106: Code: 499. DB::Exception: The specified key does not exist.: while reading key: test/eio/zxetbqyuuplfhgqbxscfjmzqigdrn, from bucket: test. (S3_ERROR), Stack trace (when copying this message, always include the lines below):

@alexey-milovidov alexey-milovidov self-assigned this Nov 22, 2022
@alexey-milovidov alexey-milovidov merged commit 26d7b23 into ClickHouse:master Nov 22, 2022
@azat azat deleted the build/libcxx-only-for-cxx branch November 22, 2022 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-not-for-changelog This PR should not be mentioned in the changelog submodule changed At least one submodule changed in this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants