-
Notifications
You must be signed in to change notification settings - Fork 18
Workspace repo #717
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
base: main
Are you sure you want to change the base?
Workspace repo #717
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks fine.
But I don't quite understand the intention behind making the repository itself the colcon workspace.
I thought the split is quite useful for the following reasons:
- Code is independent of build/generated files, which makes searching easier and doesn't "dirty" the repository, potentially making it more cluttered
- The split allows to easily add additional colcon packages to be built, by just adding them to the
COLCON_WS/srcwithout dirtying the repo (e.g. I used this when trying out ros-rust) - The directory structure is more independent as you can just expect a
colcon_wsworkspace in scripts, but the actual code can be anywhere as it's just a symlink
| # Set the default colcon workspace | ||
| export COLCON_WS="\$HOME/colcon_ws" | ||
| export COLCON_WS="\$HOME/git/bitbots/bitbots_main" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is inconsistent with COLCON_WS=$HOME/bitbots_main in setup.sh.
And you would also need to change the online command accordingly if you now expect a different directory structure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not finish the setup.sh yet. The path ~/git/bitbots/bitbots_main is the recommended one in the documentation, so I used it as the replacement, but I am open to further ideas how to handle this better.
In the dev container I didn't use a path like ~/git/... and just had the repo at the default location to simplify things.
|
Thanks for having a look :)
I agree that having the
We already do this using the lib folder inside the repo. All third party packages are already placed there. And for non Bit-Bots related things, like e.g. a fix for ros2 itself I always create a second workspace to isolate things. There are also many cases where having the colcon workspace as a repo is a bad idea imo.. E.g. at tams there are many repos for the different robots, sensors etc. and if you do an experiment you clone them inside your colcon workspace. But with the mono repo I think it is okay to couple the Bit-Bots workspace together with our code.
What would be benefits of symlinked code? I know some people do this, but I do not see that many practical advantages. I would argue that it is a good thing that the directory structure is standardized by the repo, this way the scrips etc can expect a certain configuration without relying too much on global often not directly version controlled configuration. My ideal setup would be one where you clone the repo, build it and everything works, creating minimal friction. I have often seen especially new people struggle with where which command should run (make commands vs. colcon). Aliases helped a bit there, but they also implicitly change directories etc.. My intention was to unify this by having one top-level repo where everything can be performed. Also what started this idea was that the vscode ros extension now requires that the whole workspace is open in vs code, which requires that the preconfigured settings live in the colon ws, which is unlucky as we have them inside the repo. Yeah we could symlink, but that also increases the complexity. Having the settings at the workspace level also allows for a few other nice configs. Also this reduces additional setup like creating a workspace with the correct folder structure upon setup etc.. This is mostly automated right now, but it would be neat if we wouldn't need that. Non Ros/package related scripts can live in their own scripts directory that is not part of the colon environment that way reducing potential friction points, as we had issues in the past where colcon tried to build e.g. an unrelated setup.py. This configuration also allows us to have a command runner that executes all relevant commands for typical usage cases from one place. This removes issues where colcon is called from the wrong directory, people don't know what commands exist, it brings the documentation of the common commands close to the code so they are less likely outdated and it allows for shorter commands. Parts of this can also be achieved using a alias, but this seems like a better solution. Many projects have a make file to setup, clean, .... The main idea was to reduce friction along the most taken usage paths, especially for beginners. Removing the notation of an explicit workspace might help there and allow for less moving parts overall. But I am also a bit cautious of a migration like this, as it might break existing bashrc configurations etc.. |
|
Wow, thanks for the detailed explanation. Seeing as the Vscode ROS extension requires the workspace, I think this makes a lot of sense.
I think you misunderstood what I meant. I was just talking about the current status quo where e.g. But yeah I agree as we just use the monorepo normally this change makes a lot of sense 👍 |
|
Okay, what' left to do? We have to rebase this and adapt some docs and tools, right? Then, when merging, write some instructions on how to work with the new setup. |
Yeah maybe update some docs / test them. I already adjusted most of the docs. There are some open items regarding default location of the repo and how we deal with the COLCON_WS env variable. We could also consider using direnv and move some of the zshrc stuff there. Also the robot_compile stript will be broken and we might want to test it on e.g. amy. |
Summary
We wanted to try a setup where the bitbots_main repo is the colcon workspace itself. This would make a few workspace related settings easier, fix the vscode ROS extention, clean up the repo top level and allow us to have all commands at the same level (managed now via
justinstead of make, as it e.g. supports positional args).Proposed changes
src/maketojustand add additional targets likejust buildorjust cleanChecklist
colcon build