"Matthew Rogers via GitGitGadget" [off-list ref] 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.
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.
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
Hi Junio,
On Mon, 7 Jun 2021, Junio C Hamano wrote:
"Matthew Rogers via GitGitGadget" [off-list ref] writes:
quoted
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
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
On 07/06/2021 01:54, Junio C Hamano wrote:
"Matthew Rogers via GitGitGadget" [off-list ref] writes:
quoted
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.
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.
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'.
Sorry for the delay - I've been off-line and I'm only now catching up.
Could we confirm that the CI actually tests the update. IIRC the yml
setup preloaded the vcpkg artefacts that this change looks to make work
'out of the box'.
Philip
On 10/06/2021 10:45, Johannes Schindelin wrote:
As long as the CI builds pass, I am in favor of integrating the patch
series.
quoted
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.
Ack. Philip