Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

meyerj
Copy link
Contributor

@meyerj meyerj commented Dec 9, 2021

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.

It is still possible to select the workspace explicitly by passing the
workspace hint (-w, --workspace), but the argument needs to point to
the workspace root and not to a directory contained within.
@timonegk
Copy link
Member

timonegk commented Dec 9, 2021

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants