Oops this pull request ended up getting both changes. If you're cool with both together I could close the other one? Or I'll close this one and open a new one with just the RuntimeError changes

This pull request changes the API which is not feasible. The function documents the way a circular dependency is returned to the caller (not using an exception).

This was recently broken by #87 and I have committed a fix for the regression in 7af4e21.

Thanks for pointing out the problem.

I'd like for you to reconsider the possibility of an API change. Your "fix" changes the error for cyclic packages to:

Traceback (most recent call last):
  File "src/catkin/bin/catkin_make_isolated", line 136, in <module>
    main()
  File "src/catkin/bin/catkin_make_isolated", line 132, in main
    destdir=destdir
  File "src/catkin/bin/../python/catkin/builder.py", line 779, in build_workspace_isolated
    export_tags = [e.tagname for e in package.exports]
AttributeError: 'str' object has no attribute 'exports'

This is not actionable feedback. This is a horrible bug that just makes people hate ROS. You can either keep throwing shitty AttributeErrors or you could change the internals of topological_order.py and provide useful, actionable feedback to your users in this situation.

The stack trace has already been fixed as I commented before. Since commit 7af4e21 which is included in the latest release of catkin_pkg (0.1.27) the function returns a tuple with the first value being None and the second value being a string with all packages in the circular dependency.

The new stack trace is as bad as the old one but I suppose it's not your problem because the AttributeError comes from catkin/builder.py now...