catkin / catkin_tools

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

build: allow execution from anywhere under the root of the workspace #10

Closed wjwwood closed 10 years ago

wjwwood commented 10 years ago

The idea is to allow users to execute the build verb anywhere within a workspace, similar to how git commands can be called anywhere within a git repository.

It is related to https://github.com/ros/catkin/pull/603 and should replace https://github.com/catkin/catkin_tools/pull/9

jbohren commented 10 years ago

If build is called from within a specific package, could there be shortcuts (like special verbs) for the following?

  1. build everything in the workspace (catkin build -w ../../../)
  2. build this package and it's deps (catkin build -w ../../../ current_package)
  3. build this package and no deps (catkin build -w ../../../ current_package --no-deps)
jbohren commented 10 years ago

The above could be implemented via the verb aliases just proposed in https://github.com/catkin/catkin_tools/issues/8#

wjwwood commented 10 years ago

Can you expand on the scenarios for you special verbs you described above?

It seems like you mean context aware verbs? For example:

$ caktin build
...
baz built
bar fails
...
$ cd src/bar
$ catkin buildthis
==> changing to workspace root: '../../'
==> running command: 'catkin build bar -p1 -j1 --verbose'
...
baz noop
bar fails again
...
$ cd include/bar
$ vim bar.h
# apply a fix
$ catkin buildthis
==> changing to workspace root: '../../../'
==> running command: 'catkin build bar -p1 -j1 --verbose'
$ cd ../../../
$ catkin buildthis
==> Error: not in a package within a workspace
jbohren commented 10 years ago

It seems like you mean context aware verbs?

Yes, exactly that. The build verb could take a --this argument (another separate feature which could be implemented after this feature) and If we have verb aliases, buildthis could call build --this and buildonlythis could call build --this --no-deps.

NikolausDemmel commented 10 years ago

Could this be extended to allow to build from anywhere (also outside the workspace), assuming the right devel/setup.*sh is sourced. I imagine a shortcut of something like roscd [pkg] followed by catkin build. Of course this won't work for package sourced from install space.

Otoh it might be confusing as default behavour. I'm pretty sure this has come up before for in discussions around catkin_make, but I don't know if there were good reasons against it (@tkruse ?).

jbohren commented 10 years ago

Could this be extended to allow to build from anywhere (also outside the workspace), assuming the right devel/setup.*sh is sourced. I imagine a shortcut of something like roscd [pkg] followed bycatkin build. Of course this won't work for package sourced from install space.

This would also be confusing if you had multiple workspaces.

NikolausDemmel commented 10 years ago

True to some extend, but isn't it already problematic to build a workspace while having anonther one sourced (when I don't want to overlay it)? In any case some heuristic disambiguation would be needed.

jbohren commented 10 years ago

True to some extend, but isn't it already problematic to build a workspace while having anonther one sourced (when I don't want to overlay it)? In any case some heuristic disambiguation would be needed.

Well if you don't want to overlay workspace B on workspace A, then you need to unsource workspace A before initializing workspace B. That's pretty well-defined behavior.

If you have multiple overlayed workspaces, then calling catkin build from somewhere else in the filesystem would have to either build the topmost workspace, or build all of the workspaces. It would be really cool if catkin build could do both of these things.

To that end, it seems like there are a few separate features being discussed here:

All of this, of course, requires something like https://github.com/ros/catkin/pull/603

NikolausDemmel commented 10 years ago

I fully agree with your analysis. I was thinking the most important use case ot be rebuilding a specific package quickly from anywhere with catkin build foopkg. Here the workspace for the build command to be executed in is more clearly defined as the (top-most) one where this package is found in comparison to a naked catkin build, but even there the simplest heuristic would be to rebuild the topmost workspace. Question what happens when you are in a workspace that is not the topmost one. Does it build the current, or the topmost workspace. I think the ambiguity could be dealt with mostly by having the 'find workspace from environment' feature only when supplying an additional flag, rather than by default.

The idea of building multiple workspaces in a chain of overlays at the same time is interesting and might work well if all the workspaces use the same conventions for build and devel folders, which for many users may be the case. Relevant for this is also the discussion about 'true overlaying' from a while back: https://groups.google.com/forum/#!topic/ros-sig-buildsystem/QHbpQ94UWH4

Having said all that, I think the most important feature of all of these the "context-aware building", which is what this issue is mainly about. Let's see if/when someone has the time to work on this and discuss it further at that point.

tkruse commented 10 years ago

An env var pointing to the workspace was discussed and briefly mentioned here: http://ros-users.122217.n3.nabble.com/catkin-make-convenience-improvements-td4019492.html

Besides switching workspace problems, a main issue is that when you source devel/setup.sh, you do not create a build environment, but a run environment in the shell. Subtle broken build behavior can actually occur if you invoke catkin_make for a package of a workspace where you have sourced the devel/setup.sh, if I am not mistaken, because a package can build against an older version of itself (since it's old build result resides among it's other dependencies. Another problem with this is that you can build against dependencies that you did not declare as dependencies, as long as they happen to be build before your package by coincidence.)

Now I would guess that most users do this anyway, they source devel/setup.sh, then continue to use the same shell to build their packages. When some conflict happens to beginners, I don't even want to know how much time they lose on trying to figure out what went wrong.

Unless I am mistaken, catkin_make_isolated and the parallel version do not suffer from those problems, as each package has a sane minimal build environment of it's own. If one lucky day the default catkin_make behavior was dropped in favor of a more rosbuild-like approach for setting up the build environment for each package, we could discuss again invoking a make command from anywhere after sourcing a setup.sh.

tfoote commented 10 years ago

Rebuilding chained workspaces will also potentially break any workspaces which fork from your current chain. For example if I have the core of ROS installed from source and have two independent workspaces for package feature development, when I rebuild in one of the overlays the build has no knowledge of the second overlay and if it auto rebuilds the underlay the second workspace may be broken depending on the changes.

NikolausDemmel commented 10 years ago

@tkruse : I was not aware of this issue, and I seriously wounder how many catkin users are aware of this. It would be good to confirm the behaviour of cmi and catkin build (both with and without --merge-devel) here. @wjwwood, do you know?

Before you brought up env vars I thought that for building a specific package from anywhere something like rospack find pkg and then using https://github.com/ros/catkin/pull/603 and the catkin build pkg would be enough, but I guess the devel space might not be a child of the workspace after all, even though with default catkin_tools usage, it might cover 99.9% of actual use cases.

I consider catkin_tools a good candidate to completely replace catkin_make at least for a certain group of end users, so any restrictions that have come up for catkin_make in the past (for good reasons) might not apply to catkin_tools.

@tfoote : Your point is another good reason for not making build of chained workspaces default behaviour, but IMHO shouldn't stop this from being available as an opion enabled by a flag. One way to avoid the issue in any case is using the install space for underlays that are supposed to be robust in that respect (in my view the recursive build would never touch workspaces for which the install space is sourced).

wjwwood commented 10 years ago

Unless I am mistaken, catkin_make_isolated and the parallel version do not suffer from those problems, as each package has a sane minimal build environment of it's own.

catkin build builds the environment for each package by starting with the environment in which the command was run and then running source /path/to/setup.sh --extend for each of the package's direct build and run dependencies. This only applies to the non-install and the --install --isolate-install cases, so with only the --install option, each package simply takes the environment the command was run in and adds source /path/to/install/setup.sh --extend before building.

I'm not exactly sure what implications there are for sourcing a devel space and then running catkin build again.

My personal workflow is just to have one terminal which is in the root of the workspace and is for building the workspace, and then another workspace for sourcing, running, and navigating the workspace.

NikolausDemmel commented 10 years ago

[This becomes a bit off topic, not sure where this branch of the discussion should best continue] I find it very surprising that catkin is not taking care of this (i.e. cleaning the environment before build when the workspace's own devel/setup.sh is sourced). I seriously wounder how many people are aware of this source of apparently suble bugs, especially because the very first catkin tutorial http://wiki.ros.org/catkin/Tutorials/create_a_workspace concludes with "now source the devel/setup.*sh before continuing" and then links to the next tutorial telling you all the ways you can use catkin_make. If this is indeed a problem, catkin should be fixed to either take care of it, or at least warn loud and clear.

wjwwood commented 10 years ago

We would first need to identify the ways in which the system could fail or do something incorrect when this scenario comes up.

I am happy to consider warnings and checks for catkin to make, but I would point out that this is no different from installing a CMake project into a place on your paths and building it again.

tkruse commented 10 years ago

@NikolausDemmel : That quirk of bad catkin_make usage has not directly been observed to cause a lot of real trouble, I guess. I just think it was one of the reasons not to mix the runtime environment with the buildtime setup by setting an env variable in setup.sh. I don't say it is a good reason.

NikolausDemmel commented 10 years ago

@tkruse: Fair enough. It sounded that this has actual implications in practice more often than it maybe does. It seems though, that when there is a bug caused by this, it could be extremely subtle and hard to find. Something to keep in mind for sure, but not specific to this issue.

jbohren commented 10 years ago

This looks like it can be closed now that #80 has been merged.