[PATCH] D31363: [libc++] Remove cmake glob for source files

ldionne added a comment. In D31363#2360132 , @ldionne wrote: > This patch isn't necessary anymore, as we don't use globing anymore. Let's > abandon it to clean up the review queue. Actually, I misspoke. We still use it, but only in a few places. If you s

[PATCH] D31363: [libc++] Remove cmake glob for source files

ldionne added a comment. This patch isn't necessary anymore, as we don't use globing anymore. Let's abandon it to clean up the review queue. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D31363/new/ https://reviews.llvm.org/D31363 ___ cfe-com

[PATCH] D31363: [libc++] Remove cmake glob for source files

dexonsmith resigned from this revision. dexonsmith added a reviewer: ldionne. dexonsmith added a comment. In D31363#889044 , @smeenai wrote: > @rjmccall, this adds a libc++ build dependency on LLVM's cmake modules. There > were some issues when @zturner ha

Re: [PATCH] D31363: [libc++] Remove cmake glob for source files

On Thu, Oct 5, 2017 at 10:20 AM, Chris Bieneman via Phabricator < revi...@reviews.llvm.org> wrote: > beanz added a comment. > > Building libcxx without LLVM's CMake modules is very important, not just > to Apple. *Why* is it important? The reason isn't obvious to me. The only additional cost is

[PATCH] D31363: [libc++] Remove cmake glob for source files

beanz added a comment. Building libcxx without LLVM's CMake modules is very important, not just to Apple. This is how several open source distributions work, and it would be a huge disservice to break this. Same is true for compiler-rt, libcxxabi, libunwind, etc. Historically we've been drawin

Re: [PATCH] D31363: [libc++] Remove cmake glob for source files

It’s not, but the point is we’re already skirting the line On Wed, Oct 4, 2017 at 9:37 PM Duncan P. N. Exon Smith wrote: > I haven't looked at the patch. If this is guarded behind NOT > LIBCXX_STANDALONE_BUILD checks, then it's probably fine. > > > On Oct 4, 2017, at 21:36, Zachary Turner wrote

Re: [PATCH] D31363: [libc++] Remove cmake glob for source files

I haven't looked at the patch. If this is guarded behind NOT LIBCXX_STANDALONE_BUILD checks, then it's probably fine. > On Oct 4, 2017, at 21:36, Zachary Turner wrote: > > This doesn’t match up with what beanz said. While I assume Duncan is the > final word, can we get some confirmation from

Re: [PATCH] D31363: [libc++] Remove cmake glob for source files

This doesn’t match up with what beanz said. While I assume Duncan is the final word, can we get some confirmation from beanz that everyone is on the same page? (Note that libcxx already uses some of LLVM’s cmake, but it’s behind some NOT LIBCXX_STANDALONE_BUILD checks) Assuming the answer remains

Re: [PATCH] D31363: [libc++] Remove cmake glob for source files

Thanks correct. > On Oct 4, 2017, at 18:49, Shoaib Meenai via Phabricator via cfe-commits > wrote: > > smeenai added subscribers: zturner, rjmccall. > smeenai added a comment. > > @rjmccall, this adds a libc++ build dependency on LLVM's cmake modules. There > were some issues when @zturner ha

[PATCH] D31363: [libc++] Remove cmake glob for source files

rjmccall added a reviewer: dexonsmith. rjmccall added a comment. I'll let Duncan answer that question. https://reviews.llvm.org/D31363 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31363: [libc++] Remove cmake glob for source files

smeenai added subscribers: zturner, rjmccall. smeenai added a comment. @rjmccall, this adds a libc++ build dependency on LLVM's cmake modules. There were some issues when @zturner had added a similar dependency for his work on lit. To confirm, Apple still cares about the use case of building lib

[PATCH] D31363: [libc++] Remove cmake glob for source files

smeenai added a comment. This depends on https://reviews.llvm.org/D37859 for the `SOURCE_DIR` parameter to `llvm_check_source_file_list`. https://reviews.llvm.org/D31363 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/

[PATCH] D31363: [libc++] Remove cmake glob for source files

smeenai updated this revision to Diff 115259. smeenai added a comment. Herald added a subscriber: fedor.sergeev. Address comments https://reviews.llvm.org/D31363 Files: benchmarks/CMakeLists.txt lib/CMakeLists.txt Index: lib/CMakeLists.txt =

[PATCH] D31363: [libc++] Remove cmake glob for source files

smeenai planned changes to this revision. smeenai added a comment. I like the idea of verifying that all source files have been included. Will amend this accordingly. Thanks for the cmake pointer @beanz! Comment at: lib/CMakeLists.txt:304 if (LIBCXX_ENABLE_EXPERIMENTAL_LIBRAR

[PATCH] D31363: [libc++] Remove cmake glob for source files

beanz added a comment. LLVM has a CMake module "LLVMProcessSources.cmake" which verifies that all source files are referenced in the CMakeLists file. Since it is part of the LLVM distributed modules, you can re-use it in libcxx. https://reviews.llvm.org/D31363 __

[PATCH] D31363: [libc++] Remove cmake glob for source files

EricWF added a comment. Thanks for fixing this. Sucks that we have to manually enumerate source files but this is the correct solution according to CMake. Would it be easy to still use `glob` to verify that none of the source files have accidentally been forgotten? Comment a

[PATCH] D31363: [libc++] Remove cmake glob for source files

smeenai created this revision. Herald added a subscriber: mgorny. Globbing for source files is problematic because if a source file is added or removed, the configuration won't be run again, and so the build won't pick up on the added or removed file until the next configuration. cmake's help expl