ament / ament_lint

Apache License 2.0
37 stars 107 forks source link

xmllint Requires member_of_group to be before export #496

Closed erichlf closed 2 months ago

erichlf commented 3 months ago

When I run xmllint with my packages.xml having the following order

  <export>
    <build_type>ament_cmake</build_type>
  </export>

  <member_of_group>rosidl_interface_packages</member_of_group>

I get the following error

package.xml:31: element member_of_group: Schemas validity error : Element 'member_of_group': This element is not expected.

However, simply changing things to the following

  <member_of_group>rosidl_interface_packages</member_of_group>

  <export>
    <build_type>ament_cmake</build_type>
  </export>

causes this to go away. Is it intended that the order matters here?

christophebedard commented 3 months ago

I believe the order matters, yes. Same for the order of <version>/<description>/<maintainer>/<license>/etc. See the schema: http://download.ros.org/schema/package_format3.xsd

erichlf commented 3 months ago

Yep looks like it does matter.

gavanderhoorn commented 3 months ago

@christophebedard: order matters because it seems the XSD uses a xs:sequence for the package element description.

But it's not clear to me why.

christophebedard commented 3 months ago

Consistency, probably?

gavanderhoorn commented 3 months ago

I've looked through the history of the files, but can't really find any comments on this. Dirk appears to 'just' commit the .xsds, and that's that.

christophebedard commented 3 months ago

Yeah, and the REPs (149 for format 3 or 140 for format 2) don't seem to explicitly mention that the elements have a specific order either.

I know that it is generally accepted -- or, rather, known -- that <maintainer> goes before <license>, which goes before <author> (otherwise the linter complains), but I've never seen any rationale for it.

gavanderhoorn commented 3 months ago

I remembered the original discussion on ros-users that was the start of the schemas: [ros-users] review: catkin documentation review for package.xml format 2, but there's also no mention of any explicit order there.

Perhaps the original author just used sequence and it was never really discussed.

I know that it is generally accepted -- or, rather, known -- that <maintainer> goes before <license>, which goes before <author> (otherwise the linter complains), but I've never seen any rationale for it.

I do believe that's mostly just convention.

fujitatomoya commented 3 months ago

@erichlf

we have talked about this a bit in today's ROS 2 team meeting. this is more like a question not an issue, besides this is how this works atm, and currently no plan to change the behavior. so if that is okay, we can close this for now.

if we escalate this as an issue, feel free to open this to track.