Re: [PATCH v2 0/3] Make CMake work out of the box

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jun 10, 2021, 09:45 UTC
Message-ID: <nycvar.QRO.7.76.6.2106101144050.57@tvgsbejvaqbjf.bet>
In-Reply-To: <xmqqpmwywl2z.fsf@gitster.g>

Hi Junio,

On Mon, 7 Jun 2021, Junio C Hamano wrote:

Show 20 quoted lines

> "Matthew Rogers via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > This pull request comes from our discussion here[1], and I think these
> > patches provide a good compromise around the concerns discussed there
> >
> > 1:
> > https://lore.kernel.org/git/CAOjrSZusMSvs7AS-ZDsV8aQUgsF2ZA754vSDjgFKMRgi_oZAWw@mail.gmail.com/
> >
> > CCing the people involved in the original discussion. cc: Philip Oakley
> > philipoakley@iee.email cc: Sibi Siddharthan
> > sibisiddharthan.github@gmail.com, cc: Johannes Schindelin
> > johannes.schindelin@gmx.de, cc: Danh Doan congdanhqx@gmail.com
> >
> > Matthew Rogers (3):
> >   cmake: add knob to disable vcpkg
> >   cmake: create compile_commands.json by default
> >   cmake: add warning for ignored MSGFMT_EXE
>
> I am neither cmake nor windows person, so I'll queue this as-is and
> wait for the stakeholders to chime in.

As long as the CI builds pass, I am in favor of integrating the patch series.

> I did wonder if we want this to be applicable to the maintenance
> track for 2.31, though.  There is a textual conflict with the
> addition of SIMPLE_IPC that happened during 2.32 cycle, which is
> easily resolvable.

If it isn't much work, sure. But I would think that developers who want to build using Visual Studio really should stay on newer branches.

Thanks, Dscho

Show 24 quoted lines

> I am tempted to queue a version of these three patches rebased on to
> 'maint' after making sure that the result of merging that into
> 'master' is byte-for-byte identical to applying these three patches
> directly on to 'master'.
>
> The range-diff looks like the attached.  Thanks.
>
> 1:  546c49cc88 ! 1:  585b7ca371 cmake: add knob to disable vcpkg
>     @@ contrib/buildsystems/CMakeLists.txt: endif()
>       	if(NOT EXISTS ${MSGFMT_EXE})
>       		message(WARNING "Text Translations won't be built")
>       		unset(MSGFMT_EXE)
>     -@@ contrib/buildsystems/CMakeLists.txt: file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "X='${EXE_EXTENSION}'\n")
>     - file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "NO_GETTEXT='${NO_GETTEXT}'\n")
>     +@@ contrib/buildsystems/CMakeLists.txt: file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "NO_GETTEXT='${NO_GETTEXT}'\n"
>       file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "RUNTIME_PREFIX='${RUNTIME_PREFIX}'\n")
>       file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "NO_PYTHON='${NO_PYTHON}'\n")
>     + file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "SUPPORTS_SIMPLE_IPC='${SUPPORTS_SIMPLE_IPC}'\n")
>      -if(WIN32)
>      +if(USE_VCPKG)
>       	file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "PATH=\"$PATH:$TEST_DIRECTORY/../compat/vcbuild/vcpkg/installed/x64-windows/bin\"\n")
> 2:  efa8681a22 = 2:  1cba2f9bd1 cmake: create compile_commands.json by default
> 3:  ceeca2bc0d = 3:  7824e74976 cmake: add warning for ignored MSGFMT_EXE
>