Protocol sidebar

I did read your feedback and I discovered this issue, which is, I think a better place to present the feedback.

That's a reasonable conclusion and trying to start conversations in the most appropriate place is really valuable but I think a better default for in-progress conversations is to "play them where they land" and keep a conversation going in the same thread unless explicitly asked by a maintainer or other project member to relocate the discussion. It's much easier to reference a complete discussion than to try and follow one between multiple threads.

How that we're here though. Let's continue.

In #748 which introduced tracetools to the repos file, this discussion was had and between the ROS 2 team and the tracetools contributors the conclusion at the time was that tagged releases were the best compromise. I think reviewing that discussion is going to better prepare anyone participating in this one but I'll try for a brief summary:

In general we're trying to keep dependencies tight and reliable. (Recently we even removed poco, an example in the earlier discussion of a widely used library, in favor of a more tightly scoped internal implementation of the needed features.)
A major concern when reviewing a branch based inclusion was the fact that their change control process is not *exactly our process, that is, they don't run our official CI for each change. The problem is not resolved by independent CI.
The above concern makes it possible for a change to make it through an external review and automated testing process but still break our builds. And that compounds with the fact that as none of us are maintainers of the external project, we can't change the upstream in order to resolve the issue. Our only option for resolution is re-targeting either to an unreleased commit as in Bump to the latest commit on ros2_tracing to fix deprecation. #899 or pinning back to a previously known good commit.

You presented two rationales [for targeting a tag or commit ID]:
* "changes may regress the build" and "you don't have write access to the repo"
 ...
* "This repo doesn't have CI"

I think these positions are well covered by my summary above. What I had to think about is how and why our position here is different than it is for many other aspects of our CI configuration. Tracking volatile sources is definitely our default. All our python packages from pypi are tracking volatile release tracks rather than frozen versions with explicit upgrades. We're pinned to a stable release channel for our host operating system but we don't locally cache apt repositories and only upgrade after tests, so why are we reactive in those cases but preferring a proactive approach here? The strongest rationale that I see is because of all of the above, only things built inside the repos file are likely to actually cause build failures rather than
test failures. Build failures stop the build and no tests are run. For broad timed test runs like ours that can be damning. Losing an entire night's nightlies due to a change in an external project is really strong negative feedback for the reactive approach. Regressions in Ubuntu or the right PyPI package could still cause infrastructure or build failures but I assess those risks to be much lower. For Ubuntu due to the QA processes for releasing changes into a stable distribution and for PyPI because most of the utility of those packages is during the test phase.

Here are the rationales for designating and using a stable branch instead:

* Upgrade path is automated and more obvious. It's silly to have to check out a new ros2.repos just because other ROS2 projects introduce compatibility breaks and/or deprecations or because this project makes improvements. By using a branch, `git pull` or `vcs pull` does that automatically

I'll confess that I hadn't even considered this class of issues. I'm privileged to have enough bandwidth and compute available to me that I tend to dispose of old workspaces when I'm done with them and thus think starting fresh is easier than trying to update. So I thank you for bringing it up. I don't think this point of friction is a valid reason for changing our policy on tracking external repositories though.

Changing our preferred behavior to tracking branches rather than tags won't permanently resolve this as far as I can see since the proposed alternative to use branches and pin a known good commit when the branch breaks will still interfere with the vcs pull workflow. I'm neither a vcs power user nor maintainer but to me this friction point might be better addressed with a vcs "refresh" verb which can update a workspace in place with an input repos file, updating remotes, currently checked out refs, and cloned repositories as needed. That might not be plausible but I do think this issue is best resolved by tooling changes not policy changes as the policy change would be insufficient and the improvement it provides doesn't, in my opinion, override the reasons for sticking with tags.

* Following a branch shows trust in the developer. ROS2 encourages widespread contribution and participation. By not allowing projects we use to specify their own trajectory, you're saying "we don't trust you to release stable, high quality code". It betrays the community principles.

This is not an interpretation of the existing policy that gains purchase with me at all. But I and the other ROS team members at Open Robotics are working from a place of power relative to other contributors to the project and personally I would be very interested in learning if other contributors are put off by this policy.

Speaking for myself this point seems particularly disingenuous coming from you, as you and I along with several others are in the process of updating branches (from pinned commits for released distros) for repositories you're contributing to in #894 and #891. The decision to target tagged releases for ros2_tracing was reached in mutual agreement with the contributors maintaining that repository. They've made several PRs updating their release track and even our decision to pin a forward commit until a release is prepared was made at their suggestion. If they were to start a discussion expressing a desire to put their selected branch in the repos file instead of continuing the current process, I want to believe that we would assess whether we and they had built up sufficient mutual trust to review and potentially update the previous decision. But as an argument for changing where we start the conversation this doesn't sway me at all.

I'll also add a rationale for targeting upstream's development branch that hasn't been mentioned: which is that when we track releases we get updates in (potentially large) batches rather than as they're applied which increases the time between the change and its impact and that affects how easy it is to identify how regressions are introduced. Tracking a branch may cause issues with more frequency but hopefully the delta between the passing state and the regressed state is much smaller, saving work in the diagnosis and resolution.

To try and form a personal conclusion from everything I've read and written so far. I think it's worth justifying why we express a proactive preference in this case when we otherwise default to a reactive preference. When the next externally maintained repository is proposed for inclusion in the ROS 2 repos file as a reviewer I'll look at the possible consequences of a breaking change in that repository? Will only tests fail or is core compilation also at risk? If so, I expect that the prudent default preference is for gated releases which are pre-screened by our CI process. If a relationship with the maintainers of the external repository forms as ones certainly have with the maintainers of Fast-RTPS, CycloneDDS, and ros2_tracing, we can revisit that decision together with those maintainers.

If you read through all of the above thanks for sticking it out. You deserve a coffee break.

Little update: I accidentally published this comment when it was half-written. Since anyone with email turned on got the first half I decided to finish writing and post rather than wait until the morning to review and proofread. So there may be errors or imperfect expressions in the above text. It sure is long enough that not all of it can be right.

Track released version of ros2_tracing #898

Description

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions