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

Convert to package format 2 #591

Merged
merged 4 commits into from Aug 1, 2017

Conversation

mikaelarguedas
Copy link
Contributor

No description provided.

@@ -1,4 +1,6 @@
<package>
<?xml version="1.0"?>
<?xml-model href="http://download.ros.org/schema/package_format2.xsd" schematypens="http://www.w3.org/2001/XMLSchema"?>
Copy link
Member

Choose a reason for hiding this comment

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

Honest question: since when is this a thing?

@mikaelarguedas
Copy link
Contributor Author

Honest question: since when is this a thing?

I'm not sure if the question applies to format 2 or the fact of providing a reference to the schema in an xml header. So I'll try to answer both, keeping in mind that both preceded my involvement with ROS or xml parsers.

package format 2 has been introduced in 2013-2014 to provide a format more adapted to the catkin build system, some advantages being being less verbose than the first version by adding the <depend> tag to merge <build_depend>, <exec_depend> and <build_export_depend> into a single tag. Another is to aggregate every dependency type in the package.xml so that it's a single place to get all dependencies of a package, including <doc_depend> for example. The full definition of the format is available in REP140

xml schema : AFAIU it is recommended since 2001 to provide a formal definition in xsd format to allow to write/generate complete parsers for a specific xml format.
Embedding it in the xml file is not required given that catkin already has a knowledge of the format and a parser for it. However it provides a self contained description of the format so that a package.xml is machine-readable by any tool able to fetch and interpret an xml schema without requiring any external knowledge of ROS or its tools.
As of the "when" it looks like the xsd files for both formats have been added in 06/2016 to the REP and the download resources, but the notion has been around for a while in the xml world.

Hope this provides more context.

Note: I understand that this PR may conflict with numerous existing ones and may stay on hold for a bit.

@nuclearsandwich
Copy link

@ros-pull-request-builder retest this please.

@mikaelarguedas
Copy link
Contributor Author

looks like CI is failing because base_local_planner declares a CATKIN_DEPENDS on message_generation. I fixed it in 0e941d8 but I'm not sure why this dependency is exported to downstream packages in the first place. If it's not a requirement I'm happy to update this PR accordingly

@mikaelarguedas
Copy link
Contributor Author

@mikeferguson I just rebased this PR on latest lunar. I'm still unsure about #591 (comment), let me know if their is any change necessary

@mikeferguson
Copy link
Contributor

I'm aiming to get one more thing in before this: #596 (which brings forward some of the same fixes you have here, but also fixes a number of CMake issues).

@mikeferguson mikeferguson self-assigned this Aug 1, 2017
@mikeferguson
Copy link
Contributor

Ok, I'm done totally destroying your pull request for a little while -- if you can rebase (and maybe there might be some fixups required?) we can get this in next.

@mikaelarguedas
Copy link
Contributor Author

rebased and resolved conflict. CI has been interrupted a few times but looks promising so far

@mikeferguson mikeferguson merged commit ab9312c into ros-planning:lunar Aug 1, 2017
@mikeferguson
Copy link
Contributor

Thanks! I'll do a new release into lunar in the morning

@mikaelarguedas mikaelarguedas deleted the package_format2 branch August 1, 2017 14:30
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.

None yet

4 participants