lordgamez commented on a change in pull request #1094:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1094#discussion_r645346834



##########
File path: cmake/DockerConfig.cmake
##########
@@ -65,10 +65,19 @@ add_custom_target(
     COMMAND ${CMAKE_SOURCE_DIR}/docker/DockerBuild.sh
         -u 1000
         -g 1000
+        -t minimal
         -v 
${PROJECT_VERSION_MAJOR}.${PROJECT_VERSION_MINOR}.${PROJECT_VERSION_PATCH}
-        -i minimal
+        -c ENABLE_PYTHON=OFF
+        -c ENABLE_LIBRDKAFKA=ON
+        -c ENABLE_AWS=ON
+        -c DISABLE_CONTROLLER=ON
+        -c DISABLE_SCRIPTING=ON
+        -c DISABLE_PYTHON_SCRIPTING=ON
+        -c DISABLE_ENCRYPT_CONFIG=ON
+        -c AWS_ENABLE_UNITY_BUILD=OFF
         -c DOCKER_BASE_IMAGE=${DOCKER_BASE_IMAGE}
         -c BUILD_NUMBER=${BUILD_NUMBER}
+        -c CMAKE_BUILD_TYPE=MinSizeRel

Review comment:
       Actually the minimal image was at least party about size optimization. 
It was meant for use cases in cloud environments where MiNiFi is used as a 
sidecar for every Kubernetes pod for log collection. In this case we only 
wanted to have a very small image and only include what is necessary for this 
use case as sidecards should be minimal in size if they are used in every pod. 
It's not a huge difference if we use the Release build type instead of 
MinSizeRel as it's 33MB or 39.2MB image though. What would be the benefit of 
the Release build in this use case?

##########
File path: thirdparty/openwsman/openwsman.patch
##########
@@ -79,3 +79,39 @@ diff -rupN orig/src/lib/wsman-soap.c 
patched/src/lib/wsman-soap.c
                          max_connections_per_thread = (* fptr)();
                  }
                  else{
+
+diff -rupN orig/src/lib/u/lock.c patched/src/lib/u/lock.c
+--- orig/src/lib/u/lock.c      2021-05-31 13:44:43.992941115 +0200
++++ patched/src/lib/u/lock.c   2021-05-31 12:00:21.972733061 +0200
+@@ -50,7 +50,7 @@
+ extern int pthread_mutexattr_settype(pthread_mutexattr_t *attr, int type);
+ #endif
+ 
+-#if defined (__SVR4) && defined (__sun)
++#if (defined (__SVR4) && defined (__sun)) || !defined(__GLIBC__)

Review comment:
       PTHREAD_MUTEX_RECURSIVE_NP is a non-portable glibc mutex and our docker 
base distro alpine does not have glibc only musl is available. On these systems 
only PTHREAD_MUTEX_RECURSIVE mutex is available that's why we redefine the 
non-portable symbol. An issue with a similar solution: 
https://github.com/godotengine/godot/issues/31555

##########
File path: thirdparty/openwsman/openwsman.patch
##########
@@ -79,3 +79,39 @@ diff -rupN orig/src/lib/wsman-soap.c 
patched/src/lib/wsman-soap.c
                          max_connections_per_thread = (* fptr)();
                  }
                  else{
+
+diff -rupN orig/src/lib/u/lock.c patched/src/lib/u/lock.c
+--- orig/src/lib/u/lock.c      2021-05-31 13:44:43.992941115 +0200
++++ patched/src/lib/u/lock.c   2021-05-31 12:00:21.972733061 +0200
+@@ -50,7 +50,7 @@
+ extern int pthread_mutexattr_settype(pthread_mutexattr_t *attr, int type);
+ #endif
+ 
+-#if defined (__SVR4) && defined (__sun)
++#if (defined (__SVR4) && defined (__sun)) || !defined(__GLIBC__)
+ #define PTHREAD_MUTEX_RECURSIVE_NP PTHREAD_MUTEX_RECURSIVE
+ #endif
+ 
+@@ -94,7 +94,7 @@ void u_destroy_lock(void* data)
+ void u_unlock(void* data)
+ {
+     if ( data )
+-    { 
++    {
+         pthread_mutex_unlock((pthread_mutex_t*)data);
+     }
+ }
+
+diff -rupN orig/include/u/lock.h patched/include/u/lock.h
+--- orig/include/u/lock.h      2021-05-31 13:44:43.992941115 +0200
++++ patched/include/u/lock.h   2021-05-31 12:00:30.792726402 +0200
+@@ -2,7 +2,7 @@
+ #ifndef LOCKING_H
+ #define LOCKING_H
+ 
+-#if defined (__FreeBSD__)  || defined (__OpenBSD__) || defined (__NetBSD__) 
|| defined (__APPLE__)
++#if defined (__FreeBSD__)  || defined (__OpenBSD__) || defined (__NetBSD__) 
|| defined (__APPLE__) || !defined(__GLIBC__)

Review comment:
       Same as above

##########
File path: cmake/DockerConfig.cmake
##########
@@ -65,10 +65,19 @@ add_custom_target(
     COMMAND ${CMAKE_SOURCE_DIR}/docker/DockerBuild.sh
         -u 1000
         -g 1000
+        -t minimal
         -v 
${PROJECT_VERSION_MAJOR}.${PROJECT_VERSION_MINOR}.${PROJECT_VERSION_PATCH}
-        -i minimal
+        -c ENABLE_PYTHON=OFF
+        -c ENABLE_LIBRDKAFKA=ON
+        -c ENABLE_AWS=ON
+        -c DISABLE_CONTROLLER=ON
+        -c DISABLE_SCRIPTING=ON
+        -c DISABLE_PYTHON_SCRIPTING=ON
+        -c DISABLE_ENCRYPT_CONFIG=ON
+        -c AWS_ENABLE_UNITY_BUILD=OFF
         -c DOCKER_BASE_IMAGE=${DOCKER_BASE_IMAGE}
         -c BUILD_NUMBER=${BUILD_NUMBER}
+        -c CMAKE_BUILD_TYPE=MinSizeRel

Review comment:
       Actually the minimal image was at least party about size optimization. 
It was meant for use cases in cloud environments where MiNiFi is used as a 
sidecar for every Kubernetes pod for log collection. In this case we only 
wanted to have a very small image and only include what is necessary for this 
use case as sidecars should be minimal in size if they are used in every pod. 
It's not a huge difference if we use the Release build type instead of 
MinSizeRel as it's 33MB or 39.2MB image though. What would be the benefit of 
the Release build in this use case?

##########
File path: cmake/DockerConfig.cmake
##########
@@ -65,10 +65,19 @@ add_custom_target(
     COMMAND ${CMAKE_SOURCE_DIR}/docker/DockerBuild.sh
         -u 1000
         -g 1000
+        -t minimal
         -v 
${PROJECT_VERSION_MAJOR}.${PROJECT_VERSION_MINOR}.${PROJECT_VERSION_PATCH}
-        -i minimal
+        -c ENABLE_PYTHON=OFF
+        -c ENABLE_LIBRDKAFKA=ON
+        -c ENABLE_AWS=ON
+        -c DISABLE_CONTROLLER=ON
+        -c DISABLE_SCRIPTING=ON
+        -c DISABLE_PYTHON_SCRIPTING=ON
+        -c DISABLE_ENCRYPT_CONFIG=ON
+        -c AWS_ENABLE_UNITY_BUILD=OFF
         -c DOCKER_BASE_IMAGE=${DOCKER_BASE_IMAGE}
         -c BUILD_NUMBER=${BUILD_NUMBER}
+        -c CMAKE_BUILD_TYPE=MinSizeRel

Review comment:
       Actually the minimal image was at least party about size optimization. 
It was meant for use cases in cloud environments where MiNiFi is used as a 
sidecar for every Kubernetes pod for log collection. In this case we only 
wanted to have a very small image and only include what is necessary for this 
use case as sidecars should be minimal in size if they are used in every pod. 
It's not a huge difference if we use the Release build type instead of 
MinSizeRel as it's a 33MB vs a 39.2MB image though. What would be the benefit 
of the Release build in this use case?

##########
File path: CMakeLists.txt
##########
@@ -583,8 +583,11 @@ add_subdirectory(main)
 add_subdirectory(nanofi)
 add_dependencies(nanofi minifiexe)
 
-add_subdirectory(encrypt-config)
-add_dependencies(encrypt-config minifi)
+option(DISABLE_ENCRYPT_CONFIG "Disables build of encrypt-config binary." OFF)
+if (NOT DISABLE_ENCRYPT_CONFIG)

Review comment:
       Thanks, updated in d6f56f9a2a7808d459cb72afc7da5f2f091fd710




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

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to