Merge pull request #605 from apache/feature/allow_duplicate_bundles_i…

…n_container

Feature/allow duplicate bundles in container

.github/workflows/macos.yml

@@ -30,6 +30,7 @@ jobs:
             -o celix:build_all=True
             -o celix:enable_testing_for_cxx14=True
             -o celix:enable_testing_dependency_manager_for_cxx11=True
+            -o celix:enable_cmake_warning_tests=True
             -o celix:enable_testing_on_ci=True
             -o celix:framework_curlinit=False
         run: |

.github/workflows/ubuntu.yml

@@ -31,13 +31,13 @@ jobs:
           # build profile
           conan profile new release --detect
           conan profile update settings.build_type=Release release
-          #Note no backwards compatiblity for gcc5 needed, setting libcxx to c++11.
+          #Note no backwards compatibility for gcc5 needed, setting libcxx to c++11.
           conan profile update settings.compiler.libcxx=libstdc++11 release
           conan profile show release
           # host profile
           conan profile new default --detect
           conan profile update settings.build_type=${{ matrix.type }} default
-          #Note no backwards compatiblity for gcc5 needed, setting libcxx to c++11.
+          #Note no backwards compatibility for gcc5 needed, setting libcxx to c++11.
           conan profile update settings.compiler.libcxx=libstdc++11 default
           conan profile show default
       - name: Configure and install dependencies
@@ -50,6 +50,7 @@ jobs:
             -o celix:build_all=True
             -o celix:enable_testing_for_cxx14=True
             -o celix:enable_testing_dependency_manager_for_cxx11=True
+            -o celix:enable_cmake_warning_tests=True
             -o celix:enable_testing_on_ci=True
             -o celix:framework_curlinit=False
         run: |

CMakeLists.txt

@@ -158,7 +158,7 @@ endif ()
 
 option(ENABLE_TESTING_DEPENDENCY_MANAGER_FOR_CXX11 "Test the Dependency Manager for C++11 support" OFF)
 option(ENABLE_TESTING_FOR_CXX14 "Test celix utils and framework C++ header for C++14 support" OFF)
-
+option(ENABLE_CMAKE_WARNING_TESTS "Enable cmake warning tests to test warning prints" OFF)
 option(ENABLE_TESTING_ON_CI "Whether to enable testing on CI. This influence allowed timing errors during unit tests" OFF)
 
 if (CELIX_INSTALL_DEPRECATED_API)
@@ -195,6 +195,9 @@ add_subdirectory(misc/experimental)
 #Example as last, because some example will check if underlining options are enabled
 add_subdirectory(examples/celix-examples)
 
+#include CMake warning tests
+include(cmake/celix_project/WarningTests.cmake)
+
 #export targets
 install(EXPORT celix NAMESPACE Celix:: DESTINATION share/celix/cmake FILE Targets.cmake COMPONENT cmake)
 install_celix_targets(celix NAMESPACE Celix:: DESTINATION share/celix/cmake FILE CelixTargets COMPONENT cmake)

cmake/celix_project/WarningTests.cmake

@@ -0,0 +1,55 @@
+# 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.
+
+#[[
+Misc usage of Apache Celix CMake functions to check if they work as expected.
+Including some constructions that will generate warnings.
+]]
+
+if (ENABLE_CMAKE_WARNING_TESTS AND TARGET Celix::shell AND TARGET Celix::shell_tui)
+    add_celix_container(example_with_duplicate_bundles_1
+            INSTALL_BUNDLES Celix::shell
+            BUNDLES Celix::shell_tui #add bundles with run level 3
+    )
+
+    #Adding a bundle twice on the same run level is fine and the last entry should be ignored
+    celix_container_bundles(example_with_duplicate_bundles_1 INSTALL Celix::shell)
+    celix_container_bundles(example_with_duplicate_bundles_1 LEVEL 3 Celix::shell_tui)
+
+    #Starting a bundle that is already installed is fine and should not result in a warning
+    celix_container_bundles(example_with_duplicate_bundles_1 LEVEL 3 Celix::shell)
+
+    #Adding a bundle twice on different run levels should result in an warning
+    celix_container_bundles(example_with_duplicate_bundles_1 LEVEL 4 Celix::shell_tui)
+    celix_container_bundles(example_with_duplicate_bundles_1 LEVEL 4 Celix::shell)
+
+    add_celix_container(example_with_duplicate_bundles_2
+            BUNDLES Celix::shell_tui #add bundles with run level 3
+    )
+
+    #Adding an embedded bundle as install and later with a run level which is already added as a normal bundle
+    #will not lead to a warning because multiple bundles are checked based on path and not based on the bundle
+    #symbolic name.
+    celix_container_embedded_bundles(example_with_duplicate_bundles_2 INSTALL Celix::shell_tui)
+    celix_container_embedded_bundles(example_with_duplicate_bundles_2 LEVEL 3 Celix::shell_tui)
+
+    #Adding an embedded again will not lead to an warning (ignored)
+    celix_container_embedded_bundles(example_with_duplicate_bundles_2 LEVEL 3 Celix::shell_tui)
+
+    #But adding an embedded bundle with a different run level will lead to an warning
+    celix_container_embedded_bundles(example_with_duplicate_bundles_2 LEVEL 4 Celix::shell_tui)
+endif ()

cmake/cmake_celix/ContainerPackaging.cmake

@@ -440,10 +440,9 @@ function(celix_container_bundles_dir)
 
     set(DEST_DIR "${CONTAINER_LOC}/${BD_DIR_NAME}")
     get_target_property(CLEAN_FILES ${CONTAINER_TARGET} "ADDITIONAL_CLEAN_FILES")
-    if (NOT ${DEST_DIR} IN_LIST CLEAN_FILES)
-        list(APPEND CLEAN_FILES ${DEST_DIR})
-        set_target_properties(${CONTAINER_TARGET} PROPERTIES "ADDITIONAL_CLEAN_FILES" "${CLEAN_FILES}")
-    endif()
+    list(APPEND CLEAN_FILES ${DEST_DIR})
+    list(REMOVE_DUPLICATES CLEAN_FILES)
+    set_target_properties(${CONTAINER_TARGET} PROPERTIES "ADDITIONAL_CLEAN_FILES" "${CLEAN_FILES}")
 
     foreach(BUNDLE IN ITEMS ${BD_BUNDLES})
         if (IS_ABSOLUTE ${BUNDLE} AND EXISTS ${BUNDLE})
@@ -479,14 +478,13 @@ function(celix_container_bundles_dir)
             message(FATAL_ERROR "Cannot add bundle in container ${CONTAINER_TARGET}. Provided bundle is not a abs path to an existing file nor a cmake target (${BUNDLE}).")
         endif ()
 
-        if (BUNDLE_ID AND DEST AND BUNDLE_FILE)
+        if (BUNDLE_ID AND DEST AND BUNDLE_FILE AND NOT TARGET ${CONTAINER_TARGET}_copy_bundle_${BUNDLE_ID})
             add_custom_target(${CONTAINER_TARGET}_copy_bundle_${BUNDLE_ID}
                 COMMAND ${CMAKE_COMMAND} -E copy_if_different ${BUNDLE_FILE} ${DEST}
                 BYPRODUCTS ${DEST}
                 DEPENDS ${DEPS}
             )
             add_dependencies(${CONTAINER_TARGET} ${CONTAINER_TARGET}_copy_bundle_${BUNDLE_ID})
-
             get_target_property(CLEAN_FILES ${CONTAINER_TARGET} "ADDITIONAL_CLEAN_FILES")
             list(APPEND CLEAN_FILES ${DEST})
             set_target_properties(${CONTAINER_TARGET} PROPERTIES "ADDITIONAL_CLEAN_FILES" "${CLEAN_FILES}")
@@ -595,24 +593,56 @@ function(celix_container_bundles)
             message(FATAL_ERROR "Cannot add bundle `${BUNDLE}` to container target ${CONTAINER_TARGET}. Argument is not a path or cmake target")
         endif ()
 
-       if(COPY)
-            list(APPEND BUNDLES ${COPY_LOC})
-       else()
-            list(APPEND BUNDLES ${ABS_LOC})
-       endif()
-   endforeach()
+       if (COPY)
+           set(BUNDLE_TO_ADD ${COPY_LOC})
+       else ()
+           set(BUNDLE_TO_ADD ${ABS_LOC})
+       endif ()
+
+       list(FIND BUNDLES ${BUNDLE_TO_ADD} INDEX)
+       if (INDEX EQUAL -1) #Note this ignores the same bundle for the same level
+           _celix_container_check_duplicate_bundles(${CONTAINER_TARGET} ${BUNDLE_TO_ADD} ${BUNDLES_LEVEL})
+           list(APPEND BUNDLES ${BUNDLE_TO_ADD})
+       endif ()
+    endforeach()
 
     if (BUNDLES_INSTALL)
         set_target_properties(${CONTAINER_TARGET} PROPERTIES "CONTAINER_BUNDLES_INSTALL" "${BUNDLES}")
     else () #bundle level 0,1,2,3,4,5 or 6
         set_target_properties(${CONTAINER_TARGET} PROPERTIES "CONTAINER_BUNDLES_LEVEL_${BUNDLES_LEVEL}" "${BUNDLES}")
     endif ()
 
-   if(COPY) 
+   if (COPY)
        celix_container_bundles_dir(${CONTAINER_TARGET} DIR_NAME bundles BUNDLES ${BUNDLES_LIST})
    endif()
 endfunction()
 
+#[[
+Private function to check if there are duplicate bundles in the container and print a warning if so.
+Arg CONTAINER_TARGET ADDED_BUNDLES_LIST TARGET_LEVEL
+]]
+function(_celix_container_check_duplicate_bundles)
+    list(GET ARGN 0 CONTAINER_TARGET)
+    list(GET ARGN 1 TO_ADD_BUNDLE)
+    list(GET ARGN 2 TARGET_LEVEL)
+
+    if (NOT TARGET_LEVEL) #install
+        return() #Bundles can be installed and added to a level
+    endif()
+
+    set(PARTIAL_MSG "Bundle `${TO_ADD_BUNDLE}` is added to the container multiple times. This can lead to errors \
+        during bundle installation. Bundle `${TO_ADD_BUNDLE}` for level ${TARGET_LEVEL} is already added to the \
+        container '${CONTAINER_TARGET}` at level ")
+
+    foreach(BUNDLE_LEVEL RANGE 0 6)
+        get_target_property(BUNDLES ${CONTAINER_TARGET} "CONTAINER_BUNDLES_LEVEL_${BUNDLE_LEVEL}")
+        list(FIND BUNDLES ${TO_ADD_BUNDLE} INDEX)
+        if (INDEX GREATER -1)
+            message(WARNING "${PARTIAL_MSG} ${BUNDLE_LEVEL}.")
+        endif()
+    endforeach()
+endfunction()
+
 #[[
 Embed a selection of bundles to the Celix container.
 
@@ -679,9 +709,17 @@ function(celix_container_embedded_bundles)
             message(FATAL_ERROR "Cannot add bundle `${BUNDLE}` to container target ${CONTAINER_TARGET}. Argument is not a path or cmake target")
         endif ()
         celix_target_embedded_bundle(${CONTAINER_TARGET} BUNDLE ${BUNDLE} NAME ${NAME})
-        list(APPEND BUNDLES "embedded://${NAME}")
+        set(BUNDLE_TO_ADD "embedded://${NAME}")
+
+        list(FIND BUNDLES ${BUNDLE_TO_ADD} INDEX)
+        if (INDEX EQUAL -1) #Note this ignores the same bundle for the same level
+            _celix_container_check_duplicate_bundles(${CONTAINER_TARGET} ${BUNDLE_TO_ADD} ${BUNDLES_LEVEL})
+            list(APPEND BUNDLES ${BUNDLE_TO_ADD})
+        endif ()
     endforeach()
 
+
+
     if (BUNDLES_INSTALL)
         set_target_properties(${CONTAINER_TARGET} PROPERTIES "CONTAINER_BUNDLES_INSTALL" "${BUNDLES}")
     else () #bundle level 0,1,2,3,4,5 or 6

conanfile.py

@@ -98,6 +98,7 @@ class CelixConan(ConanFile):
         "celix_install_deprecated_api": [True, False],
         "celix_use_compression_for_bundle_zips": [True, False],
         "celix_err_buffer_size": "ANY",
+        "enable_cmake_warning_tests": [True, False],
         "enable_testing_on_ci": [True, False],
         "framework_curlinit": [True, False],
     }
@@ -162,6 +163,7 @@ class CelixConan(ConanFile):
         "celix_install_deprecated_api": False,
         "celix_use_compression_for_bundle_zips": True,
         "celix_err_buffer_size": 512,
+        "enable_cmake_warning_tests": False,
         "enable_testing_on_ci": False,
         "framework_curlinit": True,
     }
@@ -197,6 +199,7 @@ def package_id(self):
         del self.info.options.build_shell_bonjour
         del self.info.options.enable_testing_dependency_manager_for_cxx11
         del self.info.options.enable_testing_for_cxx14
+        del self.info.options.enable_cmake_warning_tests
         del self.info.options.enable_testing_on_ci
 
     def build_requirements(self):