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

appstreamcli validate can’t be used on components with merge="append" #455

Closed
pwithnall opened this issue Jan 18, 2023 · 3 comments
Closed
Labels

Comments

@pwithnall
Copy link
Contributor

I’d like to validate this file in CI. Its components are of the form:

  <component merge="append">
    <id>app.drey.Dialect</id>
    <custom>
      <value key="GnomeSoftware::FeatureTile">True</value>
    </custom>
  </component>

I believe that’s valid.

However, when I run appstreamcli validate --no-net ./org.gnome.Apps.Featured.xml on it, I get a lot of errors of the form:

E: app.drey.Warp:~: component-summary-missing
E: org.gnome.DejaDup:~: component-summary-missing
E: com.github.geigi.cozy:~: component-summary-missing
E: com.github.hugolabe.Wike:~: component-name-missing

I assume I wouldn’t get these errors if the components had something to merge into, but since I’m validating this file on its own, rather than after composing with other AppStream files, there is nothing to merge the components into.

Would it be possible to elide errors about missing required elements (apart from <id>, which is always required) for components with merge="append" please? Or have I misunderstood how merging works (which is entirely possible)?

@ximion ximion added the bug label Jan 18, 2023
@ximion
Copy link
Owner

ximion commented Jan 18, 2023

No, this should work exactly as you describe, the only tag that must be present for merge components is <id/> - so this is just the validator not validating correctly (I think it did have support for this a while back though, so maybe it broke and simply was never fixed since validating merge components is ultra-rare. Nevertheless, it's a bug that should be fixed.)

I'm currently travelling but can have a look on the weekend or next week :-)

@pwithnall
Copy link
Contributor Author

Aha, OK, good to know that it’s meant to be supported. Getting it fixed would be nice, but from my point of view it’s a fairly low priority bug, so please don’t feel pressured to fix it soon. Unfortunately I don’t have time to look into it myself, sorry

@ximion
Copy link
Owner

ximion commented Jan 22, 2023

It is low-priority, but also an easy fix, and changes like these take a long time to percolate through the ecosystem, so it's nice to have it resolved with the next release so people can start to rely on it (although admittedly for this feature the number of projects that need it will probably be below 5 ^^)
Should be fixed now, thank you for the detailed issue report!

@ximion ximion closed this as completed in d3d1b59 Jan 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants