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 support for depend tag #43

Closed
wants to merge 1 commit into from
Closed

Conversation

dirk-thomas
Copy link
Member

It is not exposed in the API but will be handled as a build_depend and run_depend after parsing.

@jbohren
Copy link
Contributor

jbohren commented May 29, 2013

👍

@tfoote
Copy link
Member

tfoote commented May 29, 2013

+1

patch looks good, we need to update REP 127 accordingly

@jack-oquin
Copy link

+1

I am willing to update REP-0127, if desired.

Will this change apply to Hydro only, or to Groovy as well?

@dirk-thomas
Copy link
Member Author

catkin_pkg is cross ROS distro - so it must apply to Groovy as well.

@jack-oquin
Copy link

OK. I don't see any problem with that, Dirk, but I needed to know for documentation purposes.

@wjwwood
Copy link
Contributor

wjwwood commented May 29, 2013

👍

1 similar comment
@flixr
Copy link
Contributor

flixr commented May 30, 2013

👍

@jack-oquin
Copy link

While discussing the catkin documentation update (ros/catkin#429), I asked if we should simplify the docs by recommending the use of <depend>, where appropriate.

Dirk responded:

@wjwwood suggested some time ago that we might not allow packages to be released when they use that tag since it is likely used as shortcut and might result in bloated dependencies. If that is the case all tutorials should still teach how to use the individual dependency tags.

Is that still your thinking, William?

If we don't want people to use <depend>, we should not add it.

@jbohren
Copy link
Contributor

jbohren commented May 30, 2013

I think it should go like this:

Use whenever you have both <build_depend> and <run_depend>
dependencies.

-j

On Thu, May 30, 2013 at 10:53 AM, jack-oquin notifications@gh.neting.ccwrote:

While discussing the catkin documentation update (ros/catkin#429ros/catkin#429),
I asked if we should simplify the docs by recommending the use of ,
where appropriate.

Dirk responded:

@wjwwood https://github.com/wjwwood suggested some time ago that we
might not allow packages to be released when they use that tag since it is
likely used as shortcut and might result in bloated dependencies. If that
is the case all tutorials should still teach how to use the individual
dependency tags.

Is that still your thinking, William?

If we don't want people to use , we should not add it.


Reply to this email directly or view it on GitHubhttps://github.com//pull/43#issuecomment-18685340
.

Jonathan Bohren
Laboratory for Computational Sensing and Robotics
http://dscl.lcsr.jhu.edu/People/JonathanBohren

@jbohren
Copy link
Contributor

jbohren commented May 30, 2013

Or alternatively:

Use . If you need more granular control and know what you're doing,
use <build_depend> and <run_depend> where appropriate.

-j

On Thu, May 30, 2013 at 11:07 AM, Jonathan Bohren <jonathan.bohren@gmail.com

wrote:

I think it should go like this:

Use whenever you have both <build_depend> and <run_depend>
dependencies.

-j

On Thu, May 30, 2013 at 10:53 AM, jack-oquin notifications@gh.neting.ccwrote:

While discussing the catkin documentation update (ros/catkin#429ros/catkin#429),
I asked if we should simplify the docs by recommending the use of
, where appropriate.

Dirk responded:

@wjwwood https://github.com/wjwwood suggested some time ago that we
might not allow packages to be released when they use that tag since it is
likely used as shortcut and might result in bloated dependencies. If that
is the case all tutorials should still teach how to use the individual
dependency tags.

Is that still your thinking, William?

If we don't want people to use , we should not add it.


Reply to this email directly or view it on GitHubhttps://github.com//pull/43#issuecomment-18685340
.

Jonathan Bohren
Laboratory for Computational Sensing and Robotics
http://dscl.lcsr.jhu.edu/People/JonathanBohren

Jonathan Bohren
Laboratory for Computational Sensing and Robotics
http://dscl.lcsr.jhu.edu/People/JonathanBohren

@jack-oquin
Copy link

From a documentation perspective, defining "where appropriate" is hard.

You have to start with a discussion of the fact that <run_depend> does not just mean a run-time dependency. By the time you finish explaining the transitive build dependency situation, many readers will have shrugged off the whole discussion.

William and Dirk must have explained the transitive dependency problem to me many times. I still didn't understand it, until after I tried to document it and got it all mixed up.

The main potential simplification is to recommend <depend> for all catkin ROS package dependencies, with a few exceptions like <build_depend>message_generation</build_depend> and <run_depend>message_runtime</run_depend>.

@dirk-thomas
Copy link
Member Author

Recommending to blindly use <depend> will result in a lot of wrong (aka bloated) dependencies. Imo we should not do that.

I think the tutorials should describe the build and run dependencies and be clear enough that a normal user can decide what he has to use. E.g. if you use the dependency in this way use a), if you use it in that way use b).

And after that it should be mentioned that dependencies listed as both can be written more efficiently as @jbohren mentioned above:

Use <depend> whenever you have both <build_depend> and <run_depend> dependencies.

@wjwwood
Copy link
Contributor

wjwwood commented May 30, 2013

Like I said before, whether it is the <depend> tag or a <run_depend> and <build_depend> tag pair, they are getting misused. I see the point that the <depend> tag might encourage misuse, but honestly I think it is sufficient to allow the releasing of packages using the <depend> and documenting how to use the other tags and then encourage people to use them. We can't force people to use <run_depend> and <build_depend> tags correctly anyways.

@jack-oquin
Copy link

Thanks for the feedback. I think I understand. We want the docs to describe "best practices", or at least "good practices".

For some tasks, it really does get simpler.

  • We have agreed that ROS message dependencies should always use both, so I can simply recommend using <depend> for them.
  • Metapackages have no build step, so they always use <run_depend>.
  • In many cases, Python dependencies are just <run_depend>.

But, other tasks remain complicated:

  • While there are very few catkin packages that can be used without both <build_depend> and <run_depend>, there are a few cases where one or the other works alone. So, unfortunately we cannot make a simplified recommendation for them all.
  • System dependencies really do supply finer-grained control, but developers must understand the gory details to use them optimally.

@dirk-thomas
Copy link
Member Author

I do think that the decision guidance can describe the stuff in such a detail that the user can easily follow it. They should be able to check against a list of bullet points what matches there scenario and derive the right way to define the dependency based on that. For example:

Your package must build_depend on another package if your package matches any of these:
* calls `find_package(catkin REQUIRED COMPONENT xxx)` for that dependency in CMake
* compiles C++ code which includes header files from that dependency
* links against a library from that dependency

Your package must run_depend on another package if your package matches any of these:
* creates an executable or library which is linked against a shared library from that dependency
* exports header files which them self #include headers form that dependency
* contains Python code which requires other Python code from that dependency

Higher level rules can that nicely be derived from that and argued for. E.g. message packages match bullets 1 and (usually) 2 for build_depend as well as bullets 2 and (eventually) 3 for run_depend.

@jbohren
Copy link
Contributor

jbohren commented May 31, 2013

That looks like a good policy, to me, and then point out that if you need
both, you can use the tag.
On May 30, 2013 7:58 PM, "Dirk Thomas" notifications@github.com wrote:

I do think that the decision guidance can describe the stuff in such a
detail that the user can easily follow it. They should be able to check
against a list of bullet points what matches there scenario and derive the
right way to define the dependency based on that. For example:

Your package must build_depend on another package if your package matches any of these:

  • calls find_package(catkin REQUIRED COMPONENT xxx) for that dependency in CMake
  • compiles C++ code which includes header files from that dependency
  • links against a library from that dependency

Your package must run_depend on another package if your package matches any of these:

  • creates an executable or library which is linked against a shared library from that dependency
  • exports header files which them self #include headers form that dependency
  • contains Python code which requires other Python code from that dependency

Higher level rules can that nicely be derived from that and argued for.
E.g. message packages match bullets 1 and (usually) 2 for buid_depend as
well as bullets 2 and (eventually) 3 for run_depend.


Reply to this email directly or view it on GitHubhttps://github.com//pull/43#issuecomment-18716075
.

@jack-oquin
Copy link

Thanks, Dirk, for that helpful list of decision factors.

While they probably cover at least 2*sigma of the probability distribution, I am pretty sure there are exceptions not covered by those rules.

I suppose the real underlying rules were dictated by implementation concerns:

  • <build_depend> means that package is directly needed to build yours
  • <run_depend> means that binary package is a direct dependency of your binary package

@dirk-thomas
Copy link
Member Author

if there are cases left which are not addressed by the bullets I put together out of my head than we should add them as soon as we recognize them. I don't think that there are any "magic cases" - they should be all straight forward logic decisions (even if the reason is not clear for the average user - thats why we want that list in the first place).

@dirk-thomas
Copy link
Member Author

Well at some point the dependencies from package.xml must be utilized in the process of building the (Debian) package and installing it.<build_depend> and <run_depend> seem to be the natural mapping to these two use cases. The first get installed when configuring and building a package, the second get mapped to the Debian package dependencies which implies when someone depends on this package (for runtime or buildtime of a downstream package).

If anyone can propose an alternative we are always open to hear that. But until now nobody ever proposed an alternative (except naming these two differently).

If we want to split them into more semantic dependencies in order to use them in CMake automatically I would also like to discuss options in that direction. This is the major thing I am worried about when adding <depend> - we do not have a clear picture where we want to go in the future (dev/non-dev packages, semantic dependencies so that CMake can do more automatically).

@jack-oquin
Copy link

There were several proposals. Mine was to separate <run_depend> into its two components: actual run-time dependencies and dependencies for using the -devel package, call it <devel_depend>.

That never went anywhere. What's the point when we have no clear road-map for creating separate binary and devel packages, except by hand? It is just not a big priority.

Most things should not be designed before the problem is understood in adequate detail. Prototyping is good for that, but only if there is sufficient urgency.

@jbohren
Copy link
Contributor

jbohren commented May 31, 2013

On Fri, May 31, 2013 at 1:29 PM, Dirk Thomas notifications@gh.neting.ccwrote:

Well at some point the dependencies from package.xml must be utilized in
the process of building the (Debian) package and installing it.
<build_depend> and <run_depend> seem to be the natural mapping to these
two use cases. The first get installed when configuring and building a
package, the second get mapped to the Debian package dependencies which
implies when someone depends on this package (for runtime or buildtime of a
downstream package).

If anyone can propose an alternative we are always open to hear that. But
until now nobody ever proposed an alternative (except naming these two
differently).

I proposed alternatives including one where the tags were all
mutually-exclusive and prevented people from duplicating dependencies in
the package.xml file.

If we want to split them into more semantic dependencies in order to use
them in CMake automatically I would also like to discuss options in that
direction. This is the major thing I am worried about when adding - we do not have a clear picture where we want to go in the future
(dev/non-dev packages, semantic dependencies so that CMake can do more
automatically).

I understand this concern, but frankly, the majority of ROS packages that
are written will not be built into debian packages and will not necessitate
building dev/non-dev packages. This isn't because building debs is hard,
it's because vetting and stabilizing APIs for release is hard, time
consuming, and is not a very good use of time in a research environment.

@dirk-thomas
Copy link
Member Author

Imo it is always hitting you back if you make design / spec decision without having a clear perspective where you wanna go. So even if it is not a priority to implement a feature you should have a clear vision on how that should look like.

@jbohren catkin and the dependencies in the package.xml can not only target the research environment. It must deal with these difficult dependency question since there is a strong request towards having first-class Debian (and other) packages at some point in the future. The difficult task will be to address this need will still keeping it easy enough for the research environment.

Regarding the <devel_depend> (@jack-oquin) or the "mutually-exclusive dependencies" (@jbohren): for a decent discussion of an alternative it must be much more concrete. How are these new dependencies used in the various processes, what does it get mapped to, etc.? I would appreciate if you could write that down and send a more detailed description about these to the mailing list. That would show us more options where we could go.

@jack-oquin
Copy link

Dirk:

I think we both did that already. It went nowhere, which may have been appropriate.

I've got more productive things to do than make detailed proposals we are not likely to implement. For example, I can keep trying to document the design we already implemented and deployed.

@dirk-thomas
Copy link
Member Author

After searching again I only find these references to <devel_depend>: #43 (comment)

That describes only that there should be a separate tag for "-dev" run dependencies. It does not go into any details: does it mean it requires a dependency to be specified twice (one run_depend on the non-dev package, one devel_depend on the dev package), what does it mean for something like installing all dependencies in order to build a package in rosdep and the buildfarm?. Please point me to further information if I just can't find them in the large set of mailing lists and issue trackers.

Regarding the "mutually-exclusive tag" mentioned here: https://groups.google.com/d/msg/ros-sig-buildsystem/wbQZg_Y0igY/DibqLhi0DrUJ

It described that the four tags "build_depend", "run_depend", test_depend" and "depend" should be mutually exclusive. What exactly is the difference between that and this pending pull request adding the depend tag? Without any further explanation I just can't understand it.

So if anyone has an interest of figuring out alternative than he need to provide more information and details so that others can contribute to the discussion and make progress on the topic.

@jbohren
Copy link
Contributor

jbohren commented Jun 1, 2013

On Fri, May 31, 2013 at 6:03 PM, Dirk Thomas notifications@github.com wrote:

Regarding the "mutually-exclusive tag" mentioned here: https://groups.google.com/d/msg/ros-sig-buildsystem/wbQZg_Y0igY/DibqLhi0DrUJ

It described that the four tags "build_depend", "run_depend", test_depend" and "depend" should be mutually exclusive. What exactly is the difference between that and this pending pull request adding the depend tag? Without any further explanation I just can't understand it.

No, it described the following, if you read the original message [1], and it's a pretty simple concept:

<build_depend> and <run_depend> ---> <depend> 
<build_depend> and not <run_depend> ---> <internal_depend>
<run_depend> and not <build_depend> ---> <exec_depend>

[1] https://groups.google.com/d/msg/ros-sig-buildsystem/wbQZg_Y0igY/_7ionDPHzhwJ

@dirk-thomas
Copy link
Member Author

Thanks for clarifying your proposal. That maps to exactly what we do have with the <depend> tag introduced in this pull request plus renaming build to internal and run to exec.

In the discussion we had the argument (fro multiple people) that none of the terms (neither the current one nor your proposed renamed ones) cover exactly the semantic of the dependencies. Dependencies listed as exec dependencies could be packages which provide header files only and are necessary to build downstream packages, which is not more intuitive when being called exec imo.

I am not even sure if it is possible to find a term which would make the semantic crystal clear without any further documentation.

@jbohren
Copy link
Contributor

jbohren commented Jun 1, 2013

The point isn't just to rename them, it's to enforce that any dependency is
only listed once in the package.xml. That is very different than the
current PR. I don't understand what was so unclear about this when I
originally described it in what I thought was a succinct manner.

Regardless, I don't know if this is the best way to do it, but don't say
people haven't been giving input.

Furthermore, I'm not going to spend my time building a fully fleshed out
prototype and design specification and unit tests for every design idea
because I feel like we should be able to talk about the impacts of high
level ideas before driving depth-first into an implementation.

@dirk-thomas
Copy link
Member Author

With this PR we do have the four tags you mentioned: build_depend, run_depend, depend and test_depend. As far as I can see they are mutually exclusive - at least the code will give you an error when depend is duplicated with one of the build or run dependencies (https://github.com/ros-infrastructure/catkin_pkg/pull/43/files#L0R404). For test dependencies that was already the case before (https://github.com/ros-infrastructure/catkin_pkg/blob/master/src/catkin_pkg/package.py#L396).

The only case where it is not possible to enforce exclusive tags currently is build and run duplicates - simply because that would break nearly every package currently out there. What we could do is to add a deprecation warning in the case of duplicate build and run dependencies and tell the user to move to a single depend tag. I can add that to the PR if that is what people want to have (but bear in mind that this would generate huge lists of deprecation warnings on any kind of command which queries catkin_pkg for quite some time).

That makes it look like what your proposal is about or are there any other aspects I haven't considered?

@jbohren
Copy link
Contributor

jbohren commented Jun 1, 2013

The only case where it is not possible to enforce exclusive tags currently is build and run duplicates - simply because that would break nearly every package currently out there.

This is why I proposed tags with different names since they would have different meaning and could be introduced slowly and since we were already discussing making the nomenclature clearer to new users.

Regardless, like I said before, this PR looks good, thank you.

@jack-oquin
Copy link

I see no advantage to deprecating the most common current usage, which is both <build_depend> and <run_depend>.

Unfortunately, if we accept that they are not mutually exclusive, the main point of Jon's proposal is lost.

I guess I'm OK with that, I don't consider the various types of dependencies logically exclusive, anyway. A package can depend on another for multiple reasons.

My suggestion (I won't call it a proposal) had no mutual exclusion. Rather, it characterized each major type of dependency according when and where it comes into play.

  • <build_depend> is used for building the package. It corresponds to the Debian Build-Depends tag. No change there.
  • <run_depend> is a problem if we generate separate binary and devel packages from a single ROS package. A true "run" dependency would correspond to a Debian Depends for the binary package. I suggested restricting it to just that meaning, since most people will assume that interpretation. As that is obviously an API-breaking change, I asked not to be taken literally about that, but it probably confused everyone anyway. We would actually need a new name for that more-restricted definition, for now I'll call it <binary_depend>.
  • <devel_depend> was the new name I made up to describe the other meaning of our current <run_depend>. It corresponds to a Debian Depends for the devel package. This describes the transitive dependencies that must be provided when other packages build using interfaces provided by the devel package.

With this approach, some tags logically imply others. Using <run_depend> in its current meaning, and the newly-coined <binary_depend> for the more-restrictive interpretation, we have:

  • <depend> ::= <build_depend> | <run_depend>
  • <run_depend> ::= <devel_depend> | <binary_depend>

@dirk-thomas
Copy link
Member Author

Thank you, Jack, for the detailed description. I will try to understand it and the consequences before considering to merge this pull request because I think sketching how we could do the dev/non-dev stuff will help us to make a better decision on the <depend> tag now. I will post some feedback in the next days.

@jack-oquin
Copy link

While I am always reluctant to design features for things we may never do, I think there is some merit to giving official names to <devel_depend> and <binary_depend>. (Better tag name suggestions are welcome.)

Having them defined makes it easier to document what happens, so users can understand it better. Even if we never implement separate binary and devel packages, I would prefer to describe my own dependencies in that form, anyway, because it just seems clearer.

@jack-oquin
Copy link

First draft of a REP-0127 update available.

@jack-oquin
Copy link

Perhaps a summary of the lengthy <depend> discussion should be added to the Rationale section.

Opinions?

@jack-oquin
Copy link

We made a consensus decision to always recommend <depend> for ROS message package dependencies.

Python code generally only requires a <run_depend>, except in unusual cases like dynamic reconfigure scripts. But I plan to stick with the generic recommendation even there.

Any objections?

@dirk-thomas
Copy link
Member Author

Superseded by #88.

@dirk-thomas dirk-thomas deleted the general-depend-tag branch March 17, 2014 16:59
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