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 2021/07/05 18:28:06 UTC

[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #1124: MINIFICPP-1367 Add option to disable NanoFi build

szaszm commented on a change in pull request #1124:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1124#discussion_r664080797



##########
File path: CMakeLists.txt
##########
@@ -589,12 +592,12 @@ if (NOT DISABLE_CURL AND NOT DISABLE_CONTROLLER)
 endif()
 
 
-if (NOT DISABLE_CURL)
-  if (ENABLE_PYTHON)
-  	if (NOT WIN32)
-  		add_subdirectory(python/library)
-  	endif()
-  endif(ENABLE_PYTHON)
+if (NOT DISABLE_CURL AND ENABLE_PYTHON AND NOT WIN32)
+  if (ENABLE_NANOFI)
+		add_subdirectory(python/library)
+	else()
+		message(FATAL_ERROR "Nanofi, a dependency of the python extension is disabled, therefore Python extension cannot be enabled.")
+	endif()

Review comment:
       We have some strange indentation here.

##########
File path: cmake/BuildTests.cmake
##########
@@ -39,51 +39,54 @@ if(NOT EXCLUDE_BOOST)
 endif()
 
 function(appendIncludes testName)
-    target_include_directories(${testName} SYSTEM BEFORE PRIVATE "${CMAKE_SOURCE_DIR}/thirdparty/catch")
-    target_include_directories(${testName} BEFORE PRIVATE "${CMAKE_SOURCE_DIR}/include")
-    target_include_directories(${testName} BEFORE PRIVATE "${CMAKE_SOURCE_DIR}/libminifi/include/")
-    target_include_directories(${testName} BEFORE PRIVATE "${CMAKE_SOURCE_DIR}/libminifi/include/c2/protocols")
-    target_include_directories(${testName} BEFORE PRIVATE "${CMAKE_SOURCE_DIR}/libminifi/include/c2")
-    target_include_directories(${testName} BEFORE PRIVATE "${CMAKE_SOURCE_DIR}/libminifi/include/core")
-    target_include_directories(${testName} BEFORE PRIVATE "${CMAKE_SOURCE_DIR}/libminifi/include/core/controller")
-    target_include_directories(${testName} BEFORE PRIVATE "${CMAKE_SOURCE_DIR}/libminifi/include/core/repository")
-    target_include_directories(${testName} BEFORE PRIVATE "${CMAKE_SOURCE_DIR}/libminifi/include/core/yaml")
-    target_include_directories(${testName} BEFORE PRIVATE "${CMAKE_SOURCE_DIR}/libminifi/include/core/statemanagement")
-    target_include_directories(${testName} BEFORE PRIVATE "${CMAKE_SOURCE_DIR}/libminifi/include/core/statemanagement/metrics")
-    target_include_directories(${testName} BEFORE PRIVATE "${CMAKE_SOURCE_DIR}/libminifi/include/io")
-    if(WIN32)
-    	target_include_directories(${testName} BEFORE PRIVATE "${CMAKE_SOURCE_DIR}/libminifi/opsys/win")
-    	target_include_directories(${testName} BEFORE PRIVATE "${CMAKE_SOURCE_DIR}/libminifi/opsys/win/io")
-    else()
-    	target_include_directories(${testName} BEFORE PRIVATE "${CMAKE_SOURCE_DIR}/libminifi/opsys/posix")
-    	target_include_directories(${testName} BEFORE PRIVATE "${CMAKE_SOURCE_DIR}/libminifi/opsys/posix/io")
-    endif()
-    target_include_directories(${testName} BEFORE PRIVATE "${CMAKE_SOURCE_DIR}/libminifi/include/utils")
-    target_include_directories(${testName} BEFORE PRIVATE "${CMAKE_SOURCE_DIR}/libminifi/include/processors")
-    target_include_directories(${testName} BEFORE PRIVATE "${CMAKE_SOURCE_DIR}/libminifi/include/provenance")
+  target_include_directories(${testName} SYSTEM BEFORE PRIVATE "${CMAKE_SOURCE_DIR}/thirdparty/catch")
+  target_include_directories(${testName} BEFORE PRIVATE "${CMAKE_SOURCE_DIR}/include")
+  target_include_directories(${testName} BEFORE PRIVATE "${CMAKE_SOURCE_DIR}/libminifi/include/")
+  target_include_directories(${testName} BEFORE PRIVATE "${CMAKE_SOURCE_DIR}/libminifi/include/c2/protocols")
+  target_include_directories(${testName} BEFORE PRIVATE "${CMAKE_SOURCE_DIR}/libminifi/include/c2")
+  target_include_directories(${testName} BEFORE PRIVATE "${CMAKE_SOURCE_DIR}/libminifi/include/core")
+  target_include_directories(${testName} BEFORE PRIVATE "${CMAKE_SOURCE_DIR}/libminifi/include/core/controller")
+  target_include_directories(${testName} BEFORE PRIVATE "${CMAKE_SOURCE_DIR}/libminifi/include/core/repository")
+  target_include_directories(${testName} BEFORE PRIVATE "${CMAKE_SOURCE_DIR}/libminifi/include/core/yaml")
+  target_include_directories(${testName} BEFORE PRIVATE "${CMAKE_SOURCE_DIR}/libminifi/include/core/statemanagement")
+  target_include_directories(${testName} BEFORE PRIVATE "${CMAKE_SOURCE_DIR}/libminifi/include/core/statemanagement/metrics")
+  target_include_directories(${testName} BEFORE PRIVATE "${CMAKE_SOURCE_DIR}/libminifi/include/io")
+  if(WIN32)
+    target_include_directories(${testName} BEFORE PRIVATE "${CMAKE_SOURCE_DIR}/libminifi/opsys/win")
+    target_include_directories(${testName} BEFORE PRIVATE "${CMAKE_SOURCE_DIR}/libminifi/opsys/win/io")
+  else()
+    target_include_directories(${testName} BEFORE PRIVATE "${CMAKE_SOURCE_DIR}/libminifi/opsys/posix")
+    target_include_directories(${testName} BEFORE PRIVATE "${CMAKE_SOURCE_DIR}/libminifi/opsys/posix/io")
+  endif()
+  target_include_directories(${testName} BEFORE PRIVATE "${CMAKE_SOURCE_DIR}/libminifi/include/utils")
+  target_include_directories(${testName} BEFORE PRIVATE "${CMAKE_SOURCE_DIR}/libminifi/include/processors")
+  target_include_directories(${testName} BEFORE PRIVATE "${CMAKE_SOURCE_DIR}/libminifi/include/provenance")
 endfunction()
 
 function(createTests testName)
-    message ("-- Adding test: ${testName}")
-    appendIncludes("${testName}")
+  message ("-- Adding test: ${testName}")

Review comment:
       Unrelated, but we should really change this to `message(DEBUG, "...")`. It would be nice to reduce the build output to useful info only.

##########
File path: cmake/BuildTests.cmake
##########
@@ -120,29 +123,30 @@ FOREACH(testfile ${UNIT_TESTS})
 ENDFOREACH()
 message("-- Finished building ${UNIT_TEST_COUNT} unit test file(s)...")
 
-if(NOT WIN32)
+if(NOT WIN32 AND ENABLE_NANOFI)

Review comment:
       Can you increase the indentation of the contents, until line 147?

##########
File path: bstrp_functions.sh
##########
@@ -368,6 +390,7 @@ show_supported_features() {
   echo "W. Openwsman Support ...........$(print_feature_status OPENWSMAN_ENABLED)"
   echo "X. Azure Support ...............$(print_feature_status AZURE_ENABLED)"
   echo "Y. Systemd Support .............$(print_feature_status SYSTEMD_ENABLED)"
+  echo "Z. NanoFi Support ..............$(print_feature_status NANOFI_ENABLED)"

Review comment:
       We ran out of letters. I wonder what's going to happen to the next feature.




-- 
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