[PATCH] D26334: Add some facilities to work with a git monorepo (experimental setup)

Justin Bogner via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 7 11:03:39 PST 2016


Mehdi AMINI via llvm-commits <llvm-commits at lists.llvm.org> writes:
> mehdi_amini created this revision.
> mehdi_amini added a reviewer: jlebar.
> mehdi_amini added a subscriber: llvm-commits.
> Herald added subscribers: modocache, mgorny.
>
> Some changes are made to cmake, especially the addition of a new
> LLVM_ENABLE_PROJECTS option that makes the build system aware of
> the monorepo directory structure.
>
> Also a new script is added in llvm/utils/git-svn/. When present in
> the $PATH, it enables a `git llvm` command. It is providing at this
> point only the ability to push from the git monorepo: `git llvm push`.
> It is intended to evolves with more features, for instance I plan on
> features like `git llvm show r284955` to help working with sequential
> revision numbers.

These seem like two unrelated changes to me. I don't see what the
git-llvm tool has to do with the top-level-in-a-monorepo cmake stuff.
Maybe this should be split in two?

I'll only comment on the cmake parts here.

> mehdi_amini added a comment.
>
> Added @beanz and @bogner to make sure I didn't mess up the cmake part.
>
> mehdi_amini updated this revision to Diff 77054.
> mehdi_amini added a comment.
>
> Address Justin's comments.
>
>
> https://reviews.llvm.org/D26334
>
> Files:
>   libcxxabi/CMakeLists.txt
>   llvm/CMakeLists.txt
>   llvm/docs/GettingStarted.rst
>   llvm/utils/git-svn/git-llvm
>
 ...
> Index: llvm/CMakeLists.txt
> ===================================================================
> --- llvm/CMakeLists.txt
> +++ llvm/CMakeLists.txt
> @@ -93,6 +93,25 @@
>    endif()
>  endif()
>  
> +# Git Mono-Repo handling: automatically set the LLVM_EXTERNAL_${project}_SOURCE_DIR

If we go to a real mono-repo, I think this would make way more sense as
a top-level cmake rather than inside LLVM. That would also let us
provide a helpful error message if nothing is set at all.

As such, I'm not 100% convinced this should go in here and now, since
people may start relying on it and it would become harder to change. Is
there a way to put things in the top level of the mono-repo prototype
instead?

> +set(LLVM_ALL_PROJECTS "clang;libcxx;libcxxabi;lldb;compiler-rt;lld;polly")
> +set(LLVM_ENABLE_PROJECTS "" CACHE STRING
> +	"Semicolon-separated list of projects to build (${LLVM_ALL_PROJECTS}), or \"all\".")
> +if( LLVM_ENABLE_PROJECTS STREQUAL "all" )
> +  set( LLVM_ENABLE_PROJECTS ${LLVM_ALL_PROJECTS})
> +endif()
> +foreach(proj ${LLVM_ENABLE_PROJECTS})
> +  set(PROJ_DIR "${CMAKE_CURRENT_SOURCE_DIR}/../${proj}")
> +  if(NOT EXISTS "${PROJ_DIR}" OR NOT IS_DIRECTORY "${PROJ_DIR}")
> +    message(FATAL_ERROR "LLVM_ENABLE_PROJECTS requests ${proj} but directory not found: ${PROJ_DIR}")
> +  endif()
> +  string(TOUPPER "${proj}" upper_proj)
> +  set(LLVM_EXTERNAL_${upper_proj}_SOURCE_DIR   "${CMAKE_CURRENT_SOURCE_DIR}/../${proj}")
> +  if (proj STREQUAL "clang")

Shouldn't this be clang-tools-extra? This looks like it enables
clang-tools-extra whenever clang is enabled, which is surprising.

> +    set(LLVM_EXTERNAL_CLANG_TOOLS_EXTRA_SOURCE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/../clang-tools-extra")
> +  endif()
> +endforeach()
> +
>  # The following only works with the Ninja generator in CMake >= 3.0.
>  set(LLVM_PARALLEL_COMPILE_JOBS "" CACHE STRING
>    "Define the maximum number of concurrent compilation jobs.")
> Index: libcxxabi/CMakeLists.txt
> ===================================================================
> --- libcxxabi/CMakeLists.txt
> +++ libcxxabi/CMakeLists.txt
> @@ -144,6 +144,7 @@
>          ${LIBCXXABI_LIBCXX_PATH}/include
>          ${CMAKE_BINARY_DIR}/${LIBCXXABI_LIBCXX_INCLUDES}
>          ${LLVM_MAIN_SRC_DIR}/projects/libcxx/include
> +        ${LLVM_MAIN_SRC_DIR}/../libcxx/include

Is it possible to only add the "correct" set of headers here? If headers
exist in both places for some reason I think weird things could happen.

>          ${LLVM_INCLUDE_DIR}/c++/v1
>    )
>  
> @@ -156,6 +157,7 @@
>    PATHS ${LIBCXXABI_LIBCXX_PATH}
>          ${LIBCXXABI_LIBCXX_INCLUDES}/../
>          ${LLVM_MAIN_SRC_DIR}/projects/libcxx/
> +        ${LLVM_MAIN_SRC_DIR}/../libcxx/
>    NO_DEFAULT_PATH
>    )
>  


More information about the llvm-commits mailing list