Re: [PATCH v2 1/2] Fix CMakeLists.txt on Linux.

From: Junio C Hamano <gitster@pobox.com>
Date: May 24, 2022, 23:04 UTC
Message-ID: <xmqqtu9ecz1w.fsf@gitster.g>
In-Reply-To: <29cb31e5c502149192c6beb56bf8b372a40711e0.1653374328.git.gitgitgadget@gmail.com>

"Yuyi Wang via GitGitGadget" <gitgitgadget@gmail.com> writes:

> Subject: Re: [PATCH v2 1/2] Fix CMakeLists.txt on Linux.

Perhaps

	Subject: cmake: fix CMakeLists on Linux

i.e. with a short and meaningful <area>: prefix, no need to capitalize the first word after the prefix, without the terminating full-stop.

From: Yuyi Wang <Strawberry_Str@hotmail.com>

>
> CMakeLists.txt didn't follow the grammar of `set`, and it will fail when
> setting `USE_VCPKG` off on non-Windows platforms.

So any non-Windows, the use of set() here was broken. It is understandable because cmake support was added primarily for Windows. It is nevertheless good to see it corrected.

> When the platform is Linux, the Makefile adds `compat/linux/procinfo.o`
> to `COMPAT_OBJS`, but the CMakeLists.txt didn't add
> `compat/linux/procinfo.c` to `compat_SOURCES`. It would cause linkage
> error.

OK. I didn't know anybody cared about using cmake on non-Windows platforms for this project.

Thanks, will queue, together with the other two patches.

Show 27 quoted lines

> Signed-off-by: Yuyi Wang <Strawberry_Str@hotmail.com>
> ---
>  contrib/buildsystems/CMakeLists.txt | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
> index 185f56f414f..7f333e303c2 100644
> --- a/contrib/buildsystems/CMakeLists.txt
> +++ b/contrib/buildsystems/CMakeLists.txt
> @@ -54,7 +54,7 @@ set(CMAKE_SOURCE_DIR ${CMAKE_CURRENT_LIST_DIR}/../..)
>  
>  option(USE_VCPKG "Whether or not to use vcpkg for obtaining dependencies.  Only applicable to Windows platforms" ON)
>  if(NOT WIN32)
> -	set(USE_VCPKG OFF CACHE BOOL FORCE)
> +	set(USE_VCPKG OFF CACHE BOOL "" FORCE)
>  endif()
>  
>  if(NOT DEFINED CMAKE_EXPORT_COMPILE_COMMANDS)
> @@ -277,7 +277,7 @@ if(CMAKE_SYSTEM_NAME STREQUAL "Windows")
>  
>  elseif(CMAKE_SYSTEM_NAME STREQUAL "Linux")
>  	add_compile_definitions(PROCFS_EXECUTABLE_PATH="/proc/self/exe" HAVE_DEV_TTY )
> -	list(APPEND compat_SOURCES unix-socket.c unix-stream-server.c)
> +	list(APPEND compat_SOURCES unix-socket.c unix-stream-server.c compat/linux/procinfo.c)
>  endif()
>  
>  if(CMAKE_SYSTEM_NAME STREQUAL "Windows")