-
Notifications
You must be signed in to change notification settings - Fork 193
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
update universal build tool article to reflect decision to use colcon #168
Conversation
articles/101_build_tool.md
Outdated
It should also work with packages that do not provide manifest files themselves, given that the meta information is externally provided. | ||
This will allow the build tool to be utilized for non-ROS packages (e.g. Gazebo including its dependencies). | ||
It should also work with packages that do not provide manifest files themselves, given that the necessary meta information is provided externally. | ||
This will allow the build tool to be utilized for non-ROS packages (e.g. Gazebo including its ignition dependencies). |
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.
Why is this now restricted to ignition deps? this wouldnt work for e.g. sdformat?
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 is not "restricted". It is just an example:
e.g. ...
articles/101_build_tool.md
Outdated
|
||
### ament_cmake | ||
|
||
[ament_cmake](https://github.com/ament/ament_cmake) is an evolution of `catkin` and is also based on CMake. | ||
The main difference between `ament_cmake` and `catkin` is described in [another article](http://design.ros2.org/articles/ament.html). | ||
In the context of the build tool the biggest difference is that `ament_cmake` generates package-specific files to setup the environment to use the package after it has been built and installed. | ||
|
||
A package using `ament_cmake` uses the same manifest file as `catkin` (except that it only allows the newer format version 2). | ||
A package using `ament_cmake` uses the same manifest file as `catkin` (except that it format version 2 or higher). |
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.
missing words in refactoring ?
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.
Fixed in 553d408.
articles/101_build_tool.md
Outdated
The tool performs an "isolated" build like `catkin_make_isolated` and `catkin_tools` (one CMake invocation per package) and also parallelizes the build of packages which have no (recursive) dependencies on each other (like `catkin_tools`). | ||
While is coveres more build systems and platforms than `catkin_tools` it doesn't have any of `catkin_tools`s usability features like profiles, output handling, etc. |
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.
not sure what you meant here, "while it covers" maybe?
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.
Fixed in 553d408.
articles/101_build_tool.md
Outdated
|
||
When the first draft of this article was written the conclusion was to not to spend any resources towards a universal build tool. | ||
As a consequence the author of this article went ahead and developed [colcon](https://github.com/colcon/) as a personal project. | ||
Therefore its feature set is closely aligned with the following requirements. |
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.
Should we highlight here that this doesn't have usability features of catkin_tools either such as profiles (like it's been mentioned above for ament_tools)?
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.
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 think that's my point... You added it above for ament_tools
and not here while it applies to both.
The fact the selecting colcon requires that feature to be listed in the "Future" section doesn't justify not mentioning it here in my opinion.
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.
Please feel free to commit any proposal you think makes it better.
Something important that is missing from this discussion (not necessarily the document) is information about what the change to colcon actually means. In particular, what happens to ament? Is it replaced entirely? Or is colcon expected to sit on top of it? Perhaps a mixture, with some native colcon packages and some using ament, especially the ament CMake macros? I did not find this clearly in the document. How much work is expected for users to change to using colcon? |
The article already tries to describe clearly the difference between the build system and the build tool. The former one is Since I am the author of the article as well as the tool it is difficult for me to "see" what needs to be described better. Please feel free to propose changes to the article to improve it and clarify where necessary.
That means the user "only" has to learn how to invoke the new build tool. This page gives some advice how to transition from |
articles/101_build_tool.md
Outdated
### colcon | ||
|
||
When the first draft of this article was written the conclusion was to not to spend any resources towards a universal build tool. | ||
As a consequence the author of this article went ahead and developed [colcon](https://github.com/colcon/) as a personal project. |
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.
While it is interesting for me to see how decisions are made at OSRF, maybe this part could still just state the relevant technical facts, such as: "The discussion about the first draft was inconclusive. Colcon was created as a PoC to demonstrate the viability of the approach of a unified build tool layer above respective build-system libraries."
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.
While it is interesting for me to see how decisions are made at OSRF, maybe this part could still just state the relevant technical facts ...
As any other company OSRF has limited resources. And with that constraint comes the difficult decision what project / part to spend them on. I think it is important to acknowledge that here to provide context to readers why there was no significant development in this area in the past year.
The discussion about the first draft was inconclusive
From my point of view that does not reflect the facts. The previous draft came up with a clear rational why it would be useful to unify the different tools. At the end of the day no party was willing to put the necessary effort behind the idea to move in any of the proposed directions. That is why the topic was basically "on hold" for over a year. I think it is valuable to include the history of this article to help readers follow how we got into the current state.
Colcon was created as a PoC to demonstrate the viability of the approach of a unified build tool layer ...
I don't think this is the case either. I have implemented build systems and build tools in the past as well as wrote the initial draft of this article describing that there is a strong need for unifying the development. The technical feasibility wasn't really a concern.
colcon
was pretty much developed by myself for two reasons:
- curiosity to learn and use new stuff (e.g.
setup.cfg
files), the challenge to design a complex piece of software in a highly modular and extensible way and - as a "side effect" improve my own workflow to build Gazebo, ROS 2 and ROS 1 which I am using in my day job more efficiently.> While it is interesting for me to see how decisions are made at OSRF, maybe this part could still just state the relevant technical facts ...
As any other company OSRF has limited resources. And with that constraint comes the difficult decision what project / part to spend them on. I think it is important to acknowledge that here to provide context to readers where there was no significant development in this area in the past year.
The discussion about the first draft was inconclusive
From my point of view that does not reflect the facts. The previous draft came up with a clear rational why it would be useful to unify the different tools. At the end of the day no party was willing to put the necessary effort behind the idea to move in any of the proposed directions. That is why the topic was basically "on hold" for over a year. I think it is valuable to include the history of this article to help readers follow how we got into the current state.
Colcon was created as a PoC to demonstrate the viability of the approach of a unified build tool layer ...
I don't think this is the case either. I have implemented build systems and build tools in the past as well as wrote the initial draft of this article describing that there is a strong need for unifying the development. The technical feasibility wasn't really a concern.
colcon
was pretty much developed by myself for two reasons:
- curiosity to learn and use new stuff (e.g.
setup.cfg
files), the challenge to design a complex piece of software in a highly modular and extensible way and - as a "side effect" to improve my own workflow to build Gazebo, ROS 2 and ROS 1 which I am using in my day job more efficiently.
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.
Maybe replace "As a consequence" with "Meanwhile", to reduce the risk of sounding accusatory.
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.
Maybe replace "As a consequence" with "Meanwhile", ...
Updated in d326e11.
### Start "from scratch" / colcon | ||
|
||
Since the first draft of this article the `colcon` project has been developed with the goals and requirements of a universal build tool in mind. | ||
In its current form it is already able to build ROS 1 workspaces, ROS 2 workspaces, as well as Gazebo including its ignition dependencies. |
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 assume that colcon cannot build workspaces with a mix of ros1 and ros2 packages, since this was a limitation discussed originally. I that true? I did not see it mentioned in the changes or the colcon manuals. Might just help managing expectations to state it explicitly.
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 assume that colcon cannot build workspaces with a mix of ros1 and ros2 packages, since this was a limitation discussed originally. I that true?
I don't think that is the case. colcon
doesn't mind what kind of packages it processes in a workspace. Technically I don't see anything within the build tool prohibiting to process a heterogeneous workspace.
But there are other things affecting how useful or how feasible a mixed workspace is. Commonly ROS packages depend on other ROS packages by name, find_package
them, use their API etc. all of that needs to be "compatible" too in order to successfully build a heterogeneous workspace.
For a simple example: a library exported by a catkin
package could transparently be used and linked in an ament_cmake
package. That is more related to the compatibility between both CMake packages than to the build tool. Please feel free to give that use case a try.> I assume that colcon cannot build workspaces with a mix of ros1 and ros2 packages, since this was a limitation discussed originally. I that true?
I don't think that is the case. colcon
doesn't mind what kind of packages it processes in a workspace. Technically I don't see anything within the build tool prohibiting to do so.
But there are other things affecting how useful or how feasible a mixed workspace is. Commonly ROS package depend on other ROS packages by name, find_package
them, use their API etc. all of that needs to be "compatible" too in order to successfully build a heterogeneous workspace.
For a simple example: a library exported by a catkin
package could transparently be used and linked in an ament_cmake
package. That is more related to the compatibility between both CMake packages then to the build tool. Please feel free to give that use case a try.
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.
Should we add a sentence here to highlight the state of colcon regarding support of heterogeneous workspaces?
articles/101_build_tool.md
Outdated
|
||
- Thorough test the functionality and write documentation for developers as well as users. | ||
Based on the above information we made the not easy decision to pick `colcon` as the universal build tool. |
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.
Who is "we"?
I think it needs a section right at the end, after the decision is discussed, explaining the specific consequences of the decision in a clear format (a list would work fine; items like "ament_tools will be retired", "ament_cmake will be supported by colcon and kept", "you will need to replace execution of ament with execution of colcon", etc.). It doesn't need to be in detail, but unless you clearly state in one place "these are the impacts on ROS 2 development, and these are the impacts on you, as a ROS 2 user", you are going to get this question a lot - and probably a lot of complaints about yet another build tool. I can see that the relevant information is present in the article, now that it's been clarified by you, but it's spread out and not easy to accumulate for the reader. I did actually read the article first, but most users will skim it and miss that information. |
Ok, I agree that a summary at the end would help. I tried to capture the "next steps", "implications" and "outlook" in 9974be3. Please let me know if that addresses your point or suggest additional changes. |
articles/101_build_tool.md
Outdated
@@ -233,6 +231,10 @@ The following uses cases should be satisfied by the unified build tool. | |||
The tool needs to be able to build ROS 1 workspaces which can already be built using `catkin_make_isolated` / `catkin_tools`. | |||
It is up to the implementation to decide if it only supports the standard CMake workflow or also the *custom devel space concept* of `catkin`. | |||
|
|||
In ROS 2 the concept of the *devel space* has intentionally been removed. | |||
In the future it might be feasible to provide the concept of *symlinked installs* in ROS 1 to provide a similar benefit without the downsides. | |||
Therefore not supporting this concept in the universal build tool is considered a viable path forward. |
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.
For ROS 1 or just ROS 2? If this includes ROS 1 (only thing that make sense from the text), then I don't feel that this was properly resolved.
Though I'm in favor of getting rid of the devel space in ROS 1, I think we cannot commit to doing that (and therefore not supporting it in the build tool) until we propose how to deprecate and then remove the devel space. In my opinion, It should still be a requirement that the default build tool for ROS 1 needs to support the devel space, at least until it is in the process of being removed.
Did others (other than @dirk-thomas) have a different conclusion based on our discussions?
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 completely agree with you points. Therefore I don't see colcon
becoming the recommended tool for ROS 1 until that has been worked out.
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.
For ROS 1 or just ROS 2?
colcon
doesn't aim to support the concept of a devel space - so that applies for ROS 1 as well as ROS 2.
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.
Therefore I don't see colcon becoming the recommended tool for ROS 1 until that has been worked out.
Perhaps I'm the only one (still looking for others input here), but I feel that's an important thing to figure out now, because if the conclusion (at some later date) is that ROS 1 cannot live without the devel space and colon will never support it, I think that reality should be captured here.
If we were to later surmise that "it's unacceptable for the default ROS 1 build tool to not support devel space", then we would have a situation which is at odds with the decision we have now.
I don't suppose we have to sort it out now, but it was a surprise to me when I realized it had not been resolved.
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.
Let's summarize the "facts" first then:
- The devel space requires ROS packages to do "special" logic (not native to CMake) to support the devel space correctly (e.g. different CMake extra files for devel and install).
- The fact that a package might only work in one of the cases (devel vs. install) if it the code is incorrect is surprising to users (since they might only use one of the two).
- The advantage of not copying resources can be achieved with symlinked installations which doesn't come with any of the above cons.
- The only disadvantage of using symlink install over devel space is that it requires packages to have install rules.
Based on these we decided in ROS 2 to not support the concept of devel space. Imo the same rational applies for a new build tool in ROS 1 (since the facts haven't changed). There would be an easy transition path: to add install rules if they are not already there.
That doesn't imply that we would ever remove the devel space logic from catkin
. If users would like to continue using it they are fine to do it using the existing build tools. I don't think we will ever remove those since we don't want to break existing workflows. Instead we would just recommend something different. Users just shouldn't expect active development on the not anymore recommended build tools - but they should continue to work.
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 think you might be missing my point.
I'm not arguing for or against having the devel space or the build tool support for it.
Instead, my point is that the intended future state ought to be decided on, it sounds like it has already been, and that in the implications section should say something like "therefore the default ROS 1 build tool will not support a devel space only workspaces, i.e. the default build tool will require install rules".
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 agree with @wjwwood. We need to have certainty about the intended final result.
I have no problems with the idea of ROS 1 eventually not supporting a devel space. I personally dislike the idea and much prefer ROS 2's approach to supporting a rapid development workspace.
However, the idea that ROS 1 could continue to work with both catkin and the new build tool sounds infeasible to me, if they have different requirements on the CMakeLists.txt files, which is what I think will be the case (please correct me if I'm wrong). Users of the new build tool would probably be fine, but as more and more packages support only the new build tool, those still using catkin for their work flow might find it increasingly difficult to stay that way. This seems like it would lead to friction.
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.
my point is that the intended future state ought to be decided on, it sounds like it has already been
I didn't intend to decide anything for ROS 1 in this article. The idea was to describe how a path forward could look like. A decision about ROS 1 should be made in an REP and/or discourse discussion.
the idea that ROS 1 could continue to work with both catkin and the new build tool sounds infeasible to me, if they have different requirements on the CMakeLists.txt files, which is what I think will be the case (please correct me if I'm wrong). Users of the new build tool would probably be fine, but as more and more packages support only the new build tool, those still using catkin for their work flow might find it increasingly difficult to stay that way. This seems like it would lead to friction.
I am not sure a new recommended build tool not supporting the devel space has a big impact here. As I mentioned in this comment this kind of problem already exists in ROS 1 today. Some packages do only work in the devel space and some do only work in the install space (since these packages haven't been developed with both in mind and the developers / maintainers / users only use one of the two). The choice of build tool doesn't change that. The only difference would be that recommending the new build tool without support for devel spaces will likely shift the ratio to more packages working in install space.
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.
Is there any data available on what the ratio is between one or the other style?
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.
Is there any data available on what the ratio is between one or the other style?
Not that I know of.
articles/101_build_tool.md
Outdated
A. Use `catkin_tools` as a starting point | ||
B. Use `colcon` as a starting point | ||
|
||
We acknowledge that if this topic would have been addressed earlier that some of the duplicate effort could have been avoided. |
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.
What duplication of effort could have been avoided by making a decision earlier?
It was always possible (whether officially or in spare time) to either build something new (duplicating effort) or contribute to existing software, or a fork of existing software (avoiding some redundant work).
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.
From my point of view no involved party wanted to commit resources on this topic for a long time. If that would have been different everyone involved could have influenced the direction of the development. Since that was not the case I spend my time on colcon
because I saw the advantages of starting from scratch. I decided not to contribute to catkin_tools
since I didn't consider the architecture of the code base to be a good foundation for the goals (the reference API for output handling in the article is just one example). And refactoring the whole code base wasn't in the scope of time I was willing to spend on this.
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.
My intention was not to criticize the approach, though I do think iterating on catkin_tools
would have been a better end result, instead I was just curious how you thought making the decision earlier would have saved code duplication.
articles/101_build_tool.md
Outdated
|
||
We acknowledge that if this topic would have been addressed earlier that some of the duplicate effort could have been avoided. | ||
When the work towards a universal build tool was suspended over a year ago it was a conscious decision based on available resources. | ||
Nevertheless we have to move forward with a decision in order to avoid further uncertainty and duplication. |
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.
Avoiding uncertainty, I agree with, however.
articles/101_build_tool.md
Outdated
Since both are written in Python either of the two tools could be "transformed" to cover the pros of the other one. | ||
So the two important criteria for the decision are: | ||
|
||
- the effort it takes to so (in the short term as well as in the long term) and |
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 sentence doesn't make sense to me, maybe you mean to do
, but not sure?
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.
Fixed in 783e1ae.
articles/101_build_tool.md
Outdated
|
||
Both of the considered options have unique and valuable features and there are good arguments to build our future development on either of the two tools. | ||
Since both are written in Python either of the two tools could be "transformed" to cover the pros of the other one. | ||
So the two important criteria for the decision are: |
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.
Are these the only criteria? If not, how did we down-select to these being the most important criteria?
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.
Imo the amount of time necessary to reach the stated goal is of highest importance. Simply because we are already very thin on resources and progress on feature development is rather slow. So any extra time spend on other items like this will slow our progress down. The second point is the quality of the end result since that will immediately effect the effort necessary to maintain and develop the software in the future. Therefore these are the "two important criteria".
There are certainly more but I didn't enumerate them here. Please feel free to add more from our internal documentation.
articles/101_build_tool.md
Outdated
- Refactor the software architecture of the existing code base to support the flexibility sketched by the extension points listed above. | ||
- Support cross compilation. | ||
- Support `DESTDIR`. | ||
- Support a feature similar to the `profile` verb of `catkin_tools`. |
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.
More things:
- job server
- support for
GNUInstallDirs
(not sure ifcolcon
already supports this or not, e.g. Catkin tools don't handle arch-specific library folders catkin/catkin_tools#154) - tests to cover corner cases already found and addressed before, e.g. correct topological order with underlays (use underlay workspaces when calculating topological order ros/catkin#590)
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.
Please feel free to add them to the article.
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.
Just for ref: #169.
articles/101_build_tool.md
Outdated
|
||
*Hopefully to be continued in the future...* | ||
- Since `colcon` can be used to build ROS 1 early adopters can try to use it to build ROS 1 from source. | ||
While there is documentation how to migrate from [catkin_make_isolated](http://colcon.readthedocs.io/en/latest/migration/catkin_make_isolated.html) and [catkin_tools](http://colcon.readthedocs.io/en/latest/migration/catkin_tools.html) `colcon` won't be the recommended build tool in ROS 1 for the foreseeable future. |
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.
But the implication is that colcon
will eventually be recommended instead of catkin_make_isolated
and catkin_make
, and that catkin_tools
will not be receiving attention with respect to ROS 2. And because colcon
will eventually be the recommendation, we will therefore not spend any extra time on catkin_tools
for ROS 1 either. Is that not the case?
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.
Yes, that is the case.
👍 |
This may be tangential to the discussion, but reading the bit about inferring meta information from things like the |
I think that needs to be almost completely within the build system (not the build tool), because ideally it should build the same way with or without the build tool. |
@wjwwood , @gbiggs is talking about this text about amend_tools :
So that seems to relate to the build tool (which makes sense, if this is about inferring the build-sytem invocation order of packages based on their dependencies). But I guess this is limited to evaluating the setup.py, not about the CMakeLists.txt |
That guess is incorrect. Even But lets exclude that topic form this discussion since it is not related to the actual question of the build tool choice. |
I see what you mean now @gbiggs, thanks @tkruse. While that feature is convenient, I personally feel that it is fragile because there are so many cases where automatically extracting all the dependencies, especially in CMake, is not possible, and the |
Fair enough. We'll just have to continue accepting it as the price to pay for managing the packages properly. |
articles/101_build_tool.md
Outdated
|
||
## Decision process | ||
|
||
For the decision process only the following two options are being considering based on the rational described above: |
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.
... based on the rationale ...
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.
Fixed in 2e9804e.
articles/101_build_tool.md
Outdated
- Support for Python 3 and Windows. | ||
- Support for pure Python packages as well as packages without an in-source manifest file. | ||
- Environment setup of `ament` packages | ||
To elaborate on the rational one significant advantage of `colcon` is that it is ready to be deployed for ROS 2 right now and it covers our current use cases. |
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.
... on the rationale ...
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.
Fixed in 2e9804e.
articles/101_build_tool.md
Outdated
|
||
- Create the software architecture to support the flexibility sketched by the extension points listed above which will be easier "from scratch" than for an existing code base. | ||
- The instructions in the ROS 2 wiki to build from source will be updated to use `colcon` instead. | ||
- The `ament_tools` repository will be archive, removed from the `ros2.repos` file, and won't be released into *Bouncy*. |
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.
archived
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.
Fixed in 2e9804e.
articles/101_build_tool.md
Outdated
|
||
- Refactor the software architecture of the existing code base to support the flexibility sketched by the extension points listed above. | ||
- Support cross compilation. |
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 remember from offline discussion that there was no feature to be added here but just a lack of testing.
I recommend splitting the "missing features" from the "untested features" as it gives a better idea of how much work is still needed.
As a data point I cross-compiled ardent following these instructions with colcon and it built successfully. I didnt have a board around to do runtime testing though
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.
Thank you for trying it. I removed the bullet for now (3b1925a) since there seems to be no problem. If it turns out that there is still a problem it can be addressed as a bug fix.
* typo * clarify how the decision was made * add a few more items to the list of things needed to complete the colcon option * clarify future plans for ROS 1 * typo * note existing case for catkin packages and devel space versus install rules * remove line about devel space
In the meantime Since it is the plan to use |
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.
pushed a new nitpicks in 6f79a7a, feel free to revert any of them if they are not appropriate.
This looks good to me overall, I made a couple comments below
articles/101_build_tool.md
Outdated
@@ -3,7 +3,7 @@ layout: default | |||
title: A universal build tool | |||
permalink: articles/build_tool.html | |||
abstract: | |||
This article describes the rationale for a universal build tool. | |||
This article describes a universal build tool for ROS 1 and ROS 2. |
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.
as part of the goals is to have this build tool ROS independent. Do we need to narrow it to ROS 1 and ROS 2 here?
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 don't mind either or. I reverted the change to the abstract sentence to keep it as is: c57a468.
articles/101_build_tool.md
Outdated
It should also work with packages that do not provide manifest files themselves, given that the meta information is externally provided. | ||
This will allow the build tool to be utilized for non-ROS packages (e.g. Gazebo including its dependencies). | ||
It should also work with packages that do not provide manifest files themselves, given that the necessary meta information is provided externally. | ||
This will allow the build tool to be utilized for non-ROS packages (e.g. Gazebo including its ignition dependencies, sdformat, etc.). |
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.
Not sure if it's part of the goals but I'm mentionning it as its part of the ament_tools and colcon featureset:
- infer dependencies form the CMakeLists for packages that don't provide manifests and no information is provided externally
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.
Mentioned the option to infer metadata in 242c139.
### Start "from scratch" / colcon | ||
|
||
Since the first draft of this article the `colcon` project has been developed with the goals and requirements of a universal build tool in mind. | ||
In its current form it is already able to build ROS 1 workspaces, ROS 2 workspaces, as well as Gazebo including its ignition dependencies. |
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.
Should we add a sentence here to highlight the state of colcon regarding support of heterogeneous workspaces?
In its current form it is already able to build ROS 1 workspaces, ROS 2 workspaces, as well as Gazebo including its ignition dependencies. | ||
It uses Python 3.5+ and targets all platforms supported by ROS: Linux, macOS, and Windows. | ||
|
||
Since it hasn't been used by many people yet more advanced features like cross compilation, `DESTDIR`, etc. hasn't been tested (and will therefore likely not work yet). |
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 didnt try cross-compilation since my original review but last time I tried it seemed to work as expected
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.
Until we have exercised this I think mentioning this is the "defensive" approach.
articles/101_build_tool.md
Outdated
Neither *devel* jobs or *pull request* jobs are available nor is it supported to build a local *prerelease*. | ||
For the coming ROS 2 release *Bouncy* these job types should be available to support maintainers. | ||
|
||
In ROS 2 *Bouncy* the universal build tool will be the only supported option and `ament_tools` will be archived. |
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.
In Bouncy, while encouraging users to use the universal build tool, ament_tools
will still be released in Bouncy to ease user transition and the repository cannot be archived until the distributions using it are all EOL (in this case June 2019)
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.
Updated in 81b8dbd.
articles/101_build_tool.md
Outdated
- Environment setup of `ament` packages | ||
- The ROS 2 buildfarm(s) will be updated to use `colcon` and provide devel / PR / prerelease jobs for the *Bouncy* release. | ||
- The instructions in the ROS 2 wiki to build from source will be updated to use `colcon` instead. | ||
- The `ament_tools` repository will be archived, removed from the `ros2.repos` file, and won't be released into *Bouncy*. |
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.
Same remark as above
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.
Updated in 81b8dbd.
👍
I replied to / addressed all your recent comments. Ready for review again. |
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.
reads well to me !
Follow up of #115 which created the first version of the article.
This update describes the new alternative colcon, the decision process we followed as well as the decision to continue with
colcon
as the universal build tool in ROS (2).