You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2022/07/05 10:51:10 UTC
[GitHub] [nifi-minifi-cpp] lordgamez opened a new pull request, #1364: MINIFICPP-1878 Enable Ninja build in Windows builds
lordgamez opened a new pull request, #1364:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1364
https://issues.apache.org/jira/browse/MINIFICPP-1878
--------------------------------
Thank you for submitting a contribution to Apache NiFi - MiNiFi C++.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
- [ ] Does your PR title start with MINIFICPP-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
- [ ] Has your PR been rebased against the latest commit within the target branch (typically main)?
- [ ] Is your initial contribution a single, squashed commit?
### For code changes:
- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] If applicable, have you updated the LICENSE file?
- [ ] If applicable, have you updated the NOTICE file?
### For documentation related changes:
- [ ] Have you ensured that format looks appropriate for the output in which it is rendered?
### Note:
Please ensure that once the PR is submitted, you check GitHub Actions CI results for build issues and submit an update to your PR as soon as possible.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] lordgamez commented on a diff in pull request #1364: MINIFICPP-1878 Enable Ninja build in Windows builds
Posted by GitBox <gi...@apache.org>.
lordgamez commented on code in PR #1364:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1364#discussion_r922011417
##########
cmake/CivetWeb.cmake:
##########
@@ -0,0 +1,42 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+include(FetchContent)
+
+if (NOT WIN32)
+ set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-error" CACHE STRING "" FORCE)
+ set(CMAKE_CFLAGS "${CMAKE_CFLAGS} -Wno-error" CACHE STRING "" FORCE)
+endif()
Review Comment:
I'm not sure if they could affect other modules, but it could be possible that the targets defined after civetweb would be affected. To make sure it doesn't happen I updated to only set target specific flags in civetweb in 4e37ffca0fc66548cf89b4738a8f73a7f9a5645e
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] fgerlits commented on a diff in pull request #1364: MINIFICPP-1878 Enable Ninja build in Windows builds
Posted by GitBox <gi...@apache.org>.
fgerlits commented on code in PR #1364:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1364#discussion_r920208894
##########
win_build_vs.bat:
##########
@@ -73,12 +73,18 @@ for %%x in (%*) do (
if [%%~x] EQU [/NONFREEUCRT] set "redist=-DMSI_REDISTRIBUTE_UCRT_NONASL=ON"
if [%%~x] EQU [/L] set build_linter=ON
if [%%~x] EQU [/RO] set real_odbc=ON
+ if [%%~x] EQU [/NJ] set generator="Ninja"
Review Comment:
I would prefer `/NINJA`
##########
win_build_vs.bat:
##########
@@ -73,12 +73,18 @@ for %%x in (%*) do (
if [%%~x] EQU [/NONFREEUCRT] set "redist=-DMSI_REDISTRIBUTE_UCRT_NONASL=ON"
if [%%~x] EQU [/L] set build_linter=ON
if [%%~x] EQU [/RO] set real_odbc=ON
+ if [%%~x] EQU [/NJ] set generator="Ninja"
)
mkdir %builddir%
pushd %builddir%\
-cmake -G %generator% -A %build_platform% -DINSTALLER_MERGE_MODULES=%installer_merge_modules% -DTEST_CUSTOM_WEL_PROVIDER=%test_custom_wel_provider% -DENABLE_SQL=%build_SQL% -DUSE_REAL_ODBC_TEST_DRIVER=%real_odbc% -DCMAKE_BUILD_TYPE_INIT=%cmake_build_type% -DCMAKE_BUILD_TYPE=%cmake_build_type% -DWIN32=WIN32 -DENABLE_LIBRDKAFKA=%build_kafka% -DENABLE_JNI=%build_jni% -DOPENSSL_OFF=OFF -DENABLE_COAP=%build_coap% -DENABLE_AWS=%build_AWS% -DENABLE_PDH=%build_PDH% -DENABLE_AZURE=%build_azure% -DENABLE_SFTP=%build_SFTP% -DENABLE_SPLUNK=%build_SPLUNK% -DENABLE_GCP=%build_GCP% -DENABLE_NANOFI=%build_nanofi% -DENABLE_OPENCV=%build_opencv% -DENABLE_PROMETHEUS=%build_prometheus% -DENABLE_ELASTICSEARCH=%build_ELASTIC% -DUSE_SHARED_LIBS=OFF -DDISABLE_CONTROLLER=ON -DBUILD_ROCKSDB=ON -DFORCE_WINDOWS=ON -DUSE_SYSTEM_UUID=OFF -DDISABLE_LIBARCHIVE=OFF -DENABLE_SCRIPTING=OFF -DEXCLUDE_BOOST=ON -DENABLE_WEL=ON -DFAIL_ON_WARNINGS=OFF -DSKIP_TESTS=%skiptests% %strict_gsl_checks% %redist% -DENABLE_LINTER=%
build_linter% "%scriptdir%" && msbuild /m nifi-minifi-cpp.sln /property:Configuration=%cmake_build_type% /property:Platform=%build_platform% && copy bin\%cmake_build_type%\minifi.exe main\
+if [%generator%] EQU ["Ninja"] (
+ set "buildcmd=ninja"
+) else (
+ set "buildcmd=msbuild /m nifi-minifi-cpp.sln /property:Configuration=%cmake_build_type% /property:Platform=%build_platform% && copy bin\%cmake_build_type%\minifi.exe main\"
Review Comment:
Is the `&& copy bin\%cmake_build_type%\minifi.exe main\` not needed when building with ninja?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] szaszm commented on a diff in pull request #1364: MINIFICPP-1878 Enable Ninja build in Windows builds
Posted by GitBox <gi...@apache.org>.
szaszm commented on code in PR #1364:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1364#discussion_r921981470
##########
cmake/CivetWeb.cmake:
##########
@@ -0,0 +1,42 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+include(FetchContent)
+
+if (NOT WIN32)
+ set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-error" CACHE STRING "" FORCE)
+ set(CMAKE_CFLAGS "${CMAKE_CFLAGS} -Wno-error" CACHE STRING "" FORCE)
+endif()
Review Comment:
Is this leaking outside of the flags of this module? I wouldn't want to accidentally disable Werror globally if we have `FAIL_ON_WARNINGS=ON`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] szaszm commented on a diff in pull request #1364: MINIFICPP-1878 Enable Ninja build in Windows builds
Posted by GitBox <gi...@apache.org>.
szaszm commented on code in PR #1364:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1364#discussion_r922107019
##########
cmake/CivetWeb.cmake:
##########
@@ -0,0 +1,46 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+include(FetchContent)
+
+set(CIVETWEB_ENABLE_SSL_DYNAMIC_LOADING "OFF" CACHE STRING "" FORCE)
+set(CIVETWEB_BUILD_TESTING "OFF" CACHE STRING "" FORCE)
+set(CIVETWEB_ENABLE_DUKTAPE "OFF" CACHE STRING "" FORCE)
+set(CIVETWEB_ENABLE_LUA "OFF" CACHE STRING "" FORCE)
+set(CIVETWEB_ENABLE_CXX "ON" CACHE STRING "" FORCE)
+set(CIVETWEB_ALLOW_WARNINGS "ON" CACHE STRING "" FORCE)
+set(CIVETWEB_ENABLE_ASAN "OFF" CACHE STRING "" FORCE)
+
+FetchContent_Declare(civetweb
+ GIT_REPOSITORY "https://github.com/civetweb/civetweb.git"
+ GIT_TAG "4447b6501d5c568b4c6c0940eac801ec690b2250" # commit containing fix for MSVC issue https://github.com/civetweb/civetweb/issues/1024
+)
+
+FetchContent_MakeAvailable(civetweb)
+
+add_dependencies(civetweb-c-library OpenSSL::Crypto OpenSSL::SSL)
+add_dependencies(civetweb-cpp OpenSSL::Crypto OpenSSL::SSL)
+
+set(CIVETWEB_C_FLAGS "${CMAKE_CFLAGS} -Wno-error" CACHE STRING "" FORCE)
+set(CIVETWEB_CXX_LAGS "${CMAKE_CXX_FLAGS} -Wno-error" CACHE STRING "" FORCE)
+string(REPLACE " " ";" REPLACED_C_FLAGS ${CIVETWEB_C_FLAGS})
+string(REPLACE " " ";" REPLACED_CXX_FLAGS ${CIVETWEB_CXX_LAGS})
+target_compile_options(civetweb-c-library INTERFACE ${REPLACED_C_FLAGS})
+target_compile_options(civetweb-cpp INTERFACE ${REPLACED_CXX_FLAGS})
Review Comment:
This takes all of the global flags, appends `-Wno-error` and appends them to anything linking to civet. Two problems:
1. It's a bit excessive to append all the flags from C/CXXFLAGS again to civet. Anything before `-Wno-error` will be duplicated on the command line.
2. The flags should apply to civet itself, but not the dependencies. Use `PRIVATE` instead of `INTERFACE`.
I'm not 100% sure, but the solution should be something like this:
```suggestion
target_compile_options(civetweb-c-library PRIVATE -Wno-error)
target_compile_options(civetweb-cpp PRIVATE -Wno-error)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] lordgamez commented on a diff in pull request #1364: MINIFICPP-1878 Enable Ninja build in Windows builds
Posted by GitBox <gi...@apache.org>.
lordgamez commented on code in PR #1364:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1364#discussion_r921954555
##########
win_build_vs.bat:
##########
@@ -73,12 +73,18 @@ for %%x in (%*) do (
if [%%~x] EQU [/NONFREEUCRT] set "redist=-DMSI_REDISTRIBUTE_UCRT_NONASL=ON"
if [%%~x] EQU [/L] set build_linter=ON
if [%%~x] EQU [/RO] set real_odbc=ON
+ if [%%~x] EQU [/NJ] set generator="Ninja"
)
mkdir %builddir%
pushd %builddir%\
-cmake -G %generator% -A %build_platform% -DINSTALLER_MERGE_MODULES=%installer_merge_modules% -DTEST_CUSTOM_WEL_PROVIDER=%test_custom_wel_provider% -DENABLE_SQL=%build_SQL% -DUSE_REAL_ODBC_TEST_DRIVER=%real_odbc% -DCMAKE_BUILD_TYPE_INIT=%cmake_build_type% -DCMAKE_BUILD_TYPE=%cmake_build_type% -DWIN32=WIN32 -DENABLE_LIBRDKAFKA=%build_kafka% -DENABLE_JNI=%build_jni% -DOPENSSL_OFF=OFF -DENABLE_COAP=%build_coap% -DENABLE_AWS=%build_AWS% -DENABLE_PDH=%build_PDH% -DENABLE_AZURE=%build_azure% -DENABLE_SFTP=%build_SFTP% -DENABLE_SPLUNK=%build_SPLUNK% -DENABLE_GCP=%build_GCP% -DENABLE_NANOFI=%build_nanofi% -DENABLE_OPENCV=%build_opencv% -DENABLE_PROMETHEUS=%build_prometheus% -DENABLE_ELASTICSEARCH=%build_ELASTIC% -DUSE_SHARED_LIBS=OFF -DDISABLE_CONTROLLER=ON -DBUILD_ROCKSDB=ON -DFORCE_WINDOWS=ON -DUSE_SYSTEM_UUID=OFF -DDISABLE_LIBARCHIVE=OFF -DENABLE_SCRIPTING=OFF -DEXCLUDE_BOOST=ON -DENABLE_WEL=ON -DFAIL_ON_WARNINGS=OFF -DSKIP_TESTS=%skiptests% %strict_gsl_checks% %redist% -DENABLE_LINTER=%
build_linter% "%scriptdir%" && msbuild /m nifi-minifi-cpp.sln /property:Configuration=%cmake_build_type% /property:Platform=%build_platform% && copy bin\%cmake_build_type%\minifi.exe main\
+if [%generator%] EQU ["Ninja"] (
+ set "buildcmd=ninja"
+) else (
+ set "buildcmd=msbuild /m nifi-minifi-cpp.sln /property:Configuration=%cmake_build_type% /property:Platform=%build_platform% && copy bin\%cmake_build_type%\minifi.exe main\"
Review Comment:
Good catch, it should be copied, updated in 5aa6d9a5942b642f49b0903ee80d1f74fc0ec82c
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] lordgamez commented on a diff in pull request #1364: MINIFICPP-1878 Enable Ninja build in Windows builds
Posted by GitBox <gi...@apache.org>.
lordgamez commented on code in PR #1364:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1364#discussion_r921955068
##########
win_build_vs.bat:
##########
@@ -73,12 +73,18 @@ for %%x in (%*) do (
if [%%~x] EQU [/NONFREEUCRT] set "redist=-DMSI_REDISTRIBUTE_UCRT_NONASL=ON"
if [%%~x] EQU [/L] set build_linter=ON
if [%%~x] EQU [/RO] set real_odbc=ON
+ if [%%~x] EQU [/NJ] set generator="Ninja"
Review Comment:
I think it's better to be more explicit, updated in 5aa6d9a5942b642f49b0903ee80d1f74fc0ec82c
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] lordgamez commented on a diff in pull request #1364: MINIFICPP-1878 Enable Ninja build in Windows builds
Posted by GitBox <gi...@apache.org>.
lordgamez commented on code in PR #1364:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1364#discussion_r922111189
##########
cmake/CivetWeb.cmake:
##########
@@ -0,0 +1,46 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+include(FetchContent)
+
+set(CIVETWEB_ENABLE_SSL_DYNAMIC_LOADING "OFF" CACHE STRING "" FORCE)
+set(CIVETWEB_BUILD_TESTING "OFF" CACHE STRING "" FORCE)
+set(CIVETWEB_ENABLE_DUKTAPE "OFF" CACHE STRING "" FORCE)
+set(CIVETWEB_ENABLE_LUA "OFF" CACHE STRING "" FORCE)
+set(CIVETWEB_ENABLE_CXX "ON" CACHE STRING "" FORCE)
+set(CIVETWEB_ALLOW_WARNINGS "ON" CACHE STRING "" FORCE)
+set(CIVETWEB_ENABLE_ASAN "OFF" CACHE STRING "" FORCE)
+
+FetchContent_Declare(civetweb
+ GIT_REPOSITORY "https://github.com/civetweb/civetweb.git"
+ GIT_TAG "4447b6501d5c568b4c6c0940eac801ec690b2250" # commit containing fix for MSVC issue https://github.com/civetweb/civetweb/issues/1024
+)
+
+FetchContent_MakeAvailable(civetweb)
+
+add_dependencies(civetweb-c-library OpenSSL::Crypto OpenSSL::SSL)
+add_dependencies(civetweb-cpp OpenSSL::Crypto OpenSSL::SSL)
+
+set(CIVETWEB_C_FLAGS "${CMAKE_CFLAGS} -Wno-error" CACHE STRING "" FORCE)
+set(CIVETWEB_CXX_LAGS "${CMAKE_CXX_FLAGS} -Wno-error" CACHE STRING "" FORCE)
+string(REPLACE " " ";" REPLACED_C_FLAGS ${CIVETWEB_C_FLAGS})
+string(REPLACE " " ";" REPLACED_CXX_FLAGS ${CIVETWEB_CXX_LAGS})
+target_compile_options(civetweb-c-library INTERFACE ${REPLACED_C_FLAGS})
+target_compile_options(civetweb-cpp INTERFACE ${REPLACED_CXX_FLAGS})
Review Comment:
Good point, I think you are right, fixed in e5b0aad87214253154820ee11f858fafa622e12d
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] lordgamez commented on a diff in pull request #1364: MINIFICPP-1878 Enable Ninja build in Windows builds
Posted by GitBox <gi...@apache.org>.
lordgamez commented on code in PR #1364:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1364#discussion_r922111189
##########
cmake/CivetWeb.cmake:
##########
@@ -0,0 +1,46 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+include(FetchContent)
+
+set(CIVETWEB_ENABLE_SSL_DYNAMIC_LOADING "OFF" CACHE STRING "" FORCE)
+set(CIVETWEB_BUILD_TESTING "OFF" CACHE STRING "" FORCE)
+set(CIVETWEB_ENABLE_DUKTAPE "OFF" CACHE STRING "" FORCE)
+set(CIVETWEB_ENABLE_LUA "OFF" CACHE STRING "" FORCE)
+set(CIVETWEB_ENABLE_CXX "ON" CACHE STRING "" FORCE)
+set(CIVETWEB_ALLOW_WARNINGS "ON" CACHE STRING "" FORCE)
+set(CIVETWEB_ENABLE_ASAN "OFF" CACHE STRING "" FORCE)
+
+FetchContent_Declare(civetweb
+ GIT_REPOSITORY "https://github.com/civetweb/civetweb.git"
+ GIT_TAG "4447b6501d5c568b4c6c0940eac801ec690b2250" # commit containing fix for MSVC issue https://github.com/civetweb/civetweb/issues/1024
+)
+
+FetchContent_MakeAvailable(civetweb)
+
+add_dependencies(civetweb-c-library OpenSSL::Crypto OpenSSL::SSL)
+add_dependencies(civetweb-cpp OpenSSL::Crypto OpenSSL::SSL)
+
+set(CIVETWEB_C_FLAGS "${CMAKE_CFLAGS} -Wno-error" CACHE STRING "" FORCE)
+set(CIVETWEB_CXX_LAGS "${CMAKE_CXX_FLAGS} -Wno-error" CACHE STRING "" FORCE)
+string(REPLACE " " ";" REPLACED_C_FLAGS ${CIVETWEB_C_FLAGS})
+string(REPLACE " " ";" REPLACED_CXX_FLAGS ${CIVETWEB_CXX_LAGS})
+target_compile_options(civetweb-c-library INTERFACE ${REPLACED_C_FLAGS})
+target_compile_options(civetweb-cpp INTERFACE ${REPLACED_CXX_FLAGS})
Review Comment:
Good point, fixed in e5b0aad87214253154820ee11f858fafa622e12d
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] lordgamez commented on a diff in pull request #1364: MINIFICPP-1878 Enable Ninja build in Windows builds
Posted by GitBox <gi...@apache.org>.
lordgamez commented on code in PR #1364:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1364#discussion_r916872367
##########
cmake/BundledCivetWeb.cmake:
##########
@@ -64,12 +61,11 @@ function(use_bundled_civetweb SOURCE_DIR BINARY_DIR)
# Build project
ExternalProject_Add(
civetweb-external
- URL "https://github.com/civetweb/civetweb/archive/v1.12.tar.gz"
- URL_HASH "SHA256=8cab1e2ad8fb3e2e81fed0b2321a5afbd7269a644c44ed4c3607e0a212c6d9e1"
+ GIT_REPOSITORY "https://github.com/civetweb/civetweb.git"
+ GIT_TAG "4447b6501d5c568b4c6c0940eac801ec690b2250" # commit containing fix for MSVC issue https://github.com/civetweb/civetweb/issues/1024
Review Comment:
Replaced with FetchContent in d578834c8d66fac12537468c21b8887283a38045
##########
win_build_vs.bat:
##########
@@ -73,12 +73,17 @@ for %%x in (%*) do (
if [%%~x] EQU [/NONFREEUCRT] set "redist=-DMSI_REDISTRIBUTE_UCRT_NONASL=ON"
if [%%~x] EQU [/L] set build_linter=ON
if [%%~x] EQU [/RO] set real_odbc=ON
+ if [%%~x] EQU [/NJ] set generator="Ninja"
)
mkdir %builddir%
pushd %builddir%\
-cmake -G %generator% -A %build_platform% -DINSTALLER_MERGE_MODULES=%installer_merge_modules% -DTEST_CUSTOM_WEL_PROVIDER=%test_custom_wel_provider% -DENABLE_SQL=%build_SQL% -DUSE_REAL_ODBC_TEST_DRIVER=%real_odbc% -DCMAKE_BUILD_TYPE_INIT=%cmake_build_type% -DCMAKE_BUILD_TYPE=%cmake_build_type% -DWIN32=WIN32 -DENABLE_LIBRDKAFKA=%build_kafka% -DENABLE_JNI=%build_jni% -DOPENSSL_OFF=OFF -DENABLE_COAP=%build_coap% -DENABLE_AWS=%build_AWS% -DENABLE_PDH=%build_PDH% -DENABLE_AZURE=%build_azure% -DENABLE_SFTP=%build_SFTP% -DENABLE_SPLUNK=%build_SPLUNK% -DENABLE_GCP=%build_GCP% -DENABLE_NANOFI=%build_nanofi% -DENABLE_OPENCV=%build_opencv% -DENABLE_PROMETHEUS=%build_prometheus% -DENABLE_ELASTICSEARCH=%build_ELASTIC% -DUSE_SHARED_LIBS=OFF -DDISABLE_CONTROLLER=ON -DBUILD_ROCKSDB=ON -DFORCE_WINDOWS=ON -DUSE_SYSTEM_UUID=OFF -DDISABLE_LIBARCHIVE=OFF -DENABLE_SCRIPTING=OFF -DEXCLUDE_BOOST=ON -DENABLE_WEL=ON -DFAIL_ON_WARNINGS=OFF -DSKIP_TESTS=%skiptests% %strict_gsl_checks% %redist% -DENABLE_LINTER=%
build_linter% "%scriptdir%" && msbuild /m nifi-minifi-cpp.sln /property:Configuration=%cmake_build_type% /property:Platform=%build_platform% && copy bin\%cmake_build_type%\minifi.exe main\
+if [%generator%] EQU ["Ninja"] (
+ cmake -G %generator% -DINSTALLER_MERGE_MODULES=%installer_merge_modules% -DTEST_CUSTOM_WEL_PROVIDER=%test_custom_wel_provider% -DENABLE_SQL=%build_SQL% -DUSE_REAL_ODBC_TEST_DRIVER=%real_odbc% -DCMAKE_BUILD_TYPE_INIT=%cmake_build_type% -DCMAKE_BUILD_TYPE=%cmake_build_type% -DWIN32=WIN32 -DENABLE_LIBRDKAFKA=%build_kafka% -DENABLE_JNI=%build_jni% -DOPENSSL_OFF=OFF -DENABLE_COAP=%build_coap% -DENABLE_AWS=%build_AWS% -DENABLE_PDH=%build_PDH% -DENABLE_AZURE=%build_azure% -DENABLE_SFTP=%build_SFTP% -DENABLE_SPLUNK=%build_SPLUNK% -DENABLE_GCP=%build_GCP% -DENABLE_NANOFI=%build_nanofi% -DENABLE_OPENCV=%build_opencv% -DENABLE_PROMETHEUS=%build_prometheus% -DENABLE_ELASTICSEARCH=%build_ELASTIC% -DUSE_SHARED_LIBS=OFF -DDISABLE_CONTROLLER=ON -DBUILD_ROCKSDB=ON -DFORCE_WINDOWS=ON -DUSE_SYSTEM_UUID=OFF -DDISABLE_LIBARCHIVE=OFF -DENABLE_SCRIPTING=OFF -DEXCLUDE_BOOST=ON -DENABLE_WEL=ON -DFAIL_ON_WARNINGS=OFF -DSKIP_TESTS=%skiptests% %strict_gsl_checks% %redist% -DENABLE_LINTER=%build_linter% "%
scriptdir%" && ninja
+) else (
+ cmake -G %generator% -A %build_platform% -DINSTALLER_MERGE_MODULES=%installer_merge_modules% -DTEST_CUSTOM_WEL_PROVIDER=%test_custom_wel_provider% -DENABLE_SQL=%build_SQL% -DUSE_REAL_ODBC_TEST_DRIVER=%real_odbc% -DCMAKE_BUILD_TYPE_INIT=%cmake_build_type% -DCMAKE_BUILD_TYPE=%cmake_build_type% -DWIN32=WIN32 -DENABLE_LIBRDKAFKA=%build_kafka% -DENABLE_JNI=%build_jni% -DOPENSSL_OFF=OFF -DENABLE_COAP=%build_coap% -DENABLE_AWS=%build_AWS% -DENABLE_PDH=%build_PDH% -DENABLE_AZURE=%build_azure% -DENABLE_SFTP=%build_SFTP% -DENABLE_SPLUNK=%build_SPLUNK% -DENABLE_GCP=%build_GCP% -DENABLE_NANOFI=%build_nanofi% -DENABLE_OPENCV=%build_opencv% -DENABLE_PROMETHEUS=%build_prometheus% -DENABLE_ELASTICSEARCH=%build_ELASTIC% -DUSE_SHARED_LIBS=OFF -DDISABLE_CONTROLLER=ON -DBUILD_ROCKSDB=ON -DFORCE_WINDOWS=ON -DUSE_SYSTEM_UUID=OFF -DDISABLE_LIBARCHIVE=OFF -DENABLE_SCRIPTING=OFF -DEXCLUDE_BOOST=ON -DENABLE_WEL=ON -DFAIL_ON_WARNINGS=OFF -DSKIP_TESTS=%skiptests% %strict_gsl_checks% %redist% -DENABLE_LINT
ER=%build_linter% "%scriptdir%" && msbuild /m nifi-minifi-cpp.sln /property:Configuration=%cmake_build_type% /property:Platform=%build_platform% && copy bin\%cmake_build_type%\minifi.exe main\
+)
Review Comment:
Good point, thanks, updated in d578834c8d66fac12537468c21b8887283a38045
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] fgerlits closed pull request #1364: MINIFICPP-1878 Enable Ninja build in Windows builds
Posted by GitBox <gi...@apache.org>.
fgerlits closed pull request #1364: MINIFICPP-1878 Enable Ninja build in Windows builds
URL: https://github.com/apache/nifi-minifi-cpp/pull/1364
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] szaszm commented on a diff in pull request #1364: MINIFICPP-1878 Enable Ninja build in Windows builds
Posted by GitBox <gi...@apache.org>.
szaszm commented on code in PR #1364:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1364#discussion_r915508058
##########
win_build_vs.bat:
##########
@@ -73,12 +73,17 @@ for %%x in (%*) do (
if [%%~x] EQU [/NONFREEUCRT] set "redist=-DMSI_REDISTRIBUTE_UCRT_NONASL=ON"
if [%%~x] EQU [/L] set build_linter=ON
if [%%~x] EQU [/RO] set real_odbc=ON
+ if [%%~x] EQU [/NJ] set generator="Ninja"
)
mkdir %builddir%
pushd %builddir%\
-cmake -G %generator% -A %build_platform% -DINSTALLER_MERGE_MODULES=%installer_merge_modules% -DTEST_CUSTOM_WEL_PROVIDER=%test_custom_wel_provider% -DENABLE_SQL=%build_SQL% -DUSE_REAL_ODBC_TEST_DRIVER=%real_odbc% -DCMAKE_BUILD_TYPE_INIT=%cmake_build_type% -DCMAKE_BUILD_TYPE=%cmake_build_type% -DWIN32=WIN32 -DENABLE_LIBRDKAFKA=%build_kafka% -DENABLE_JNI=%build_jni% -DOPENSSL_OFF=OFF -DENABLE_COAP=%build_coap% -DENABLE_AWS=%build_AWS% -DENABLE_PDH=%build_PDH% -DENABLE_AZURE=%build_azure% -DENABLE_SFTP=%build_SFTP% -DENABLE_SPLUNK=%build_SPLUNK% -DENABLE_GCP=%build_GCP% -DENABLE_NANOFI=%build_nanofi% -DENABLE_OPENCV=%build_opencv% -DENABLE_PROMETHEUS=%build_prometheus% -DENABLE_ELASTICSEARCH=%build_ELASTIC% -DUSE_SHARED_LIBS=OFF -DDISABLE_CONTROLLER=ON -DBUILD_ROCKSDB=ON -DFORCE_WINDOWS=ON -DUSE_SYSTEM_UUID=OFF -DDISABLE_LIBARCHIVE=OFF -DENABLE_SCRIPTING=OFF -DEXCLUDE_BOOST=ON -DENABLE_WEL=ON -DFAIL_ON_WARNINGS=OFF -DSKIP_TESTS=%skiptests% %strict_gsl_checks% %redist% -DENABLE_LINTER=%
build_linter% "%scriptdir%" && msbuild /m nifi-minifi-cpp.sln /property:Configuration=%cmake_build_type% /property:Platform=%build_platform% && copy bin\%cmake_build_type%\minifi.exe main\
+if [%generator%] EQU ["Ninja"] (
+ cmake -G %generator% -DINSTALLER_MERGE_MODULES=%installer_merge_modules% -DTEST_CUSTOM_WEL_PROVIDER=%test_custom_wel_provider% -DENABLE_SQL=%build_SQL% -DUSE_REAL_ODBC_TEST_DRIVER=%real_odbc% -DCMAKE_BUILD_TYPE_INIT=%cmake_build_type% -DCMAKE_BUILD_TYPE=%cmake_build_type% -DWIN32=WIN32 -DENABLE_LIBRDKAFKA=%build_kafka% -DENABLE_JNI=%build_jni% -DOPENSSL_OFF=OFF -DENABLE_COAP=%build_coap% -DENABLE_AWS=%build_AWS% -DENABLE_PDH=%build_PDH% -DENABLE_AZURE=%build_azure% -DENABLE_SFTP=%build_SFTP% -DENABLE_SPLUNK=%build_SPLUNK% -DENABLE_GCP=%build_GCP% -DENABLE_NANOFI=%build_nanofi% -DENABLE_OPENCV=%build_opencv% -DENABLE_PROMETHEUS=%build_prometheus% -DENABLE_ELASTICSEARCH=%build_ELASTIC% -DUSE_SHARED_LIBS=OFF -DDISABLE_CONTROLLER=ON -DBUILD_ROCKSDB=ON -DFORCE_WINDOWS=ON -DUSE_SYSTEM_UUID=OFF -DDISABLE_LIBARCHIVE=OFF -DENABLE_SCRIPTING=OFF -DEXCLUDE_BOOST=ON -DENABLE_WEL=ON -DFAIL_ON_WARNINGS=OFF -DSKIP_TESTS=%skiptests% %strict_gsl_checks% %redist% -DENABLE_LINTER=%build_linter% "%
scriptdir%" && ninja
+) else (
+ cmake -G %generator% -A %build_platform% -DINSTALLER_MERGE_MODULES=%installer_merge_modules% -DTEST_CUSTOM_WEL_PROVIDER=%test_custom_wel_provider% -DENABLE_SQL=%build_SQL% -DUSE_REAL_ODBC_TEST_DRIVER=%real_odbc% -DCMAKE_BUILD_TYPE_INIT=%cmake_build_type% -DCMAKE_BUILD_TYPE=%cmake_build_type% -DWIN32=WIN32 -DENABLE_LIBRDKAFKA=%build_kafka% -DENABLE_JNI=%build_jni% -DOPENSSL_OFF=OFF -DENABLE_COAP=%build_coap% -DENABLE_AWS=%build_AWS% -DENABLE_PDH=%build_PDH% -DENABLE_AZURE=%build_azure% -DENABLE_SFTP=%build_SFTP% -DENABLE_SPLUNK=%build_SPLUNK% -DENABLE_GCP=%build_GCP% -DENABLE_NANOFI=%build_nanofi% -DENABLE_OPENCV=%build_opencv% -DENABLE_PROMETHEUS=%build_prometheus% -DENABLE_ELASTICSEARCH=%build_ELASTIC% -DUSE_SHARED_LIBS=OFF -DDISABLE_CONTROLLER=ON -DBUILD_ROCKSDB=ON -DFORCE_WINDOWS=ON -DUSE_SYSTEM_UUID=OFF -DDISABLE_LIBARCHIVE=OFF -DENABLE_SCRIPTING=OFF -DEXCLUDE_BOOST=ON -DENABLE_WEL=ON -DFAIL_ON_WARNINGS=OFF -DSKIP_TESTS=%skiptests% %strict_gsl_checks% %redist% -DENABLE_LINT
ER=%build_linter% "%scriptdir%" && msbuild /m nifi-minifi-cpp.sln /property:Configuration=%cmake_build_type% /property:Platform=%build_platform% && copy bin\%cmake_build_type%\minifi.exe main\
+)
Review Comment:
Could you avoid the duplication of this monster of a line? I'm thinking about introducing a build command variable earlier, and executing that instead of `&& ninja` or `&& msbuild`.
Hopefully something like this could work:
```suggestion
if [%generator%] EQU ["Ninja"] (
set "buildcmd=ninja"
) else (
set "buildcmd=msbuild /m nifi-minifi-cpp.sln /property:Configuration=%cmake_build_type% /property:Platform=%build_platform% && copy bin\%cmake_build_type%\minifi.exe main\"
)
cmake -G %generator% -DINSTALLER_MERGE_MODULES=%installer_merge_modules% -DTEST_CUSTOM_WEL_PROVIDER=%test_custom_wel_provider% -DENABLE_SQL=%build_SQL% -DUSE_REAL_ODBC_TEST_DRIVER=%real_odbc% -DCMAKE_BUILD_TYPE_INIT=%cmake_build_type% -DCMAKE_BUILD_TYPE=%cmake_build_type% -DWIN32=WIN32 -DENABLE_LIBRDKAFKA=%build_kafka% -DENABLE_JNI=%build_jni% -DOPENSSL_OFF=OFF -DENABLE_COAP=%build_coap% -DENABLE_AWS=%build_AWS% -DENABLE_PDH=%build_PDH% -DENABLE_AZURE=%build_azure% -DENABLE_SFTP=%build_SFTP% -DENABLE_SPLUNK=%build_SPLUNK% -DENABLE_GCP=%build_GCP% -DENABLE_NANOFI=%build_nanofi% -DENABLE_OPENCV=%build_opencv% -DENABLE_PROMETHEUS=%build_prometheus% -DENABLE_ELASTICSEARCH=%build_ELASTIC% -DUSE_SHARED_LIBS=OFF -DDISABLE_CONTROLLER=ON -DBUILD_ROCKSDB=ON -DFORCE_WINDOWS=ON -DUSE_SYSTEM_UUID=OFF -DDISABLE_LIBARCHIVE=OFF -DENABLE_SCRIPTING=OFF -DEXCLUDE_BOOST=ON -DENABLE_WEL=ON -DFAIL_ON_WARNINGS=OFF -DSKIP_TESTS=%skiptests% %strict_gsl_checks% %redist% -DENABLE_LINTER=%build_linter% "%sc
riptdir%" && %buildcmd%
```
##########
cmake/BundledCivetWeb.cmake:
##########
@@ -64,12 +61,11 @@ function(use_bundled_civetweb SOURCE_DIR BINARY_DIR)
# Build project
ExternalProject_Add(
civetweb-external
- URL "https://github.com/civetweb/civetweb/archive/v1.12.tar.gz"
- URL_HASH "SHA256=8cab1e2ad8fb3e2e81fed0b2321a5afbd7269a644c44ed4c3607e0a212c6d9e1"
+ GIT_REPOSITORY "https://github.com/civetweb/civetweb.git"
+ GIT_TAG "4447b6501d5c568b4c6c0940eac801ec690b2250" # commit containing fix for MSVC issue https://github.com/civetweb/civetweb/issues/1024
Review Comment:
I would prefer to stay on HTTP and a release version, and keep the necessary patch.
Switching to FetchContent and keeping the git commit ref is a reasonable alternative, too. It doesn't make the commit reference any more stable and tested, but at least it helps with caching artifacts between builds, since one can specify an external FETCHCONTENT_BASE_DIR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] szaszm commented on a diff in pull request #1364: MINIFICPP-1878 Enable Ninja build in Windows builds
Posted by GitBox <gi...@apache.org>.
szaszm commented on code in PR #1364:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1364#discussion_r922107019
##########
cmake/CivetWeb.cmake:
##########
@@ -0,0 +1,46 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+include(FetchContent)
+
+set(CIVETWEB_ENABLE_SSL_DYNAMIC_LOADING "OFF" CACHE STRING "" FORCE)
+set(CIVETWEB_BUILD_TESTING "OFF" CACHE STRING "" FORCE)
+set(CIVETWEB_ENABLE_DUKTAPE "OFF" CACHE STRING "" FORCE)
+set(CIVETWEB_ENABLE_LUA "OFF" CACHE STRING "" FORCE)
+set(CIVETWEB_ENABLE_CXX "ON" CACHE STRING "" FORCE)
+set(CIVETWEB_ALLOW_WARNINGS "ON" CACHE STRING "" FORCE)
+set(CIVETWEB_ENABLE_ASAN "OFF" CACHE STRING "" FORCE)
+
+FetchContent_Declare(civetweb
+ GIT_REPOSITORY "https://github.com/civetweb/civetweb.git"
+ GIT_TAG "4447b6501d5c568b4c6c0940eac801ec690b2250" # commit containing fix for MSVC issue https://github.com/civetweb/civetweb/issues/1024
+)
+
+FetchContent_MakeAvailable(civetweb)
+
+add_dependencies(civetweb-c-library OpenSSL::Crypto OpenSSL::SSL)
+add_dependencies(civetweb-cpp OpenSSL::Crypto OpenSSL::SSL)
+
+set(CIVETWEB_C_FLAGS "${CMAKE_CFLAGS} -Wno-error" CACHE STRING "" FORCE)
+set(CIVETWEB_CXX_LAGS "${CMAKE_CXX_FLAGS} -Wno-error" CACHE STRING "" FORCE)
+string(REPLACE " " ";" REPLACED_C_FLAGS ${CIVETWEB_C_FLAGS})
+string(REPLACE " " ";" REPLACED_CXX_FLAGS ${CIVETWEB_CXX_LAGS})
+target_compile_options(civetweb-c-library INTERFACE ${REPLACED_C_FLAGS})
+target_compile_options(civetweb-cpp INTERFACE ${REPLACED_CXX_FLAGS})
Review Comment:
This takes all of the global flags, appends `-Wno-error` and appends them to anything linking to civet. Two problems:
1. It's a bit excessive to append all the flags from C/CXXFLAGS again to civet. Anything before `-Wno-error` will be duplicated on the command line.
2. The flags should apply to civet itself, but not the dependencies. Use `PRIVATE` instead of `INTERFACE`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org