catkin / catkin_tools

Command line tools for working with catkin
Apache License 2.0
163 stars 146 forks source link

Detect nested workspaces and use the outermost workspace by default #705

Open meyerj opened 2 years ago

meyerj commented 2 years ago

At the moment catkin_tools refuses to create nested workspaces with the catkin init verb and always uses the innermost workspace where the .catkin_tools marker directory is found, starting from the current working directory or the hint passed via the --workspace or -w argument.

This pull request is more meant as a question rather than a request to merge. Would a patch like that, or parts of it, be accepted for upstream, eventually with some polishing? Or does anyone has better ideas/best practices on how to solve the following use case?

Use case:

We sometimes commit the .catkin_tools/profiles/*/config.yaml files with predefined profiles to a Git repository, which is very convenient to share build profiles and commonly used cmake flags within the team. All the other files in .catkin_tools/ are ignored by .gitignore entries. Some of those repositories can be included in others as Git submodules, that then symlink to all or a subset of the catkin packages from their respective source-space and typically have their own committed .catkin_tools directory.

Without this patch it happens that the catkin build and other verbs pick up the innermost workspace depending on the curent working directory, which then leads to unexpected results when sourcing the devel- or install-space of the outermost workspace and the update has not been applied.

I am aware that nesting workspaces like that may not be the smartest idea, and the more standard approach is to strictly separate source repositories and those "workspace" repositories that combine multiple source repositories, documentation, scripts, Dockerfiles etc. for a specific project. On the other hand you then always need an extra repository, for example to run CI for each source repository without replicating the build configuration.

Proposed solution:

Instead of the innermost, always find the outermost workspace in find_enclosing_workspace() and do not refuse to create nested inner workspaces if asked explicitly with --workspace/-w. The breaking change is that --workspace/-w must always point to exactly the workspace root, not to any directory within, because otherwise a call like

catkin init -w /path/to/workspace/external/nested

would be ambiguous again if /path/to/workspace already has a .catkin_tools marker directory. Eventually it would make sense to print a warning in case a workspace is initialized within another workspace, that then would not be used by default by catkin.

timonegk commented 2 years ago

do not refuse to create nested inner workspaces if asked explicitly with --workspace/-w --workspace/-w must always point to exactly the workspace root

Both of this is already the case, so this part would not be a breaking change.

The only breaking change would be to always use the outermost workspace. I am a bit afraid of changing that because I could imagine some users depending on it in a similar configuration you use. Maybe another solution would be to either put the folder as .catkin_tools.example in the repository and add a symlink to .catkin_tools where necessary or maybe to add support for a file similar to CATKIN_IGNORE that instructs catkin_tools to ignore a certain configuration directory. Anyway, we can definitely add a warning when the user is calling catkin inside of a nested workspace.