Add environment hook for CLASSPATH #27
Conversation
|
How likely is this to get merged? I'm trying to use colcon to build a non-ros mixed language workspace, including some components built with gradle, and this PR was very helpful in getting that working. |
|
@jacobperron thanks! I agree this should be in |
IMHO this patch seems well suited to here, e.g. "colcon python" adds modules to Anyways, getting this in any of the two works for me 😄 . |
colcon_gradle/task/gradle/build.py
Outdated
| @@ -56,14 +58,25 @@ def add_arguments(self, *, parser): # noqa: D102 | |||
| help='Run a specific task instead of the default task') | |||
|
|
|||
| async def build( | |||
| self, *, additional_hooks=None, skip_hook_creation=False | |||
| self, *, additional_hooks=[], skip_hook_creation=False | |||
There was a problem hiding this comment.
using an empty list as a default value is generally a problem
https://stackoverflow.com/questions/1132941/least-astonishment-and-the-mutable-default-argument
There was a problem hiding this comment.
Good catch! I've rebased my changes on master and fixed this. PTAL
| 'classpath_jars', Path(args.install_base), pkg.name, | ||
| 'CLASSPATH', os.path.join('share', pkg.name, 'java', '*'), | ||
| mode='prepend') | ||
| additional_hooks += create_environment_hook( |
There was a problem hiding this comment.
do we also need the directory?
I think that the wildcard is enough
There was a problem hiding this comment.
Reading the the docs, I'm lead to believe we need both the path and the wildcard to account for class files AND jars:
A class path entry that contains an asterisk () does not match class files. To match both classes and JAR files in a single directory mydir, use either mydir:mydir/ or mydir/*:mydir. The order chosen determines whether the classes and resources in mydir are loaded before JAR files in mydir or vice versa.
Signed-off-by: Jacob Perron <[email protected]>
jacobperron
force-pushed
the
jacob/classpath_hook
branch
from
c8e6356 to
e16c81e
Compare
This is a quick fix to a problem described in ros2-java/ament_java#9
TL;DR
Jars produced by gradle packages are not added to the CLASSPATH, and so this PR let's colcon handle it.
Maybe this is better suited to go in colcon-ros-gradle, I don't know. But, long-term, I think we should have a standalone Java library that contains helpers for registering ament packages. Such a library can be used by the ament gradle plugin or elsewhere.