ros2 / rosidl_typesupport

Packages which provide the typesupport for ROS messages and services
Apache License 2.0
13 stars 34 forks source link

Reduces action interfaces CMake target name length to please Windows. #41

Closed hidmic closed 5 years ago

hidmic commented 5 years ago

Fixes ros2/build_cop#154. Had to reduce the name length for action targets a bit. I did so everywhere for consistency.

hidmic commented 5 years ago
sloretz commented 5 years ago

Ah it looks like the path in the nightly has the job name in it nightly_win_extra_rmw_release. It's possible ci_windows is short enough but nightly_win_extra_rmw_release isn't.

The change looks good but we won't know for sure until a nightly runs with this fix.

hidmic commented 5 years ago

I've been testing out path lengths separately. Worst case seems to be:

C:\J\workspace\nightly_win_extra_rmw_release\ws\x08uild\example_interfaces\example_interfaces__rosidl_typesupport_cpp__generate_action_interfaces.dir\Release\example_.46C9518E.tlog\example_interfaces__rosidl_typesupport_cpp__generate_action_interfaces.lastbuildstate

That one has a total count of 266 characters. With this change, it goes down to 246 characters.

clalancette commented 5 years ago

C:\J\workspace\nightly_win_extra_rmw_release

Separately, it might be a good idea to rename this job to nightly_win_extra_rmw_rel; not only is it shorter (gaining us a bit more headroom), it also matches job names like https://ci.ros2.org/view/nightly/job/nightly_win_rel/ .

hidmic commented 5 years ago

Sounds good to me, though I'm not sure I have enough powers to do that. Do you @clalancette ?

clalancette commented 5 years ago

Sounds good to me, though I'm not sure I have enough powers to do that. Do you @clalancette ?

Yeah, it starts with a change to https://github.com/ros2/ci/ . Once that is approved, merged, then I have to remember how to deploy it to the buildfarm :). I'll take a look at it in a little bit.

nuclearsandwich commented 5 years ago

Yeah, it starts with a change to https://github.com/ros2/ci/ . Once that is approved, merged, then I have to remember how to deploy it to the buildfarm :). I'll take a look at it in a little bit.

In order to keep job history intact we'll rename the job via the jenkins UI

hidmic commented 5 years ago

Thanks @sloretz ! Merging.