[libc++] Compare to libc++ source directory for out-of-source check

[smeenai]

Summary

When ensuring an out-of-source build, compare to the libc++ source
directory instead of the top-level cmake source directory (which would
be the LLVM source directory for an in-tree build). For an in-tree
build, LLVM already ensures that the build directory is different from
the LLVM source directory, so the existing check is redundant, and the
new check ensures the build directory is different from the libc++
source directory. For an out-of-tree build, the existing and new checks
are identical.

[beanz]

This patch doesn't make sense to me.

In an in-tree build CMAKE_SOURCE_DIR would be the LLVM source directory which shouldn't match your CMAKE_BINARY_DIR.

In an out-of-tree build CMAKE_SOURCE_DIR would match the libcxx sure directory, which shouldn't match CMAKE_BINARY_DIR.

In an out-of-tree build CMAKE_SOURCE_DIR and CMAKE_CURRENT_SOURCE_DIR, and for an in-tree build (where they would be different) there should be no situation where the CMAKE_CURRENT_SOURCE_DIR could possibly be equal to the current source directory unless the user does something *really* strange.

By *really* strange I mean a workflow that involved checking out LLVM, and libcxx, then configuring LLVM from inside the libcxx directory. If we're looking to capture that kind of situation (configuring a build inside an arbitrary directory in the source tree) our existing mechanisms are insufficient in a great many ways, so I don't think we should go down that path.

If you think this patch is important can you please explain the specific cases where this catches errors that the current code doesn't?

[smeenai]

In D26375#588949, @beanz wrote:

This patch doesn't make sense to me.

In an in-tree build CMAKE_SOURCE_DIR would be the LLVM source directory which shouldn't match your CMAKE_BINARY_DIR.

In an out-of-tree build CMAKE_SOURCE_DIR would match the libcxx sure directory, which shouldn't match CMAKE_BINARY_DIR.

In an out-of-tree build CMAKE_SOURCE_DIR and CMAKE_CURRENT_SOURCE_DIR, and for an in-tree build (where they would be different) there should be no situation where the CMAKE_CURRENT_SOURCE_DIR could possibly be equal to the current source directory unless the user does something *really* strange.

By *really* strange I mean a workflow that involved checking out LLVM, and libcxx, then configuring LLVM from inside the libcxx directory. If we're looking to capture that kind of situation (configuring a build inside an arbitrary directory in the source tree) our existing mechanisms are insufficient in a great many ways, so I don't think we should go down that path.

If you think this patch is important can you please explain the specific cases where this catches errors that the current code doesn't?

My reasoning was that conceptually, it makes sense for libc++ to be concerned about its own source tree as far as out-of-source builds go. When libc++ is build in-tree, LLVM's build system already takes care of ensuring that the build directory is not the LLVM source directory, so it's redundant for libc++ to do the same.

I don't feel strongly about this though, and from your comment it sounds like I may have just misunderstood the point of this check (i.e. the redundancy was in fact the intended effect), in which case I'm happy abandoning.

cmake/Modules/MacroEnsureOutOfSourceBuild.cmake

 # MACRO_ENSURE_OUT_OF_SOURCE_BUILD(<errorMessage>)
 
 macro( MACRO_ENSURE_OUT_OF_SOURCE_BUILD _errorMessage )
 
-string( COMPARE EQUAL "${CMAKE_SOURCE_DIR}" "${CMAKE_BINARY_DIR}" _insource )
+string( COMPARE EQUAL "${CMAKE_CURRENT_SOURCE_DIR}" "${CMAKE_BINARY_DIR}" _insource )
 if( _insource )
  message( SEND_ERROR "${_errorMessage}" )
  message( FATAL_ERROR
  "In-source builds are not allowed.
  CMake would overwrite the makefiles distributed with Compiler-RT.
  Please create a directory and run cmake from there, passing the path
  to this source directory as the last argument.
  This process created the file `CMakeCache.txt' and the directory `CMakeFiles'.
  Please delete them."
  )
 endif( _insource )
 
 endmacro( MACRO_ENSURE_OUT_OF_SOURCE_BUILD )