colcon-core icon indicating copy to clipboard operation
colcon-core copied to clipboard

Consider not accessing run/exec dependencies during colcon build

Open clalancette opened this issue 3 months ago • 6 comments

I've recently been doing some work to investigate colcon behavior, and I found a somewhat non-intuitive result.

In particular, if pkg_a has an <exec_depend> on pkg_b, which has a <build_export_depend> on pkg_c, then building pkg_a accesses the environment for both pkg_b and pkg_c. But that seems unnecessary; since pkg_a only has an exec_depend, it shouldn't need to access either of those dependencies at build-time. You can see an example of this in action at https://github.com/clalancette/colcon_exec_ws .

After talking to @cottsay a bit about this, this is basically how colcon does it at present. All dependencies are collapsed into a build, run, or test dependency: https://github.com/colcon/colcon-ros/blob/65ac2ec65c5704aa9a4d9fffdd20de4389a45e1b/colcon_ros/package_identification/ros.py#L117-L122 . And all of these dependencies are accessed by colcon at present.

In the future we could consider splitting these out to be more fine-grained, which would fix this particular issue and possibly make builds a bit faster. But it would probably cause a lot of problems in the ecosystem with incorrect dependencies, so it would have to be rolled out rather slowly.

clalancette avatar Aug 25 '25 18:08 clalancette

I'd be a fan of this. We are trying to speed up our production runtime builds that only use exec_depend

Ryanf55 avatar Sep 17 '25 06:09 Ryanf55

FWIW, in the Nix builds at Clearpath we correctly propagated/handled build_export_depend and I don't recall having to patch much stuff, at least up to ros-desktop-full on the ROS 1 side.

mikepurvis avatar Sep 17 '25 20:09 mikepurvis

FWIW, in the Nix builds at Clearpath we correctly propagated/handled build_export_depend and I don't recall having to patch much stuff, at least up to ros-desktop-full on the ROS 1 side.

I'm not too worried about the ROS 2 core; I think it should be relatively good with respect to this, and anything that is a problem can be quickly fixed. But colcon is used to build thousands of packages, so I think this will cause pain to the rest of the ecosystem. If we are going to change this, I think we should hide it behind a feature flag.

clalancette avatar Sep 17 '25 23:09 clalancette

Last I checked, there were some problems in the rosidl pipeline that I didn't have sufficient context to untangle. Shipping this today would probably instantly break core builds on all ROS 2 distributions. Of course these are bugs that should be fixed, but we'd need a rollout plan.

Backwards compatibility with colcon extensions is probably something to consider too. There's probably places where we assume run is a dependency category, so if we suddenly split that into other categories, it's kind of a breaking change. Not intractable, but another "wrench" so-to-say.

cottsay avatar Sep 23 '25 19:09 cottsay

Can rosindex or some other tool give some visibility into how much build_export_depend is even being used? I would have guessed that in the overwhelming majority of cases, developers haven't really engaged with what the different dependency types mean and just roll with <depend> for everything.

mikepurvis avatar Sep 23 '25 21:09 mikepurvis

Can rosindex or some other tool give some visibility into how much build_export_depend is even being used? I would have guessed that in the overwhelming majority of cases, developers haven't really engaged with what the different dependency types mean and just roll with <depend> for everything.

I have a (slightly older) local checkout of most of the packages in rolling. There are 1407 package.xml files in that checkout. Of those, 79 have a <build_export_depend> tag. 811 have a <depend> tag. So yes, your intuition here is mostly correct.

clalancette avatar Sep 23 '25 23:09 clalancette