pnoltes commented on code in PR #453:
URL: https://github.com/apache/celix/pull/453#discussion_r1045087559


##########
cmake/cmake_celix/ContainerPackaging.cmake:
##########
@@ -263,7 +264,16 @@ 
$<JOIN:$<TARGET_PROPERTY:${CONTAINER_TARGET},CONTAINER_EMBEDDED_PROPERTIES>,\\n\
 "
         )
 
-        file(GENERATE OUTPUT "${LAUNCHER_SRC}" INPUT "${STAGE1_LAUNCHER_SRC}")
+        #Rerun generate to do a second parsing of generator expressions
+        file(GENERATE OUTPUT "${STAGE2_LAUNCHER_SRC}" INPUT 
"${STAGE1_LAUNCHER_SRC}")

Review Comment:
   Indeed the second stage is needed to parse another set of generation 
expression. 
   Example given a celix container:
   ```
   add_celix_container(Example NO_COPY BUNDLES Celix::shell)
   ```
   
   Then the first stage will replace 
`$<TARGET_PROPERTY:Example,BUNDLES_CONTAINER_BUNDLES_LEVEL_3>` into a space 
separated list of bundles, which in this case would be 
`"$<TARGET_PROPERTY:Celix::Shell,BUNDLE_FILE> "`.
   
   And the second stage will replace 
`$<TARGET_PROPERTY:Celix::Shell,BUNDLE_FILE>` to the absolute path to the Celix 
shell bundle.
   
   I added the custom command to ensure that the launcher src is only updated 
if the stage 2 is different. I though this was needed to prevent rebuilding 
executable when running `cmake . ; make -j`, but seeing the cmake documentation 
this should not be the case. From the cmake latest file(GENERATE ...) 
documentation
   "
   Generated files are modified and their timestamp updated on subsequent cmake 
runs only if their content is changed.
   "
   
   I will retest if the add_custom_command is really needed.



-- 
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: dev-unsubscr...@celix.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to