Skip to content

Conversation

@gonzodepedro
Copy link
Collaborator

Adds a cmake file and instructions to CMakeList.txt to build a sources jar for rcljava

Signed-off-by: Gonzalo de Pedro gonzalo@depedro.com.ar

Signed-off-by: Gonzalo de Pedro <gonzalo@depedro.com.ar>
@jacobperron jacobperron changed the title Buidl sources jar for rcljava Build sources jar for rcljava Feb 23, 2021
Copy link

@jacobperron jacobperron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding this!

I've given it a first pass and left some initial feedback. Was there some references used when adding this? It might be good to attribute any references used.

@gonzodepedro
Copy link
Collaborator Author

Thanks for adding this!

I've given it a first pass and left some initial feedback. Was there some references used when adding this? It might be good to attribute any references used.

This is highly inspired in the add_jar function found in UseJava cmake module. It is also intended to be used together with it, as there are some functions defined in that module used here.

Signed-off-by: Gonzalo de Pedro <gonzalo@depedro.com.ar>
@jacobperron
Copy link

After looking at this a bit more, I'm confused why add_jar doesn't supply an option to just bundle the source files, or resource-only jars (ie. no compilation). Perhaps it's not a common use-case?

It seems rather simple to do from the command-line, e.g.

jar -cf mysource.jar src/*.java

@gonzodepedro Do you think we could make this cmake implementation simpler (to make maintenance easier)? It seems like there's a lot of special cases being handled that we don't care about. Really, all we want is a way to bundle resources files without doing any compiling.

Roughly, I imagine the minimum function looking something like this:

function(add_source_jar TARGET_NAME)
 
  # argument parsing goes here ...

  # Run 'jar passing source files directly (we don't really care what kinds of files they are)
  add_custom_command(
    OUTPUT ${_add_source_jar_OUTPUT}
    COMMAND ${Java_JAR_EXECUTABLE} -cf ${_add_source_jar_OUTPUT} ${_add_source_jar_SOURCES}
    WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
  )
  add_custom_target(${TARGET_NAME} ALL DEPENDS ${_add_source_jar_OUTPUT})

  # Set install property (for 'install_jar()').
  set_property(        
    TARGET ${_target_name}
    PROPERTY
      INSTALL_FILES ${_source_jar_file}
  )

  # Maybe also set JAR_FILE property?
  # I don't think that JNI_SYMLINK or CLASSDIR makes sense for resource-only jars
endfunction()

What do you think?

Signed-off-by: Gonzalo de Pedro <gonzalo@depedro.com.ar>
Copy link

@jacobperron jacobperron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for refactoring and moving to rcljava_common. Looks more straight-forward now 😅

A couple more minor comments, otherwise LGTM

endif()
include(UseJava)
include(JavaExtra)
#include(cmake/add_source_jar.cmake)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: we can remove this line

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed

"SOURCES"
${ARGN}
)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably handle the case where OUTPUT_NAME is not given. Maybe it can default to the "${_TARGET_NAME}.jar"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Signed-off-by: Gonzalo de Pedro <gonzalo@depedro.com.ar>
@jacobperron jacobperron merged commit 95cb555 into osrf:galactic-devel Mar 30, 2021
ivanpauno pushed a commit that referenced this pull request May 17, 2021
* Add cmake function for bundling source files into a jar
* Create and install source jar for rcljava

Signed-off-by: Gonzalo de Pedro <gonzalo@depedro.com.ar>

Co-authored-by: Jacob Perron <jacob@openrobotics.org>
jacobperron added a commit to ros2-java/ros2_java that referenced this pull request May 17, 2022
* Add cmake function for bundling source files into a jar
* Create and install source jar for rcljava

Signed-off-by: Gonzalo de Pedro <gonzalo@depedro.com.ar>

Co-authored-by: Jacob Perron <jacob@openrobotics.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants