Package identification & build tasks for colcon-ros-cargo

[nnmm]

This includes cargo-ros-install, a wrapper around cargo build which installs files into the correct places for ROS 2 tools to find them.

[nnmm]

@esteve @mxgrey I cleaned this up – feel free to do a first review iteration.

[nnmm]

@esteve @mxgrey There is no rush, but do you know roughly when you'll be able to review this?

I know it's a lot of code. If I can do anything to make reviewing easier (more comments, video call, splitting it up, etc.), let me know.

[esteve]

@nnmm thank you so much for working on this!

One question, though, how does this compare to rust-lang/cargo#6100 ? It seems to me that --out-dir does a similar job.

I will test this later, but wanted to ask, does the Cargo plugin modify the source space of a package in any way? Or does everything happen in the build space?

[nnmm]

One question, though, how does this compare to rust-lang/cargo#6100 ? It seems to me that --out-dir does a similar job.

Yes, --out-dir does a very similar job as cargo-ros-install in that both put the binaries in the specified location. However, (1) --out-dir is unstable and (2) this is only a part of what cargo-ros-install does: It also creates package markers and copies sources.

I will test this later, but wanted to ask, does the Cargo plugin modify the source space of a package in any way? Or does everything happen in the build space?

It does not modify the source space of a package in any way. The colcon-ros-cargo plugin creates .cargo/config.toml but that's it.

[nnmm]

Here I followed the decision made by the people in the chat: To resolve to the source space by default. However, I wanted to point out a few things I realized in working on colcon-ros-cargo:

• We need to copy sources to the install directory anyway, not only to make messages work, but also for depending on another workspace, like mentioned yesterday. But the performance impact of that is totally negligible. Even if we're worried, copying could be done in parallel to building.
• I can empirically confirm that scanning the source space takes upwards of 1 second for a workspace like AutowareAuto.

So the cost matrix looks like this (x is the use case, y is the design decision):

                      ┌─────────────────────┬────────────────────────┐
                      │ build cargo         │ generate a config.toml │
                      │ packages with       │ and not use colcon     │
                      │ colcon (frequency:  │ afterwards (frequency: │
                      │ high)               │ low)                   │
┌─────────────────────┼─────────────────────┼────────────────────────┤
│ Searching workspace │ Needless workspace  │ No cost                │
│ plus install space  │ scan (1s+) or pass  │                        │
│ is the default      │ in --no-search-ws   │                        │
├─────────────────────┼─────────────────────┼────────────────────────┤
│ Searching only the  │ No cost             │ Need to pass in        │
│ install space is    │                     │ --search-ws            │
│ the default         │                     │                        │
└─────────────────────┴─────────────────────┴────────────────────────┘

Since generating a config.toml basically needs to be done only once, I think the left half should be weighted stronger. That's why I thought it would be better to switch the default around, so that it uses searches the install space by default.

[esteve]

If I'm understanding this correctly. I think it should only search the install space, like what happens with every other package in a colcon workspace, that's why I asked in the chat if the source space was accessed in any way a few days ago. The source spaces of other packages are never accessed with the other build types, only the install spaces.

[nnmm]

Yep, I think this was a bit unfortunate: The two of us discussed this once in the past before some of the other people joined, and agreed that dependencies should point to the install space. Later, the topic was brought up again and people voted in favor of dependencies pointing to the source space by default, and I couldn't reach you for comment.

I changed the default now to searching only the install space. It's not a big deal since depending on the source space is still possible with the flag.

I also noticed that the README got lost at some point and added it back.

[esteve]

No problem! That's what PR are for 🙂