-
Notifications
You must be signed in to change notification settings - Fork 136
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
draft of REP 143 #87
draft of REP 143 #87
Conversation
@@ -1,6 +1,6 @@ | |||
REP: 133 | |||
Title: Separation of build environment and source tree tools | |||
Author: Tully Foote <tfoote@willowgarage.com>, Dirk Thomas <dthomas@willowgarage.com>, Thibault Kruse <kruset@in.tum.de>, William Woodall <wwoodall@willowgarage.com> | |||
Author: Tully Foote <tfoote@willowgarage.com>, Dirk Thomas, Thibault Kruse <kruset@in.tum.de>, William Woodall <wwoodall@willowgarage.com> |
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.
Even if the answer is yes, please keep this separate from this PR. I only changed mine since the REPs can not have different email addresses for the same person.
lgtm |
+1 |
1 similar comment
+1 |
* test_commits: a boolean flag used to enable CI jobs for each commit to | ||
the branch specified under ``versions``. (default: false) | ||
* test_pull_requests: a boolean flag used to enable CI jobs for each pull | ||
request against the branch specified under ``versions``. (default: false) |
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.
Typo: version
This REP is to be implemented in version 0.4 of the Python package *rosdistro*. | ||
It will serve as a reference implementation for this REP. | ||
A draft implementation can be found in the "rep143" branch of | ||
[ros-infrastructure/rosdistro](https://github.com/ros-infrastructure/rosdistro/tree/rep143). |
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.
You probably won't keep this tree around, right? If so, it might be best to open a PR from this tree to master, and link to that (the PR ilnk will stick around, and the history of what was in the PR will stick around long after the tree goes away).
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 references: e1107df
@dirk-thomas You aren't introducing anything in the rosdistro python code that is going to barf if I keep the release_builds/source_builds/etc in my own files, right? I took a quick readthrough of the changes and don't see anything, but I want to be sure. The reason I ask is that buildbot-ros uses data from those files to configure the build, and we probably won't have a chance to update them to the new config module right away. |
@mikeferguson As soon as the REP is accepted the rosdistro repository will be update to the latest version of the index and distribution files. At that point the index does not refer to the build files anymore. So you can't query the build files through the rosdistro API anymore. If it help buildbot the obsolete build files can be kept around in the repo for some time. But it should be very easy to migrate to the new ros_buildfarm.config module instead. Basically the API of the build files are identical or have only small enhancements. The existing code was simply moved from rosdistro to ros_buildfarm.config. |
I'm not sure if I understand the proposed workflow for overlaying a custom rosdistro over the default OSRF rosdistro. I see two possibilities:
Which of these is the intended use-case? |
These two look very similar and only differ on the point of whether to use the current OSRF distribution file from github or using a local, potentially frozen copy. If you want to develop closed software on top of a frozen ROS distribution, it might help to keep a copy of the whole rosdistro repo and add a distribution file on top for your own software. Alternatively, you may just want to run a buildfarm for software that is not published to rosdistro, but that should build against the latest ROS packages. In this case you probably want to reference the distro file on github. @dirk-thomas would it make sense to give more semantics to these multiple distribution files? I am thinking about something like debian components. What this means is a dictionary instead of a list, with named distribution files. This can help a buildfarm build only a subcollection of the available software. |
@mkoval Both options are valid and it is your choice to decide which trade off you would like to have. If you reference the ROS indigo distribution.yaml directly you will get any change immediately and with on effort. If you fork it and use the forked distribution files you have more control about when you merge in changes from the official repo. While this allows you do be more conservative and potentially stable it require to merge in changes whenever it suites your needs. |
@po1 I don't see an advantage of having a dictionary. What would the keys be and what are there semantic meaning? How is the order maintained? |
You are right about the order, that would defeat the purpose of overriding a distro. What I had in mind is more about having independent distribution subsets, but I guess this falls out of the scope of the proposed change. |
@po1 In order to more comfortably identify sets of packages / repositories in build files they should have the ability to identify them based on in which distribution file they are defined. I have added a list of tag names to the distribution file (see 0bcd0d9). The build files can then specify a whitelist / blacklist of tag names which they want to operate on (instead of applying to all which is the default behavior). |
That sounds good! |
+1 (self-vote) to merge this draft as-is. We will wait until the buildfarm is deployed (and potentially more feedback comes up) before making it final. And for that I would also copy all content from REP 141 into this one in order to be a complete document on its own (rather then only containing the diff). |
+1 for merging as draft. We can have a separate pull-request / review for finalizing. |
This REP is still work-in-progress.