build: Generate .catkin file when using merged develspace and catkin in "parallel-safe" mode

[jbohren]

If catkin gets a "parallel-safe" mode, as described in ros/catkin/issues/675, then we could use that to generate a .catkin develspace marker file from the information generated by the process described in ros/catkin/issues/674.

[wjwwood]

Thanks for keeping track of this issue and for looking into it originally. I really appreciate the help.

[jonbinney]

This is still a problem, and in my opion is a pretty big problem with catkin tools right now. Sometimes after running catkin build and sourcing devel/setup.bash, some packages are not in my ROS_PACKAGE_PATH. This has been happening to me and my coworkers for months, and I only yesterday figured out that it was due to the concurrency problems in catkin tools.

Here's a test script that will generate a workspace with a bunch of packages and try to elicit the problem: https://gist.github.com/jonbinney/0a09e57db3676e0bd29f
That scripts compiles a workspace with 20 packages 100 times. On my computer, it caused a corrupted devel/.catkin (and thus a corrupted ROS_PACKAGE_PATH) 12/100 times.

Note that while having lots of packages and having a carefully crafted cmakelists.txt in the test script causes the bug to happen more often, the bug exists for any workspace. In practice I run into this bug several times a week during normal ROS development.

[wjwwood]

@jonbinney did you ever run this with the --isolated-devel option? My guess is that this issue would not be present in that case. But obviously the isolated devel space is not very nice to use.

These are the solutions as I see them:

• Isolated devel space with symlinked "merged" devel space
  • This is @jbohren's approach in [major] executor/clean: Adding per-package cleaning, linked develspaces, and a new execution pipeline #196
  • It does an isolated devel space for every package, but symlinks unique files, and merges non-unique ones (like .catkin), see: [major] executor/clean: Adding per-package cleaning, linked develspaces, and a new execution pipeline #196 (the description about linked devel space)
• Change catkin to use something like ament's package resource
  • We solve this problem more generically in ament by avoiding packages from installing files which may collide (avoiding situations like .catkin all together)
  • This would be hard since we'd need to get it in catkin and then release and make sure no other existing tools are affected adversely.
• Have catkin_tools look at the .catkin file before building packages, build packages, then update the .catkin file to make sure it is correct.
  • This is similar to what @jonbinney's test script is doing, but rather than just asserting that it is correct or not, it would also just update it if needed.

The first option is tied up in the #196 pr, which is going slowly, and the second option seems like a lot of work. I'd say that the best short term solution would be to implement the work around of option 3 and then replace it with option 1 when #196 gets merged. I don't have time to implement the work around, but maybe a pioneering individual would be willing to make the patch.

[jonbinney]

@wjwwood I was testing catkin build with no options. --isolated-devel doesn't seem to be an option for catkin build (at least according to the help text)

[wjwwood]

@jonbinney You have to set it with catkin config --isolate-devel before doing catkin build.

[jonbinney]

Running test now with --isolate-devel. So far so good.

[jonbinney]

Confirmed, works 100 times out of 100 with --isolate-devel!

[jonbinney]

Another option would be to make --isolate-devel the default for now. That would make things slow, but if people need fast they should be using catkin_make directly right now.

[wjwwood]

Aside from being slow --isolated-devel is inconvenient to use (in my opinion). Yet another option is to make --install the default, and users can use the install space. I believe that would also not have the problem (certainly not when using --isolated-devel but also perhaps with out --isolated-devel too).

[jack-oquin]

An added advantage of --install might be the fact that the ament design uses install space and not devel.

[jonbinney]

I'm fine with install, as long as the devel space is gone. My biggest concern is that right now most people run catkin build; . devel/setup.bash, which results in bad builds sometimes. Whatever change is made should make sure that either those commands always result in a valid setup, or that users are forced to change their behavior.

[NikolausDemmel]

-1 for making --install the default.

The devel space has many advandtages over install for developers (in particular for Python), and we are talking about mostly developer users for now, I presume. I would guess that many defs have come to rely on them, even if they might not be aware of the mechanics. I have not come across a signle instance of someone actually using the install space for development (for anything, really, except maybe for from-source installs of a base ROS system) in daily work. Moreover, I frequently come across (unreleased) packages that are not properly configured for installation and thus only work with the devel space as-is. Thus making --install the default might take quite a few by surprise.

I wounder how much effort @wjwwood's third suggestion would really be. I haven't dug into it, but I guess all the code for manipulating the .catkin file has to be there already. Patching it up post-build should be pretty straight-forward. What I'm less sure about is what to do when the build fails or is aborted, but maybe "nothing" is enough for this workaround. I.e. the workaround only takes effect when the build succeeds.

An added advantage of --install might be the fact that the ament design uses install space and not devel.

This makes me wounder how easy it would be to extend catkin / catkin_tools to support linked install spaces like ament, but that is a different story.

[jbohren]

@NikolausDemmel My PR (#196) already implements a linked devel space, so creating a linked install space wouldn't be a big leap.

[wjwwood]

-1 for making --install the default.

You're right, without the symlink install option it would be inconvenient to use.

@NikolausDemmel My PR (#196) already implements a linked devel space, so creating a linked install space wouldn't be a big leap.

I'm not sure about this, since what I think you're talking about it is doing an isolated install and then do a symlink to a common folder, but that doesn't make it so that you can edit Python files without re-installing. What I think @NikolausDemmel is talking about is how ament cmake packages can replace the cmake install step with symlinking, so the files it installs are just symlinks to either the build folder or the source folder.

[jbohren]

@wjwwood Can we close this now that master is populating the .catkin file on it's own?

[wjwwood]

Sure.