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 a mode to allow absent "date" for future releases #398

Closed
Jehan opened this issue May 3, 2022 · 7 comments
Closed

Add a mode to allow absent "date" for future releases #398

Jehan opened this issue May 3, 2022 · 7 comments

Comments

@Jehan
Copy link
Contributor

Jehan commented May 3, 2022

Since commit c13a605, appstreamcli is complaining on an absent date which makes complete sense for the finale file validation. Nevertheless we usually create the <release> tag as early as possible in advance. One of the reasons is that the <description> can be (and should be!) localized so we need to leave time to translators by filling the release text as soon as we know it.

At this point, we might not know the release date yet. Even if we had a global timeframe, it will likely shift as we discover last-minute issues. We could just set a random date, but it means that (1) we need to edit it again, possibly several times, as we get closer to the release, making useless commits and (2) the worst case is that if we forget to edit the date (because the appstreamcli test didn't return any error), we might release with wrong release date in metadata.

On the other end, if we were allowed to ignore the date with a specific relaxed check, then when we are preparing our actual release, the test would use the stricter check requiring a date. This way, we can prepare the <release> tag without knowing the date even long in advance, and still get the date validation upon release. This is possible in GIMP (my use case) since we use even micro version for release versions and odd micro versions for dev code.

So for instance, most of the time, our build would test our XML, ignoring the release date, with:

appstreamcli validate --relax desktop/org.gimp.GIMP.appdata.xml

And when we bump the micro version, preparing for the release, it would test with:

appstreamcli validate desktop/org.gimp.GIMP.appdata.xml

Hence allowing us to spot the missing date (much better than having a random date we set long ago which would not trigger any kind of warning).
For the record, appstream-util had the validate-relax mode which was doing just that (among other differences), i.e. ignoring the <release> date.

Thanks!

gnomesysadmins pushed a commit to GNOME/gimp that referenced this issue May 3, 2022
appstreamcli on CI fails with error:

> E: org.gimp.GIMP:394: release-time-missing date

I didn't have such error on my local build, because it is a new check
only recently added. The CI appstreamcli errored-out this way.

It forces us to add a bogus release date since we don't know it yet. I
chose May 29, because we usually release on Sundays, and it might soon
be time to make a dev release (so who knows, we might get lucky and it
might happen this day), though it should not be taken as a promise or a
real plan just now. I just needed to fill something in, in order to fix
the CI.

I opened a report to ask that appstreamcli proposes a relaxed check mode
where we can test on the <release> tag before knowing the future date:
ximion/appstream#398
@ximion ximion closed this as completed in db2f51d May 3, 2022
@ximion
Copy link
Owner

ximion commented May 3, 2022

I'm not a fan of relaxed checks in general - either a file is valid or it isn't. Usually there's a smarter way to solve these issues, and appstreamcli will at some point gain the ability to have users increase (or decrease, but only for a select few) the severity of select issue tags, which would also solve issues like this.

Here, I think the best solution would be to simply relax this check for development releases, and verify that a date is present with an instant validation failure once the release is a stable release.
Does that approach work for you?

@Jehan
Copy link
Contributor Author

Jehan commented May 4, 2022

either a file is valid or it isn't

Sure I agree. And the fact is that our AppStream file is not valid at this point, but unless we invent time travel, we can't do anything about it. We still want to be able to translate the content as early as possible, and we also want to catch other types of bugs in this file as soon as we create them (but this specific bug, the missing date, is just a circumstantial bug which can't be fixed, at least not in our current very bazaar-style organization which comes with the fact everything is volunteer-managed). So basically there are times where we want to accept a partial-invalidity while catching others.

Usually there's a smarter way to solve these issues

Here with appstreamcli, I saw only one (in current condition), which would be to grep out the "^E: .*release-time-missing date" line, then grep remaining ^E: and ^W: lines (hopefully the output format won't ever change then!) to check if we have other errors or warnings, not caring about appstreamcli return value. I'm not sure I would call it "smarter", but "uglier" definitely. 😜

I'd prefer the tool to acknowledge the fact that projects don't necessarily know their release date in advance.

Here, I think the best solution would be to simply relax this check for development releases

It is not related to development releases at all. It is actually even truer for stable releases since we will want the <release> tag to be available all the more in advance to get proper translations for stable releases (we are less picky on missing translations for dev releases). :-)

Maybe you got confused by the odd/even versionning scheme since I was saying an odd micro version is for dev code. It's not "dev code" in the same meaning as <release type="development"/>. It's dev code as "unreleased" code meaning. For instance our stable release GIMP 2.10.32 has an internal dev version 2.10.31 but the <release> tag is versionned 2.10.32 and with type="stable", and we want this tag to be available as soon as possible, long before we even know the release date.

Anyway it's all not so relevant here. All this to say, what I am asking has nothing to do with the release type.

Does that approach work for you?

So no, this doesn't work, regarding the type differentiation. Don't do a different check depending on release type.

This being said, it doesn't have to be a generic relaxed check. I don't mind if we have a flag to only relax a specific check. It's cool with me. Like --check-version-relax or whatever. What I want is to (1) Check everything but the date as long as we are in dev mode and (2) Check everything, including release date when we enter release mode. Our build system knows when we enter these modes (since it's carved in our versionning scheme), all we need is a flag to add to appstreamcli call.

@ximion ximion added this to the AppStream 1.0 milestone May 5, 2022
@ximion ximion reopened this May 5, 2022
@ximion
Copy link
Owner

ximion commented May 5, 2022

Looks like you'll need to wait a bit for the tag override system to be implemented. This isn't the first thing on my todo list, but it's actually quite high up there, so might even be part of the next release.

@Jehan
Copy link
Contributor Author

Jehan commented May 5, 2022

Awesome. By the way, I would suggest to revert the last commit (db2f51d) for the time being. As said previously, I don't see why type="development" release should allow absent date as a general rule. When we enter releasing mode in our dev process, we want appstreamcli to go full-validating and tell us about the missing date (at this point, the release is near and we know the date).

@ximion
Copy link
Owner

ximion commented May 13, 2022

Here with appstreamcli, I saw only one (in current condition), which would be to grep out the "^E: .*release-time-missing date" line, then grep remaining ^E: and ^W: lines (hopefully the output format won't ever change then!) to check if we have other errors or warnings, not caring about appstreamcli return value. I'm not sure I would call it "smarter", but "uglier" definitely.

Just to add to that, I think that's a terrible idea in any form, and the output format of appstreamcli shouldn't be parsed anyway (even though it hasn't changed for such a long time that I don't even remember when it was last touched. However, validate has a YAML output mode that some projects use for better CI integration. That one is easily parseable and should be stable too, just pass --format=yaml :-)
Still, to fix this issue please rather wait for the proper fix. Yet, for other purposes this output mode might be useful ;-)

@Jehan
Copy link
Contributor Author

Jehan commented May 13, 2022

Just to add to that, I think that's a terrible idea in any form, and the output format of appstreamcli shouldn't be parsed anyway

I do agree.

Still, to fix this issue please rather wait for the proper fix. Yet, for other purposes this output mode might be useful ;-)

Right now, I don't use my grep hack, this was mostly to demonstrate how bad it was right now to handle this case (which is a normal case of software development, it's not like our situation of not knowing our release date is a really exceptional case!).

For the time being, I just used a random date, though I consider this "hack" just as bad, maybe even worse than the grep hack (because in worst case scenario, since it hides the issue, it might be possible that we forget to fix the date at release-time, hence we release with wrong metadata). So now that you tell me about this --format=yaml option, maybe I will switch to using this instead if the proper fix takes long (because I really don't like the idea of writing and committing random dates in our repo each time we prepare a new release).

@ximion ximion closed this as completed in 8fa860c May 18, 2022
@ximion
Copy link
Owner

ximion commented May 18, 2022

Try appstreamcli validate --override=release-time-missing=info /path/to/your/app.metainfo.xml ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants