[libc-commits] [PATCH] D137867: [libc] move fork into threads folder

[Tue Ly via Phabricator via libc-commits]

lntue added a comment.

In D137867#3926310 <https://reviews.llvm.org/D137867#3926310>, @sivachandra wrote:

> In D137867#3926258 <https://reviews.llvm.org/D137867#3926258>, @michaelrj wrote:
>
>> yes, the windows build was failing (and anything else that doesn't have a threads implementation) because the `fork_callbacks` cmake target was including `mutex` unconditionally.
>
> The fork callback machinery uses a mutex and is not related to threads. So, the pattern to follow would be this: https://github.com/llvm/llvm-project/blob/main/libc/src/__support/File/CMakeLists.txt#L1. Which is to exclude listing a target if mutex is not available. This patch is essentially doing that, but clubbing fork callbacks machinery with thread machinery is incorrect.

Putting `fork_callback` inside the target check (without the `return` part) and adding message `Skipping ...` with reason similar to unit tests sounds good to me.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137867/new/

https://reviews.llvm.org/D137867