Fix compile error: by k-okada · Pull Request #4 · rosjava/genjava

genjava failed to compile in

1. if there is depends to message_generation but no message ware set, pr2_2dnav and pr2_2dnav_slam
2. if there is implict depends to message_generation

, which are reporeted on jsk-ros-pkg/jsk_visualization#551 (comment)

to check case 1) You can download https://github.com/PR2/pr2_navigation_apps package

to check case 2) you can check this with 1.0.27 of https://github.com/jsk-ros-pkg/jsk_visualization, or
if you change 'message_generaion' to one of output of rospack depends-on message_generation, say 'nodelet'

• gradle_project.py: run genjava if the package depends on message_generation/genmsg implicitly
• genjava-extras.camke.em: do not run genjava when three is no message to build

Strictly speaking, I think case 2 is not correct, because it fails when compiling package which depends on message_generation explicitly but not implicitly.
(With 'explicitly', I mean the package calls cmake macros like add_message_files ) http://answers.ros.org/question/186986/cmake-error-unknown-cmake-command-add_message_files/
So even if your change is merged, it fails when there is a package which does not implicitly depend on message_generation but explicitly.
(I know this is quit rare, so your change does work in most cases)

In my opinion, genjava should show error message, when it finds a package which is not correctly configured. (like not listed message_generation as build_depend)

I used explicitly as message_generation is listed in package.xml and implicitly mean some package listed in package.xml depends on message_genetation.
I choose this word from https://github.com/ros-infrastructure/rospkg/blob/master/src/rospkg/rospack.py#L245

◉ Kei Okada

2016/01/16 14:35、Kentaro Wada notifications@github.com のメッセージ:

Strictly speaking, I think case 2 is not correct, because it fails when compiling package which depends on message_generation explicitly but not implicitly.
(With 'explicitly', I mean the package calls cmake macros like add_message_files ) http://answers.ros.org/question/186986/cmake-error-unknown-cmake-command-add_message_files/
So even if your change is merged, it fails when there is a package which does not implicitly depend on message_generation but explicitly.
(I know this is quit rare, so your change does work in most cases)

In my opinion, genjava should show error message, when it finds a package which is not correctly configured. (like not listed message_generation as build_depend)

—
Reply to this email directly or view it on GitHub.

I see.
In that case, the error is because of the not-well-configured package and the change in k-okada@0634856 is not necessary, no?

I think it should be a warning.

It's been quite some while since I've looked at/run this code, but I'll try to provide some useful input anyway.

For 1. the first commit looks like it will certainly make it more robust.

For 2. I would say that genjava should fail when there is only an implicit dependency (using the conventional meaning @k-okada is referring to, i.e. a build_depends listed not in its own package.xml but one of its dependencies) on message_generation. I agree with @wkentaro - it would be good to have a decent warning. However this should probably be built into the message generation cmake api at a higher level (I think).

I added some notes to the second commit.

I agree to it is better to output warning message on that case, but other
message generators except rosjava may not fall nor warn. So what we can do
at message generator layer is just to pass both cases, I think.

◉ Kei Okada

Any proceedings? There are still a lot of packages that cannot be built.
Should we fix all malformed package.xml of packages or genjava?