Skip to content
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

add article about dependency groups #138

Closed
wants to merge 6 commits into from
Closed

Conversation

dirk-thomas
Copy link
Member

While not complete yet this draft describes the use cases and goals and outlines what needs to happen next in order to determine if that is feasible and if yes how it can be implemented.

Especially for the Debian part any kind of concrete input is appreciated.

@dirk-thomas dirk-thomas added the in progress Actively being worked on (Kanban column) label Aug 25, 2017
@dirk-thomas dirk-thomas self-assigned this Aug 25, 2017
@nuclearsandwich
Copy link
Member

I've only had time to do a quick first read. Nothing seems glaringly out-of-place.

I'll be able to review the Debian binary package overview. Other important ones to investigate based on ROS 2's known target platforms are Homebrew, Chocolatey, and potentially pkgsrc. We should definitely take a look at RPM-based distros too though I vaguely recall that they have a similar implementation of "provides". I can try drafting up sections for Homebrew and Chocolatey as long as someone is up for reviewing the work against their own reading of the docs. Since we have superflore for Gentoo / OpenEmbedded in progress maybe @allenh1 can review a later draft of this and make sure that the proposal won't contain anything that will clash irreconcilably with that ecosystem.

I didn't bother looking at spelling at this draft stage.

@dirk-thomas dirk-thomas added in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels Oct 10, 2017
@dirk-thomas
Copy link
Member Author

I updated the PR with more concrete information as well as possible format extensions. @ros2/team I would like to get feedback on the design draft from multiple people.

Copy link
Member

@wjwwood wjwwood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made a small fixup commit, but otherwise makes sense to me. However, I will admit that a few places may remain fuzzy to someone with less context than me. I think we could benefit from a review from someone not familiar with the subject.

```
<package>
<name>rmw_implementation</name>
<group_depend satisfy="at-least-one">i_am_a_rmw_implementation</group_depend>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since at-least-one is pretty verbose I was considering any (based on the Python function). Does that sound reasonable to native speakers?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, I like any better.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'any' and 'all' would make sense. The other thing not discussed here would be 'one', where it would require one and only one from the group, but we can also scope that out. That's sometimes done where there's multiple implementations but they're not side by side installable.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, any is probably fine. I originally advised for the more specific key phrase, as I felt any could mean zero as well, e.g. in regex * is any and + is at least one.


## Processes

The following two processes need to be update to consider dependency groups.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The buildfarm will also require changes won't it? It uses the rosdistro cache to determine build job ordering separately from the build or release tooling.

Setting aside potential inconsistencies between package group members at bloom-time and group members in the current rosdistro cache:

  • For satisfy-all we would need to trigger a build whenever any upstream group member is rebuilt.

  • For satisfy-at-least-one, in the cases like rmw implementations, we would want all group members to be available at build time (bloom would likely list them all as Build Depends so that support for them is compiled in. Regardless of which is chosen at install-time) which means we can use the same behavior as satisfy-all.

But I don't see how the buildfarm would be able to choose which group members to include or not if there was a reason that all members could not be part of the same build (for example if two dependencies were mutually exclusive).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added mentioning ros_buildfarm in 0534486.

But I don't see how the buildfarm would be able to choose which group members to include or not if there was a reason that all members could not be part of the same build (for example if two dependencies were mutually exclusive).

If some of the group members conflict with each other satisfy-at-least-one could fall back to remove a package from the set (instead of using all). Which one to be removed can be decided based on a preferred list and a deterministic order (e.g. alphabetical).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nuclearsandwich Out of curiousity, what situations are you thinking of with two mutually exclusive dependencies? It's not that I can't imagine it ever happening, but more like I would mostly consider it a bug in the packaging/metadata if it did happen. I'm just wondering in what scenarios you see it happening.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: need to be updated

It will need all package manifests available from `rosdistro` to resolve a dependency group to a set of packages.

For the `satisfy-all` semantic it can simply generate dependencies on all packages from the group.
For the `satisfy-at-least-one` semantic the release tool could either use a platform specific mechanism like `provides` in Debian control files or simply expand the group to all packages (same as `satisfy-all`).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see a concern with using the satisfy semantic to determine whether or not the release tool uses a platform packaging mechanic like provides: From my read of the document thus far, there is no distinction in the group membership declaration between a group which is meant to be used with satisfy-at-least-one (Use case (1)) and a group intended for use with satisfy-all (Use case (2))

The Provides mechanism is used on the group member packages, not on the depending packages which is where the distinction between satisfy-all and satisfy-at-least-one is made.

Groups intended for both use case (1) and (2) are not distinguished and would by extension share the same release template in bloom (or any other per-package release tool) so virtual packages generated by the use of Provides in Debian would be created for both groups of rmw implementations and groups of msg containing packages.

I think this is only a serious problem if creating virtual packages for use case (2) groups is undesirable rather than superfluous.

The Debian policy manual has more info on how virtual packages and provides works. I notice that virtual packages cannot accept version constraints nor can a package provide a specific version of a virtual package. That may or may not be important down the line. It might be worth looking at other package specifications group and provider mechanics, particularly RPM to make sure that overlapping these two concerns will work on other platforms too.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my read of the document thus far, there is no distinction in the group membership declaration between a group which is meant to be used with satisfy-at-least-one (Use case (1)) and a group intended for use with satisfy-all (Use case (2))

The group dependency will have an attribute which identifies which satisfy semantic is desired: see example https://github.com/ros2/design/pull/138/files#diff-c8d974466de0d80b2299df802e7b1d7bR132

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The dependent package specifies the desired semantic but that is unknown the to the group members themselves: https://github.com/ros2/design/blame/0534486af1398a3410159130183ed4c918dcdbbd/articles/102_dependency_groups.md#L141

Also, one package may have <group_depend satisfy="all">i_am_a_rmw_implementation</group_depend> while another may depend on the same group with satisfy-at-least-one: <group_depend satisfy="at-least-one">i_am_a_rmw_implementation</group_depend>

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming bloom decides to generate a Provides entry for the member_of_group tag using the name of the dependency name. In that case the package declaring the group_depend tag will still use the satisfy attribute to either expand all group members (in case of all) or depend on the group name (in case of at-least-one) which will then rely on the Provides to be present.

I don't see a problem with this since the package which needs to make the semantic decision does have the satisfy attribute. The members of the group simply do always the same thing (generate Provides with the group name, if anybody uses it with at-least-one or not is not relevant) since they are not aware of the way they are being used.

Maybe I am missing your point?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The members of the group simply do always the same thing

Ah, because the mention of the provides behavior was only in the paragraph about any/at-least-one it makes it seem like the release tool needed to know the difference. Perhaps the section about group member behavior should be pulled above of the section about dependency resolution for each use case.

This brings up a secondary concern about whether this overlapping group mechanic will be suitable for packaging systems other than Debian. From my reading of the debian documentation there's no ill effects from a package providing a virtual package that isn't used. I mentioned previously that I'd take a look at Homebrew's mechanisms for this and will do so first thing tomorrow.

@nuclearsandwich
Copy link
Member

One observation: Because group membership is declared in the package.xml it means that rosdep keys cannot be group members.

For the rmw implementation use case we currently build two of the three available rmw implementations within the rosdistro, but I think some could potentially be system dependencies resolved by rosdep.

This would be okay today because we have the rmw_implementation ROS package depending on rmw_$DDS_PROVIDER ROS packages. But is that the package structure we want to keep or just what we're using because it fit within the existing package constraints?

So all group member packages would have to follow that same structure, perhaps with the substitution of a virtual package for rmw_implementation.

A ROS package foobar which depends on something that provides foo or bar could not <group_depend satisfy='at-least-one'>ros_foobar</group_depend> and have ros_foobar resolve to the system dependencies libfoo-dev or libbar-dev. They would need to create ROS packages foo_ros and bar_ros which could declare membership in foobar and each depend on libfoo-dev and libbar-dev respectively.

@dirk-thomas
Copy link
Member Author

Because group membership is declared in the package.xml it means that rosdep keys cannot be group members.

Yes, the fact that "rosdep keys" can't be used comes from the fact that the dependency group inverts the direction of dependency declaration. Instead of A declaring that it depends on B, B is declaring that it is a dependency of A (using the agreed upon group name). Since non-ROS packages usually don't have a manifest they can't provide that kind of information.

This is certainly a constraint. Do you see a way how we could enable this?

Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've got some suggestions on improved wording, and some questions in my review.

The goal would be that at least one RMW implementation needs to be present in order to build or use the `rmw_implementation` package.
It would be acceptable if more than one or even all RMW implementations are available but that is not required.

In case multiple package declare to be part of that dependency group the `rmw_implementation` package might want to provide an ordered list of preferred names which should be considered in that order.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe change this to:

If multiple packages are declared as part of that dependency group the `rmw_implementation` package might want to provide an ordered list of preferred names which should be considered in that order.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kind of prefer the "active" wording "the package declaring" rather than "are declared".


This article describes the concept of "dependency groups" which could be used for different purposes.
In terms of terminology a dependency group is a set of dependencies identified by the name of the group.
For such a group a different semantic can then be applied (instead of just satisfy each dependency on its own).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

satisfy -> satisfying


## Processes

The following two processes need to be update to consider dependency groups.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nuclearsandwich Out of curiousity, what situations are you thinking of with two mutually exclusive dependencies? It's not that I can't imagine it ever happening, but more like I would mostly consider it a bug in the packaging/metadata if it did happen. I'm just wondering in what scenarios you see it happening.

```
<package>
<name>rmw_implementation</name>
<group_depend satisfy="at-least-one">i_am_a_rmw_implementation</group_depend>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, I like any better.

In the ROS ecosystem a ROS package provides information about its dependencies in the manifest.
The latest format is specified in the [REP 140](http://www.ros.org/reps/rep-0140.html).
It supports different kind of dependencies (`build`, `exec`, `test`, `doc`, etc.).
But each dependency is mandatory and needs to satisfied on its own.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reword as:

It supports different kinds of dependencies (`build`, `exec`, `test`, `doc`, etc.), but each dependency is mandatory and must be satisfied on its own.


## Use Cases

The following subsections will describe two different use cases how dependencies could be used.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reword as:

The following subsections will describe how dependencies could be used in two different use cases.


The semantic of the group dependency (B) can be modeled as an attribute of the group dependency tag.

Each preferred / excluded package (C) name can be listed in child-tags of the dependency group to keep the information atomic.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exclusions and preferences are mentioned here and above, but there are no examples in the Example section, and I didn't see a mention of it in REP 0149. It would be nice to see examples of that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the scope of this document I would rather keep it as-is with a text description. The XML could look like this and will be specified in the REP once we have a consensus about the direction:

<group_depend>
  group_name
  <prefer>try_this_first</prefer>
  <prefer>try_this_second</prefer>
  <exclude>do_not_use_that</exclude>
</group_depend>

Copy link
Contributor

@tfoote tfoote left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to suggest considering a slightly different approach. I don't think that we need to add a new concept of a group. We can reuse the existing dependencies and follow a similar paradigm as both debian and rpm packages using a "Provides" declaration.

If we have packages declare that they "Provide" dependency key "foo" that would be the same semantics as declaring they're in group "foo". In debian language it's a virtual package. And if any of a virtual packages is installed that dependency is considered resolved. And we can update our toolchain to do the same logic.

This is implemented in both debian: https://www.debian.org/doc/manuals/debian-faq/ch-pkg_basics.en.html and RPM packages: http://ftp.rpm.org/api/4.4.2.2/dependencies.html

The one thing that we need that they don't implement is the "all" element. And that's a little bit of a misnomer. That's really a hint to a tool like bloom to say if you know of anything that provides this capability, I'd like you to install all known versions. As such I'd suggest resolving this via an export that ask for <depends_expand_all_available>key<depend_expand_all_available/> or maybe something a little more compact that bloom and rosinstall generator can leverage.

The provides could also be implemented in the export section too. Though it would probably be worth adding to the top level spec.

```
<package>
<name>rmw_implementation</name>
<group_depend satisfy="at-least-one">i_am_a_rmw_implementation</group_depend>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'any' and 'all' would make sense. The other thing not discussed here would be 'one', where it would require one and only one from the group, but we can also scope that out. That's sometimes done where there's multiple implementations but they're not side by side installable.

@dirk-thomas
Copy link
Member Author

dirk-thomas commented Oct 11, 2017

If we have packages declare that they "Provide" dependency key "foo" that would be the same semantics as declaring they're in group "foo".

Does that mean that packages which want to depend on "foo" use an existing *depend tag? If yes, I don't like the idea that now the value in the dependency tags is potentially not a rosdep key anymore. I don't see where the logic should come from to decide if the name is a "provide" or a rosdep key. Without knowledge of both sets I think this is not decidable.

As such I'd suggest resolving this via an export that ask for <depends_expand_all_available>key<depend_expand_all_available/> or maybe something a little more compact that bloom and rosinstall generator can leverage.

I don't like the idea that something in the export section is actually declaring dependencies. I prefer those to be explicitly declared as dependencies.

--

Just as a general comment: declaring all new information in the export section will not let us get around of defining a format 3 (not sure if that is the reason why you are trying to declare the information in that section?). Tools need to be able to distinguish manifests which do use these new information or those which don't. A "correct" tool which support format 2 would not process a format 3 manifest since it can't guarantee that it actually "understands" it - same for an updated format 2 manifest which has additional dependency information in the export section.

@nuclearsandwich
Copy link
Member

This is certainly a constraint. Do you see a way how we could enable this?

Not without adding something to the rosdep definitions themselves that adds them to a group. I think it's do-able, but I can also see us doing without it to avoid the complexity. If a package starts as a rosdistro package and a member of a group, but then becomes upstreamed as a rosdep key, it would lose its group membership. I cannot think of a real case where that would happen but it is a possibility.

@nuclearsandwich
Copy link
Member

@nuclearsandwich Out of curiousity, what situations are you thinking of with two mutually exclusive dependencies? It's not that I can't imagine it ever happening, but more like I would mostly consider it a bug in the packaging/metadata if it did happen. I'm just wondering in what scenarios you see it happening.

For whatever reason I couldn't reply inline. The case that comes quickest to mind would be a package like ffmpeg which can be compiled with only unencumbered codecs or with nonfree/patent encumbered format support as well. Both place libffmpeg.so on the file system and can't be co-installed. In Debian files this would be two packages that provide the same virtual package but conflict with each other.

An example from robot-land (that may also hit upon the fact that rosdep keys can't be group members) would be if a DDS vendor has an OSS version and a commercial version that both place lib$VENDORDDS.so and so cannot be co-installed.

@dirk-thomas
Copy link
Member Author

For whatever reason I couldn't reply inline. The case that comes quickest to mind would be a package like ffmpeg which can be compiled with only unencumbered codecs or with nonfree/patent encumbered format support as well. Both place libffmpeg.so on the file system and can't be co-installed. In Debian files this would be two packages that provide the same virtual package but conflict with each other.

Good example. But isn't that already supported by?

  • ffmpeg_unencumbered and ffmpeg_nonfree declaring the conflict with each other
  • using a group depend on ffmpeg_group with satisfy=any - obviously the satisfy=all is not valid here since that would be same as declaring explicit dependendencies on both ffmpeg packages

@tfoote
Copy link
Contributor

tfoote commented Oct 11, 2017

Does that mean that packages which want to depend on "foo" use an existing *depend tag? If yes, I don't like the idea that now the value in the dependency tags is potentially not a rosdep key anymore. I don't see where the logic should come from to decide if the name is a "provide" or a rosdep key. Without knowledge of both sets I think this is not decidable.

The dependency would still be declared in the usual <depend*> tags. Add it would still have the logic of comparing to rosdep keys as well as keys from the rosdistro. The difference is that rosdistro's list of keys available would be the union of package names as well as the keys the "Provide". I think that since we already have conflicts it would make sense to have format 3 to extend for this.

I don't like the idea that something in the export section is actually declaring dependencies. I prefer those to be explicitly declared as dependencies.

I agree. I am suggesting only that the export section syntax would only be a hint for some tools(bloom and rosinstall generator) to saym "If you find a dependency with this key, I'm asking that you expand it to included all implementations you know about", instead of just passing it through. So if you have a <exec_depend>virtual_package_name</exec_depend> and <export><depend_expand_all_available>virtual_package_name</depend_expand_all_available> bloom and other tools would expand virtual_package_name into virtual_implementation1, virtual_implementation2, ... Otherwise the dependency will be passed through directly or the first/default implementation package will be picked, in a similar way that apt resolves virtual package dependencies. As a corollary behavior "any" would be the default.


In the ROS ecosystem a ROS package provides information about its dependencies in the manifest.
The latest format is specified in the [REP 140](http://www.ros.org/reps/rep-0140.html).
It supports different kinds of dependencies (`build`, `exec`, `test`, `doc`, etc.) but each dependency is mandatory and needs to satisfied on its own.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs to be satisfied?

An example for this is the `rmw_implementation` package.
Currently it depends on all RMW implementation packages (e.g. `rmw_fastrtps_cpp`, `rmw_connext_cpp`, etc.) in order to ensure that it may only be built after all RMW implementation packages have been built.
The goal would be that at least one RMW implementation needs to be present in order to build or use the `rmw_implementation` package.
It would be acceptable if more than one or even all RMW implementations are available but that is not required.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this imply the responsibility of the user that the semantics of the build does not change when a second item of the group is found afterwards? I am imagining the scenario where we find only one RMW, start building with the "shortcut" version, then figuring out a second RMW impl is available, but rmw_implementation is already setup wo/ dlopen support. Does this make sense?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From source all rmw impl. in the workspace will be build before the rmw_implementation package. If the workspace only contains one and later you build another workspace on top with another rmw implementation that wouldn't change rmw_implementation.

When building Debian packages at least one needs to be available. But more could be available.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After getting a bit more context offline, I got a few more insights which resolves my question here. The key point was that still all packages are traversed by ament before the first package is built.


## Processes

The following two processes need to be update to consider dependency groups.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: need to be updated

### (2) All packages of a specific "type"

Another use case is that a dependency group doesn't need to be explicitly defined by all dependency names (called `satisfy-all`).
Instead a dependency group is defined by an identifier and the expectation is that all packages which declare that they are being part of that group should be available.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am struggling following this. If not by an identifier (and I assume this could be a string here) how do you declare a dependency group in use case 1)? What is the difference between for example a group rmw_implementations and a group message_packages ?

Copy link
Member Author

@dirk-thomas dirk-thomas Oct 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both groups are just defined by the name used in a group_depend tag. Any package can then declare to be a member of it. There is no difference between use case 1 and 2 in terms of how the groups are defined - only the criteria how they are satisfied is different.

Maybe I don't understand your question and you can rephrase it?

### Build tool

The build tool needs to consider the declared dependency groups when computing the topological order.
Since it operates on a set packages in a workspace it has all manifests for all of these packages available.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: set of packages?


### (1) Need at least one "Provider"

A package might want to declare that at least one dependency from a group needs to be present (called `satisfy-at-least-one`).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how's the behavior if two packages declare the same dependency group?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Each one is evaluated on its own. Not sure I get what you are aiming for with the question?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nvmd. I just read the examples which clarified my question.

@nuclearsandwich
Copy link
Member

Good example. But isn't that already supported by?

It's supported by the proposed semantic. But the buildfarm implementation has some new complexities that I need to think through.

Since as of now, all group members must be rosdistro packages rather than rosdep keys, both ffmpeg_unencumbered and ffmpeg_nonfree would be packages built on the buildfarm.

Is something like the following considered valid by this proposal?

<group_build_depend satisfy="all">rmw_implementations</group_build_depend>
<group_exec_depend satisfy="any">rmw_implementations</group_exec_depend>

That would enable the use case I outlined above, where we need all group members available to compile support for whichever might be chosen at runtime. It would also allow us to do

<group_depend satisfy="any">
  ffmpeg
  <prefer>ffmpeg_unencumbered</prefer>
</group_depend>

Or whatever the syntax shakes out to be, and have the both the release tool and the buildfarm agree that ffmpeg_unencumbered should be the build dependency and the upstream job. In cases where no preference is expressed we need to figure out how both the release tool and the buildfarm choose which member to select at build time.

@dirk-thomas
Copy link
Member Author

But the buildfarm implementation has some new complexities that I need to think through.

What complexities are you thinking about? The buildfarm shouldn't have to do much beside considering the dependency groups when generating job dependencies. All other parts should be handled by bloom and rosdep.

Is something like the following considered valid by this proposal?

<group_build_depend satisfy="all">rmw_implementations</group_build_depend>
<group_exec_depend satisfy="any">rmw_implementations</group_exec_depend>

Yes, that would be valid.

@dirk-thomas
Copy link
Member Author

The dependency would still be declared in the usual <depend*> tags. Add it would still have the logic of comparing to rosdep keys as well as keys from the rosdistro. The difference is that rosdistro's list of keys available would be the union of package names as well as the keys the "Provide". I think that since we already have conflicts it would make sense to have format 3 to extend for this.

A package can declare a group dependency on "foo" without any package declaring to be a member of "foo". Therefore I don't even think the question "if a dependency key is a group name" can be answered by rosdistro / rosdep. So an invalid rosdep key can not be distinguished from a dependency group name. Therefore I think it must be annotated explicitly in the manifest as a dependency group name.

@tfoote
Copy link
Contributor

tfoote commented Oct 12, 2017

Or whatever the syntax shakes out to be, and have the both the release tool and the buildfarm agree that ffmpeg_unencumbered should be the build dependency and the upstream job. In cases where no preference is expressed we need to figure out how both the release tool and the buildfarm choose which member to select at build time.

@dirk-thomas and @dhood and I talked about this a bit yesterday. And there's two very distinct use cases for these rules one is when you're working in a source space and the other is when you're fetching resources based on names/keys. In particular from the source space the resolutions should rely on what's present and assert that conditions are met. And this is what should be encoded in the package.xml. However when we're releasing we may want to choose one or more implmentations to bundle into the build. So when we generate debian packages or rosinstall generations we need to be able to specifically select subsets of groups. For example we might want to build binary packages for only implementation_A as well as binary packages that include implementation_A and implementation_B for flexibility but slightly less optimized and a larger footprint. And the complicating use case is that someone else wants to package with only implementation_B. As such any preference should not be encoded into the packages themselves but should come from the environment in which the tools are being invoked. We discussed possibly using environment variables, or possibly an ini file that would allow for selecting preferred packages from within a group that the packaging and distribution tools would respect.

A package can declare a group dependency on "foo" without any package declaring to be a member of "foo". Therefore I don't even think the question "if a dependency key is a group name" can be answered by rosdistro / rosdep. So an invalid rosdep key can not be distinguished from a dependency group name. Therefore I think it must be annotated explicitly in the manifest as a dependency group name.

Another option we talked about that might be possible here would be to have the group membership act more like a reverse depends declaration on a real package instead of allowing "virtual" packages. Thus you would be required to declare your group or reverse dependencies onto a package that exists and if that package doesn't exist in the rosdistro than the reverse depends is considered invalid. And in source space an undeclared

@dirk-thomas
Copy link
Member Author

In particular from the source space the resolutions should rely on what's present and assert that conditions are met.

I don't think the build tool should "assert that conditions are met". It doesn't do so for any of the current dependencies and imo shouldn't try to do so for the new group dependencies. Instead it should utilize them when available (kind of best effort as it is right now with normal dependencies).

As such any preference should not be encoded into the packages themselves but should come from the environment in which the tools are being invoked. We discussed possibly using environment variables, or possibly an ini file that would allow for selecting preferred packages from within a group that the packaging and distribution tools would respect.

While this idea might enable additional use cases I don't think it is in the scope of this PR. At least I don't plan on designing this into that direction. For now I will keep the scope to:

  • use the group dependencies in the source builds to allow adding new members to the group without the need to change existing packages
  • the release dependencies will still be embedded in the manifest (as they are right now with "normal" dependencies)

Another option we talked about that might be possible here would be to have the group membership act more like a reverse depends declaration on a real package instead of allowing "virtual" packages. Thus you would be required to declare your group or reverse dependencies onto a package that exists and if that package doesn't exist in the rosdistro than the reverse depends is considered invalid. And in source space an undeclared

The paragraph ends kind of abruptly. Is there something missing at the end?

@tfoote
Copy link
Contributor

tfoote commented Oct 12, 2017

I don't think the build tool should "assert that conditions are met". It doesn't do so for any of the current dependencies and imo shouldn't try to do so for the new group dependencies. Instead it should utilize them when available (kind of best effort as it is right now with normal dependencies).

Yes, the build tool won't do anything. But the CMake logic will assert if required components are not found etc.

While this idea might enable additional use cases I don't think it is in the scope of this PR. At least I don't plan on designing this into that direction. For now I will keep the scope to:

  • use the group dependencies in the source builds to allow adding new members to the group without the need to change existing packages
  • the release dependencies will still be embedded in the manifest (as they are right now with "normal" dependencies)

That's fine for now if just declare the overlapping dependencies using the regular tags. I was mostly responding to the suggestion that we encode the preferences with a <prefer> tag which would add infrastructure in a direction that I don't think we want to take it.

Another option we talked about that might be possible here would be to have the group membership act more like a reverse depends declaration on a real package instead of allowing "virtual" packages. Thus you would be required to declare your group or reverse dependencies onto a package that exists and if that package doesn't exist in the rosdistro than the reverse depends is considered invalid. And in source space an undeclared

The paragraph ends kind of abruptly. Is there something missing at the end?

oops... Not sure what happened there.

... reverse depends would be a soft fail that might warn but nothing more.

@dirk-thomas
Copy link
Member Author

Based on the discussion there doesn't seem to be a consensus on which dependencies should be used for releasing a package and which should be used for building from source. Even for the example use cases of (1) rmw_implementation and (2) ros_bridge it is being questioned if the group dependencies should be used in the first place. I also didn't see any alternative proposal how to handle this (please correct me if you think there is a concrete proposal on the table.)

Therefore I am considering to significantly reduce the scope of this PR: to only address the ordering problem of packages in from-source builds. All of the use cases suffer from the fact that in order to add a new rmw impl. or message package another package (the aggregating one) has to be updated to list the new package as a dependency.

To avoid that overhead a package could declare to be a dependency of another package. That declaration is "reverse" compared to all other dependencies we declare. It would only be interpreted by the build tool and used for the topological order. Therefore the reverse dependency doesn't need to distinguish any of the dependency type (build, exec, etc.). It also can be declare in the export section since neither bloom nor rosdep nor rosinstall_generator would use it.

E.g. a package a contains the following in the manifest:

<export>
  <dependency_of>b</dependency_of>
</export>

The build tool would then treat this the same as if b would declare <depend>a</depend> in its manifest.

How b is going to "know" about a is not defined by this. In ament packages a would:

  • either register itself in the ament resource index using a resource name b uses
  • or register itself as an extension to an extension point b uses.

Please comment on this significantly reduced proposal.

@nuclearsandwich
Copy link
Member

For anyone else coming at this before ☕️ the updated proposal referenced in the comment above is the comment above and not the document in the PR source.

I understand the purpose of the drastic reduction in scope is the desire to reach consensus and begin implementing the draft and creating a ratifiable REP. One of the initial issues prompting the this design proposal was the desire to avoid hand-managing dependencies at build time by patching package.xml files for release. The reduced scope proposal doesn't accomplish this. Instead it replaces patching out unavailable dependencies with patching in desired ones and I think that puts us in a slightly worse position release-wise. Additionally, I think it provides too incomplete a solution to the use case of wanting to depend on all packages of a certain category or group to be considered a solution there.

rmw implementations case

For the packages rmw_implementation and rosidl_typesupport I currently need to overlay the package.xml in order to remove rmw implementations that are not supported by the debian builds (connext, right now). Because this file overlays the source package.xml rather than modifies it, it means that every time bloom is run the releasing engineer needs to manually verify that the upstream package.xml and the overlaid package.xml differ only in the desired ways. That is something that's probably solvable by customizing the bloom release commands but only because we would be deleting known content rather than inserting potentially unknown content.

With the new proposal bloom, which operates per package would have no knowledge that other packages have declared reverse dependencies. This means that the releasing engineer has to bring that knowledge somehow. Which requires more activity, either to programatically find potentially desired dependencies and insert them or to hold the entire network of dependencies in their head long enough to update the package manifest correctly.

packages with messages case

Declaring a reverse dependency rather than a group requires the upstream package to know all of its downstreams. So this would work for the ros1 bridge but any other package that wants to do something with "all the message packages" would need to convince every message package maintainer to adopt a reverse dependency on their package or apply a bunch of one-line patches to create a functioning source space which seems like it'd be easier or just as much work for that individual to directly depend on all those packages. So in the general case this reduced proposal is not a net-reduction of dependency management work.

@nuclearsandwich
Copy link
Member

Using groups in source builds

I think a reduced scope proposal I could support would be one that adds source build support for declaring group membership and build ordering based on some form of group dependency declaration.

Declaring group membership

<export>
  <group>rmw_implementations</group>
</export>
<export><group>messages</group><export>

Declaring to build downstream of a group

<export>
  <build_with_group>rmw_implementations</build_with_group>
</export>

This would not improve the releasing process but it would also not make it worse, which I think the earlier reverse dependency proposal would.

Also, it not require changes to rosdep, rosinstall, ros_buildfarm, or bloom which means it is also a reduced scope proposal. But it would allow us to evaluate groups in source builds as a potential solution for these problems and later decide to promote their use to more tools or retire them in favor of a better proposal.

@wjwwood
Copy link
Member

wjwwood commented Oct 17, 2017

Declaring a reverse dependency rather than a group requires the upstream package to know all of its downstreams. So this would work for the ros1 bridge but any other package that wants to do something with "all the message packages" would need to convince every message package maintainer to adopt a reverse dependency on their package or apply a bunch of one-line patches to create a functioning source space which seems like it'd be easier or just as much work for that individual to directly depend on all those packages.

This is reason enough, in my opinion, to not take the simplified reverse dependency declaration instead of the group mechanism.


@nuclearsandwich your suggestion of a simplified group declaration and group dependency declaration which only affects the build tools (not the release or dependency tools) is basically what I wanted all along, so I'd be ok with that. I went along with the discussion of how to address this issue in the release pipeline because I knew it was an important use case too.

However, I do feel it would be nice to get them out of the <export> section and have them be first class elements of the package manifest specification and in the API which presents that information.


This is how I always imagined the rmw_implementation solution would go:

package.xml for rmw_implementation

<package format="2">
  <name>rmw_implementation</name>
  ...
  <!-- explicit dependency is used for packaging, but is redundant in source build -->
  <depend>rmw_fastrtps_cpp</depend>
  <depend>rmw_opensplice_cpp</depend>

  <!-- augments explicit dependencies in source build scenario only -->
  <!-- would add rmw_connext_cpp to the list of implementations for instance -->
  <group_depend>rmw_implementations</group_depend>
</package>

package.xml for rmw_*_cpp like packages

<package format="2">
  <name>rmw_{fastrtps, opensplice, connext}_cpp</name>
  ...
  <!-- declares itself a member of the group -->
  <member_of_dependency_group>rmw_implementations</member_of_dependency_group>
</package>

For the "packages with messages" case would be much the same:

package.xml for ros1_bridge

<package format="2">
  <name>ros1_bridge</name>
  ...
  <!-- explicit dependency is used for packaging, but is redundant in source build -->
  ...
  <depend>sensor_msgs</depend>
  <depend>std_msgs</depend>
  ...

  <!-- augments explicit dependencies in source build scenario only -->
  <!-- would add "my_custom_interfaces" to the list of implementations for example -->
  <group_depend>packages_with_rosidl_interfaces</group_depend>
</package>

package.xml for packages with messages

<package format="2">
  <name>my_custom_interfaces</name>
  ...
  <!-- declares itself a member of the group -->
  <member_of_dependency_group>packages_with_rosidl_interfaces</member_of_dependency_group>
</package>

If, in the future, we could find the time to solve the release case, nothing would really change, except we might need more meta information in the package.xml files (like blacklists, whitelists, version dependencies, more specific dependency types), but fundamentally it would still be "I am part of a group" and "I depend on packages in a certain group". If we found a more detailed release solution, then we would just drop the redundant and explicit dependency declarations above and let the release tools interpretation of the group mechanism determine how the output is generated.

@dirk-thomas
Copy link
Member Author

I currently need to overlay the package.xml in order to remove rmw implementations that are not supported by the debian builds (connext, right now).

With this proposal the dependencies in rmw_implementation can and should be changed to list exactly the ones we want to build Debian packages against. So no more patching is necessary.

@dirk-thomas
Copy link
Member Author

dirk-thomas commented Oct 17, 2017

@nuclearsandwich If I understand your proposal correctly the only difference to my comment is that your declares the group name in the export section where mine relies on the "group name" to be a package?

The explicit declaration of the group name has the advantage that e.g. a forked package my_ros1_bridge will still be able to utilize the dependency on all message packages.

@dirk-thomas
Copy link
Member Author

dirk-thomas commented Oct 17, 2017

However, I do feel it would be nice to get them out of the <export> section and have them be first class elements of the package manifest specification and in the API which presents that information.

Lets first settle on what and how and later decide on the implementation details of the markup.

@nuclearsandwich
Copy link
Member

wjwood: However, I do feel it would be nice to get them out of the <export> section and have them be first class elements of the package manifest specification and in the API which presents that information.

dthomas: Lets first settle on what and how and later decide on the implementation details of the markup.

Agreed. I only proposed use of the export section because it gives us license to trial and refine without needing to implement a spec change.

@nuclearsandwich
Copy link
Member

Uhhhh @dirk-thomas something very weird happened to your comment #138 (comment)

I was probably "editing" it to quote it from source rather than rendered markdown and then when I pull quoted another block of text with the r hotkey GitHub dumped it into that text buffer instead of the new comment buffer.

I've taken what I meant to post as a separate comment and moved it below.


With this proposal the dependencies in rmw_implementation can and should be changed to list exactly the ones we want to build Debian packages against. So no more patching is necessary.

Under the simplified reverse dependency proposal you're proposing that we would remove rmw_connext_cpp from the list of exact dependencies in the rmw_implementation package.xml and instead add a reverse depend in rmw_connext_cpp's manifest on rmw_implementation.

This would have worked for beta 3, but it breaks down if we start supporting platforms that don't support various rmw implementations. Like if there's no opensplice on Debian Stretch armv7 but it's still listed in the explicit dependencies when bloomed that will fail.

@nuclearsandwich If I understand your proposal correctly the only difference to my comment is that your declares the group name in the export section where mine relies on the "group name" to be a package?

The explicit declaration of the group name has the advantage that e.g. a forked package my_ros1_bridge will still be able to utilize the dependency on all message packages.

Yes. The layer of indirection that the "group name" provides is, in my opinion, one of the major justifications for any proposal.

In a system with direct or reverse dependencies. Total knowledge is required by some either dependencies or dependents. In these proposals a group name acts as a key that can be connected so that neither dependency nor dependent needs complete knowledge of the other members of the system in order to directly relate to them.

@dirk-thomas
Copy link
Member Author

I only proposed use of the export section because it gives us license to trial and refine without needing to implement a spec change.

Putting information into the export section doesn't necessarily avoid a spec change. If tools need to honor the information they need to know about the new tags in order to function as expected.

@dirk-thomas
Copy link
Member Author

This would have worked for beta 3, but it breaks down if we start supporting platforms that don't support various rmw implementations. Like if there's no opensplice on Debian Stretch armv7 but it's still listed in the explicit dependencies when bloomed that will fail.

That problem is orthogonal to the group dependencies. Neither "normal" dependencies nor group dependencies offer per-platform customization.

@dirk-thomas
Copy link
Member Author

Thank you for all the feedback on this. I will close this in favor of ros-infrastructure/rep#138 since it will cover the spec in a ROS version agnostic way so we don't need a ROS 2 specific design document.

@dirk-thomas dirk-thomas deleted the dependency_groups branch October 26, 2017 17:09
@dirk-thomas dirk-thomas removed the in review Waiting for review (Kanban column) label Oct 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants