Metadata Files and Associated Verbs

[jbohren]

Overview

_See documentation for the changes pending here: http://jbohren-ct.readthedocs.org/en/metadata-file/index.html_

This PR (wip) covers a multitude of usability improvements to catkin_tools. This is in part motivated by features which used to be supported by rosbuild such as context-aware building and partial cleaning of the workspace.

The key architectural feature of this patch is the creation of a metadata directory called .catkin_tools in the root of a catkin workspace. This directory is not only used by the verbs to determine the location of the root of the workspace, but also it is used to store persistent information between builds. The API for interacting with this information is presented below.

The information stored in the .catkin_tools directory is contained in independent "configuration profiles" between which a user can switch without having to create multiple workspaces or do his or her own bookkeeping of command-line arguments.

Another focus of this patch is the organization and clarification of the command-line interface to catkin_tools verbs. This involves the addition of argument groups and the re-organization of arguments between verbs. For example, several of the "workspace configuration" arguments like the specification of source, build, and devel spaces have been moved from catkin build to catkin config. This simplifies the parameters to catkin build and makes it easier for newer users to figure out the important command-line options.

This patch also adds several new verbs which take advantage of this metadata utility.

Example Workflow

This PR currently enables an initialization and build workflow like so (try it yourself):

$ mkdir -p ~/my_ws/src
$ cd ~/my_ws
$ catkin config # declare this a catkin workspace with a default configuration
$ ls -a
.  ..  .catkin_tools src
$ cd src
$ git clone https://github.com/ros/catkin.git
$ git clone https://github.com/ros/rospack.git
$ git clone https://github.com/ros/cmake_modules
$ git clone https://github.com/ros/ros.git
$ pwd
~/my_ws/src
$ catkin build # call `build` from anywhere in the workspace 
<building everything...>
$ catkin clean --build --devel # clean products without dangerous `rm -rf`
$ cd rospack
$ catkin build --this # context-aware building based on working directory
<building rospack and its dependencies...>

Some cool context-aware aliases that this PR curently enables:

these: build --this              # Build this package and its dependencies
this: build --this --no-deps     # Build only this package
those: build --start-with --this # Build this package and all those which depend on it

Related Issues

Once complete, this PR should address the needs of the following issues:

• (build: Add marker file to workspace root #27) build: Add marker file to workspace root
• Adds the marker file.
• (new verb proposal: catkin info #67) new verb proposal: catkin info
• Creates the verb which dumps the marker file info along with other information.
• (build: allow execution from anywhere under the root of the workspace #10) build: allow execution from anywhere under the root of the workspace
• Uses the marker file to find the root of the workspace, and then build from there.
• (build: Add "--clear-cmake-cache" argument #68) *build: Add "--clear-cmake-cache" argument`
  • Adds a clean verb.
• (build: prevent mixing catkin_make and catkin build on same build folder #11) *build: prevent mixing catkin_make and catkin build on same build folder`
  • Reports an error if someone tries calling catkin build with a buildspace created by catkin_make
• (execution: move common functionality out of catkin_build and into a neutral module #2) move common functionality out of catkin_build and into a neutral module
  • Moves most of the common functionality into the catkin_tools module

IRC Discussions

• #ros 2014-07-21
• #ros 2014-07-29

TODO(NE)

• PEP8 silliness (use https://pypi.python.org/pypi/autopep8/)
• document all new features in RST documentation
• enable building from anywhere in the source tree
• add init verb to mark a directory as a catkin_tools workspace
• add clean verb which deletes buid/, devel/ and re-initializes .catkin_tools.yml
• add --cmake-cache-only argument to catkin clean
• add info verb to dump workspace information
• make workspace arguments in new verbs non-positional
• place metadata in individual profile directories, with a config.yml file under catkin_tools which lists the default profile
• create profile or configure verb or something similar for managing profiles
• separate "configuration-like" arguments out of build verb
• add configure verb to set build configuration without actually building
• enable context-aware building of a package (only that package, starting with that package, or that package and deps)
• add --this argument to build to build the package defined by the currently-enclosing directory
• clean up refactor of Context class (includes separating out verb-independent functionality of common.py, color.py and run_unix.py)
• require config/init call to use catkin build with stored configuration
• automatically generate .catkin_build marker if build config or init are called
• don't store cmake/make args (for now)
• store cmake/make args via catkin config
• unify context configuration
• make build (config) options only apply tmporarily
• replace all user-facing description of "metadata" as "profiles"
• sort profiles alphabetically in catkin -h
• remove catkin info verb (duplicate behavior with catkin config) this will be resurrected later in time
• remove catkin init verb (duplicate behavior with catkin config) -- replace with alias to config --init leave in catkin init
• have catkin config --init create directories like source space don't add directory-creating functionalities, maybe at some point later add acatkin mkspaces for making the source space dir
• add checks to detect incompatible build configs like merged vs unmerged spaces, or install vs noinstall
• add checks to detect if the user has used catkin_make or catkin_make_isolated previously in a workspace
• verify that --workspace and --profile arguments work with all verbs
• make catkin clean display help if run with no arguments
• fix --start-with --this so that you can run catkin build --this --start-with foo_pkg
• document python API functions & classes
• create a consistent appearance (colors/formatting/spacing) for all verbs
  • build appearance
  • config appearane
  • clean appearance
  • profile appearance
  • init appearance
• add helpful output to catkin init (like initialized empty catkin workspace in...) and then To build your workspace blah blah...)
• simplify CMAKE_PREFIX_PATH persistence in context
• Clarify cm vs cmi vs ct: Metadata Files and Associated Verbs #80 (diff)
• Fix DESTDIR documentation (give reference to spec): Metadata Files and Associated Verbs #80 (diff)

Other Ideas

• add a catkin build --from-paths DIR argument for building all packages in a directory
• store a list of the last set of packages which were built. if they've changed or their paths have changed since the last run, prompt the user to re-source the setup file in the develspace (or just always put this line at the end of build)

Configuration Profile (Metadata) Details

• Metadata is stored in a .catkin_tools directory
• This directory contains the following:
  • A simple instructive README file explaining what the directory is
  • A profiles.yml file describing the active profile (if different from default)
  • One or more directories containing configuration values, where each directory corresponds to a "configuration profile"
• Each configuration profile directory contains individual yaml files for verb-sepcific metadata. I.e. catkin build creates/updates the file .catkin_tools/<PROFILE-NAME>/build.yml
• When initializing the metadata directory (either automatically when calling a verb which writes to it, or explicitly using catkin init), the metadata module checks to see if that directory is contained within another catkin_tools-based workspace.

See https://github.com/jbohren-forks/catkin_tools/blob/metadata-file/catkin_tools/metadata.py for metadata API documentation.

[wjwwood]

This looks awesome @jbohren, I appreciate you taking the initiative to work on this. I have been wanting to do it for a while, but I am currently caught up in more low level refactors.

I was thinking about the metadata file and the workflow, and it may be that something like the config based verb's, which is similar to how make works with Makefile's and cmake works with CMakeLists.txt files, would work well here. @tkruse mentioned this, along with some other approaches before: #7 (comment)

At the time, I was focused on implementing a replacement for cmi, but I don't think it is unreasonable to redesign the workflow now that we have an working tool. I could imagine something more like this:

• Call catkin config_workspace which generates the workspace root file
  • This verb takes the normal options like --build-space, --install-space, etc.. and assigns them in the root file
• Later you call catkin build which has no options, like --build-space, but rather takes all configuration from the workspace file

I'm not 100% convinced of the common convenience of this workflow, but maybe some smoothing of edges around it could make it convenient, like having catkin build implicitly call catkin config_workspace when being invoked with options or for the first time on a workspace.

[jbohren]

At the time, I was focused on implementing a replacement for cmi, but I don't think it is unreasonable to redesign the workflow now that we have an working tool. I could imagine something more like this:

• Call catkin config_workspace which generates the workspace root file
  • This verb takes the normal options like --build-space, --install-space, etc.. and assigns them in the root file
• Later you call catkin build which has no options, like --build-space, but rather takes all configuration from the workspace file

Yeah, I was also thinking something similar. As it currently stands, the following are true:

• Calling catkin init [WORKSPACE] will generate a .catkin_tools.yml file in the given path or in the current working directory. This file will not contain any verb-specific information. In order to keep things clear, I'm trying to have it so each verb only modifies the section of the .catkin_tools.yml file which it owns.
  • To that end, catkin build --configure-only could be an option if someone wanted to generate the file with the configuration but not actually build.
• After calling catkin init you can call catkin build from anywhere in the contained file tree. This will then populate the .catkin_tools.yml file with information related to the build verb.

An alternative to using a single yaml file is to use a .catkin_tools directory with various configuration files in it. I think I'm going to do this just so that this thing can grow in the future.

[wjwwood]

An alternative to using a single yaml file is to use a .catkin_tools directory with various configuration files in it. I think I'm going to do this just so that this thing can grow in the future.

I agree that having a folder with multiple files is better.

[jbohren]

I agree that having a folder with multiple files is better.

Done in f4696d2.

[jbohren]

With the new .catkin_tools metadata directory:

• Metadata is stored in a .catkin_tools directory with a simple README file
• Individual yaml files are used for verb-sepcific metadata. I.e. catkin build creates/updates the file .catkin_tools/build.yml
• When initializing the metadata directory, the metadata module checks to see if that directory is contained within another catkin_tools-based workspace.

[wjwwood]

This will require a rebase.

[jbohren]

This will require a rebase.

I figured as much.

[jbohren]

Now featuring catkin clean @jack-oquin @NikolausDemmel

[NikolausDemmel]

I get the following with this PR:

$ catkin i -DCMAKE_BUILD_TYPE=Release
==> Expanding alias 'i' from 'catkin i -DCMAKE_BUILD_TYPE=Release' to 'catkin install -DCMAKE_BUILD_TYPE=Release'
==> Expanding alias 'install' from 'catkin install -DCMAKE_BUILD_TYPE=Release' to 'catkin build --install -DCMAKE_BUILD_TYPE=Release'
Traceback (most recent call last):
  File "/usr/local/bin/catkin", line 9, in <module>
    load_entry_point('catkin-tools==0.0.0', 'console_scripts', 'catkin')()
  File "/Users/demmeln/work/catkin_tools/catkin_tools/commands/catkin.py", line 192, in main
    sys.exit(args.main(args) or 0)
  File "/Users/demmeln/work/catkin_tools/catkin_tools/verbs/catkin_build/cli.py", line 159, in main
    build_metadata = metadata.get_metadata(marked_workspace, 'build')
  File "/Users/demmeln/work/catkin_tools/catkin_tools/metadata.py", line 126, in get_metadata
    (metadata_dir, metadata_file_path) = get_paths(workspace_path, verb)
  File "/Users/demmeln/work/catkin_tools/catkin_tools/metadata.py", line 48, in get_paths
    metadata_dir = os.path.join(workspace_path, METADATA_DIR_NAME)
  File "/usr/local/Cellar/python/2.7.7_2/Frameworks/Python.framework/Versions/2.7/lib/python2.7/posixpath.py", line 77, in join
    elif path == '' or path.endswith('/'):
AttributeError: 'NoneType' object has no attribute 'endswith'

[jbohren]

I get the following with this PR:

Sorry about that, should be fine again now if you call build without having first called catkin init.

[NikolausDemmel]

Yeah I just noticed that init was necessary.

I know this is still wip, but here a few initial comments:

• the install space is not saved in the file; it is not cleaned (it should be an option maybe)
• I would prefer if clean and init to not take the workspace as a positional argument, but use the same optional argument as the build verb
• clean could maybe take packages as positional arguments to only clean specified packages (plus deps?). Might only work for isolated devel?
• some inconsistency with the space-defining arguments. When I supply e.g. --devel-space, the value overwrites anything in the marker file. Supplying --path-suffix does not change the spaces accordningly (except for install).
• arguments like -D... are not yet saved in the build.yml file

But great stuff overall, thanks for working on this!

[jbohren]

But great stuff overall, thanks for working on this!

Thanks for the feedback!

[davetcoleman]

This PR has many +1's for me! There are several features that I already use via aliases, so I'll be happy to switch to a more standard version. Thanks @jbohren

[jbohren]

@wjwwood One thing that would be nice is to not have to specify the various keys which get stored in metadata explicitly. There's a lot of logic in catkin_build.Context which isn't really captured in the arguments (like how --space-suffix is ignored if you pass explicit build/devel/install spaces). Do you have any thoughts on how to merge the Context concept with metadata? Maybe a verb's description could also include a list of argument keys which get stored?

[NikolausDemmel]

One more point that just popped to mind: We have discussed multiple named configs in one workspace before (Release, Debug, x-compiling, ...). While for most users this is maybe not so relevant and implementing it is a low priority, if we introduce this config directory now, we might as well lay it out in a way with named "profiles" in mind. I'm thinking something like this layout:

.catkin_tools/default ---> ./release
.catkin_tools/release/build.yml
.catkin_tools/debug/build.yml

I.e. default symlinks to the currently "active" profile. The verbs might eventually take an optional argument to specify a named configuration other than the default (in this case release or debug), and there could be a way to switch the default (change the symlink).

As long as we named profiles are not implemented the init command could create such a folder layout, which might allow to seemlessly transition to named profiles later on:

.catkin_tools/default ---> ./profile1
.catkin_tools/profile1/build.yml

(I'm not attached at all to how this is layed out exactly)

[NikolausDemmel]

(caveat: this PR already contains a lot of useful stuff. I don't want to extend its scope too much. We might rather get it merged and then open new issues to track possible improvements like I suggest in the following).

If I understand correctly, you currently propose to have verb-specific metadata files in the .catkin_tools folder. IMHO things like the source/build/result spaces should not be set by individual verbs separately, but rather be shared by all verbs for that workspace(-profile). It might still make sense to allow individual verbs to create their own files with specific info, but there should be some place for common metadata.

I guess implementing this goes together with allowing multiple verbs to share part of their CLI argument definitions and parsing (i.e. all verbs that operate on source/builld/result spaces, all verbs that take packages are arguments, all verbs that take the --workspace argument), without duplicating them in code.

[jbohren]

I.e. default symlinks to the currently "active" profile.

I like this whole concept a lot, even so much as to rename "metadata" as "profiles".

IMHO things like the source/build/result spaces should not be set by individual verbs separately, but rather be shared by all verbs for that workspace(-profile).

As it currently stands, there's nothing preventing code from reading or writing an arbitrary yaml file in the metadata directory. The way I see it, verbs will likely read each-others metadata, but we should encourage verbs to only write to their own file.

[wjwwood]

I wouldn't make the --space-suffix option a configuration in the .catkin_tools/build.yaml. I would make that file very explicit, and leave fancier options to the tools to expand.

[jbohren]

I wouldn't make the --space-suffix option a configuration in the .catkin_tools/build.yaml. I would make that file very explicit, and leave fancier options to the tools to expand.

I agree. I didn't mean to suggest that. I was just saying that right now the --space-suffix argument is ignored if you've built before. This happens because both the --space-suffix flag and the previous spaces get passed into the catkin_build.Context class, and that class ignores the --space-suffix if the spaces are specified.

It's also onerous to have to convert the spaces between relative (for config file) and absolute (for catkin_build.Context) and back again.

[wjwwood]

This is why I originally proposed separate commands for configure and build, because if you look at it as a configuration command then --space-suffix is sort of the equivalent to specifying all the spaces explicitly and therefore would not be overridden by the existing values in the config. The only time it should be overridden is when there are more specific command line arguments.

[jbohren]

This is why I originally proposed separate commands for configure and build, because if you look at it as a configuration command then --space-suffix is sort of the equivalent to specifying all the spaces explicitly and therefore would not be overridden by the existing values in the config. The only time it should be overridden is when there are more specific command line arguments.

Yeah, I think this is still a good idea. Looking at the arguments that you can pass to catkin build they're all either configuration-type arguments or behavior-type arguments (see below). Do you think all of the configure-type arguments should be in a different verb, or is it just --space-suffix? I'll admit it would probably clean things up a lot if we moved the configure-type args into a configure verb. Most of these won't be called very often, usually just when setting up a workspace. Previously they needed to be in the build command because there was nowhere to preserve this information.

Configure:

--source
--build
--devel
--isolate-devel
--install-space
--isolate-install
--space-suffix
--extend
--cmake-args
--make-args
--catkin-make-args

Behavior:

packages
--no-deps
--start-with
--workspace
--install
--parallel-jobs
--force-cmake
--no-install-lock
--force-color
--verbose
--interleave-output
--no-status
--list-only

[wjwwood]

Yeah, and it works out that the configuration options are mostly in the Context object, so I sort of organically ended up with that separation.

The two that don't line up with your list above are install and workspace. In my opinion install is weird because it does affect behavior, but it also describes the setup of the workspace because it is not the same to install (chains off of install space) and not install (chains off devel space), since packages chain off different directories in one versus the other.

The workspace variable sort of straddles the line since it affects the build tool by determining where it looks for configuration files and it affects the config tool by changing the defaults for spaces and where the config files are at.

[jbohren]

I think my unit tests have uncovered a huge bug when using catkin_tools with Python 3 where catkin build's subprocesses will always interperet the first line of stdout as a returncode, which is hardly ever 0, and will kill the subprocess and report failure. More later.

[jbohren]

Woot. @wjwwood Tests passing in https://travis-ci.org/catkin/catkin_tools/builds/36103845

[wjwwood]

Nice.

[wjwwood]

Great work @jbohren, can't thank you enough for investing the time in this 😄

[jbohren]

Merge might have been a bit premature, the python3 compatibility is there, but now the verbose debug isn't displaying newlines correctly. I think you might be able to just use the "revert" button, otherwise, I can make a new PR.

[wjwwood]

Lets make a new pull request/issue.

[wjwwood]

Also, unless you are actively using it with Python3 you might just wait on that. I plan on switching over to the subprocess stuff I have in osrf_pycommon, and I went to some effort to make sure that works on Windows and Python3 and everything other corner of the matrix.

[jbohren]

Opened #93 which fixes it. Just needed to find the portable way to do it.

[jbohren]

Also as an aside, writing 2-3 portable python code is a huge pain.

[jbohren]

@wjwwood Is there anything preventing the readthedocs job from running?

[wjwwood]

I just have to poke it, doing that now.

Actually it is already done: http://catkin-tools.readthedocs.org/en/latest/

[Levi-Armstrong]

This MR suggests that an option --cmake-cache-only was added to catkin clean but does not seem to be available. Was this not added for some reason?