Message dependencies do not update when a dependent message's dependencies change

[jbrindza]

I think I said that right in the title.

Basically there is a case when the message generation is not rerun even when it should be. The problem is the list of .msg dependencies is generated during the cmake configure step and if you add a new field to a .msg file that is another message type that dependency list is never updated (until you rerun the cmake step).

You can reproduce the problem as follows.

1. Create a message package with two messages.

   a1.msg:

    int32 a1
   

   a2.msg:

    int32 a2
   

2. Run catkin_make. It will generate a1.h and a2.h.

3. In a1.msg add an field that is of type a2.

   a1.msg:

    int32 a1
    a2 a2_msg
   

4. Run catkin_make. a1.h is regenerated correctly.

5. Add any field to a2.msg.

   a2.msg:

    int32 a2
    float32 a2_float
   

6. Run catkin_make. a2.h is regenerated correctly. However a1.h is not. You can look at the "Definition" in a1.h and see it is missing the new field "a2_float". The md5sum is also wrong. If you delete the build directory and do a clean build the md5sum for a1 is now different.

[dirk-thomas]

I can confirm the described behavior. Whenever a dependency to other messages is introduced a CMake reconfigure is necessary.

There are two possible fixes for this:

• stamp() each message/service file which will trigger a reconfigure every time one of those files is touched. The patch for that would be adding the following to pkg-genmsg.cmake.em:

@{all_deps = dict(msg_deps.items() + srv_deps.items())}
# stamp all message/service files to trigger reconfigure on change
@[for f in all_deps.keys()]@
get_filename_component(filename "@(f)" NAME)
configure_file("@(f)"
  ${CMAKE_CURRENT_BINARY_DIR}/catkin_generated/genmsg_stamps/${PROJECT_NAME}/${filename}.stamp
  COPYONLY)
@[end for]@# messages and services

• detect the change of dependencies at make-time, print a reasonable error message telling that the user has to rerun cmake and error out

While version one is easy to implement it results in a significant overhead for reconfiguring in a lot of cases where it would not be necessary.

The second version takes more effort to implement and will take a constant time during make to perform that check (invoking another Python script for each message/service file). But the big advantage will be that reconfiguring will only needed when absolutely necessary. I have implemented this in PR #42. Please consider testing it and providing feedback if it works for you.

[dirk-thomas]

The latest release of genmsg (0.5.1) failed downstream packages because of the implementation of version two in #42 (e.g. http://jenkins.ros.org/view/IbinT64/job/ros-indigo-std-srvs_binarydeb_trusty_i386/8/console).

The problem is that during the install step we currently do not source the setup file. But since install depends on all targets it tries to run the dependency check command which requires genmsg to be available in Python. (Potentially install can perform exactly the same as a build.)

We have two options here:

• fall back to implement version one with the obvious disadvantages stated above
• update the Debian rule file generated by bloom to also source the setup before performing an installation (patching only the genmsg gbp is not sufficient since it is required for every downstream package using genmsg).

@esteve @tfoote @wjwwood What do you think?

[wjwwood]

Related: ros-infrastructure/bloom#204

[tfoote]

If we're already exposing the setup file to the make command. We should do the same for the install command since it usually inherently invokes the make command. This will also resolve the bloom issue for gradle. So I'd suggest we do the debuan rule file update.

[dirk-thomas]

I would agree to this. I we want to go this route we should do it soonish because all affected indigo packages will need to be re-released - and the number is growing every day.

[wjwwood]

Is the patch proposed in ros-infrastructure/bloom#204 appropriate? I can just merge that one if it is.

[dirk-thomas]

It overrides the install command and only adds the sourcing of the setup file if it is available (like it does for all the other targets). So yes, it looks to me that it does exactly the right thing.

Should I try do some indigo releases with this patch locally applied to validate that it actually makes the genmsg stuff pass on the farm?

[wjwwood]

That sounds like a good idea, if it works then I can merge it and release a new version of bloom.

[dirk-thomas]

Patched bloom version worked for:

• std_msgs: 0.5.8-1 in 'indigo/distribution.yaml' [bloom] rosdistro#3369 --> http://jenkins.ros.org/job/ros-indigo-std-msgs_binarydeb_trusty_amd64/
• ros_comm_msgs: 1.10.1-1 in 'indigo/distribution.yaml' [bloom] rosdistro#3370 --> http://jenkins.ros.org/job/ros-indigo-std-srvs_binarydeb_trusty_amd64/

@wjwwood Please go ahead with a bloom patch release containing the PR (with tabs).

This ticket will be closed again - we stick to version two and change the Debian rule file instead.