extend rational why support for a single CMake context was dropped#116
Conversation
wjwwood
left a comment
There was a problem hiding this comment.
The only thing I would add is that if you add target level dependencies across packages then you need to make that conditional, otherwise you code may not compile with an isolated build (since the target won't exist in the second package anymore). With that the non-isolated tool now only works with packages that correctly handle intra-package target dependencies and all-catkin workspaces (no plain cmake and no pure python or other things).
|
The catkin variable If you think strongly about it please add a sentence to clarify this. |
|
My point is that people have to know to use that or their packages will likely fail at some point with the non-isolated tool due to a race condition. So that puts pressure to support the non-isolated case on package maintainers who may not use that tool. I don't feel strongly about it, and I agree it's probably too detailed to be mentioned in that article without a lot of supporting material (so people can understand it without a lot of context). |
articles/100_ament.md
Outdated
| While this significantly speeds up the process it has several drawbacks. | ||
| Since all packages within a workspace share the same CMake context all targets are within the same namespace and must therefore be unique across all packages. | ||
| The same applies to global variables, functions, macros, tests, etc. | ||
| Furthermore a package might need to declare target level dependencies to targets in other packages to avoid CMake targets to be parallelized which need to be sequential. |
There was a problem hiding this comment.
@wjwwood Maybe appending the following will clarify enough without adding to much additional information about how it works?
which need to be sequential (but only when being built in the same CMake context).
There was a problem hiding this comment.
Yeah, and if I can suggest a rewording, I think this might be clearer:
... declare target level dependencies to targets in other packages to avoid CMake targets being parallelized when they ought to be sequential (but only when being built in the same CMake context).
c55ea4f to
2184a75
Compare
Elaborate why support for a single CMake context was dropped from ament based on this comment: #115 (comment)