[GitHub] [nifi-minifi-cpp] lordgamez commented on a change in pull request #1094: MINIFICPP-1576 Allow build of all extensions in docker and cleanup docker files

[GitBox <gi...@apache.org>]

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




-- 
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:
users@infra.apache.org