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

news-to-metainfo: --limit=N should parse only N release notes #360

Closed
fedelibre opened this issue Sep 27, 2021 · 9 comments
Closed

news-to-metainfo: --limit=N should parse only N release notes #360

fedelibre opened this issue Sep 27, 2021 · 9 comments

Comments

@fedelibre
Copy link

AFAICS the command appstreamcli news-to-metainfo --limit=2 NEWS METAINFO.in METAINFO.xml parses the whole NEWS file and then uses only the information retrieved in the latest two release entries to copy them in the metainfo file.

As a result, when you try to introduce this script in a project with a long history and a long NEWS file (not compliant with the format appstreamcli expects to find), it seems the best solution is moving the old NEWS to NEWS.old and then write a new NEWS file from scratch. Fixing all the old release entries is not worth the effort.

I'm wondering if my analysis is correct or if I'm missing something.
What if --limit may be adjusted to parse only the first N occurencies of Version in the NEWS file so we can fix the format of the latest releases only and keep the history in a single file?

@ximion ximion closed this as completed in 05f9b1c Oct 6, 2021
@ximion
Copy link
Owner

ximion commented Oct 6, 2021

Please test the appstreamcli version from master and see if this change works for you :-)

@fedelibre
Copy link
Author

Thanks for the quick fix!

I have a patch ready to test it, but the binary I compiled in my ~/.local prefix has some problems:

⬢[fede@toolbox appstream]$ which -a appstreamcli
~/.local/bin/appstreamcli
⬢[fede@toolbox appstream]$ appstreamcli --version
appstreamcli: error while loading shared libraries: libappstream.so.4: cannot open shared object file: No such file or directory

I'm working in a toolbox container because I'm on Fedora Silverblue (immutable OS).

These are the commands I used to build appstream:

meson build --prefix=/var/home/fede/.local
ninja -C build
ninja install -C build

Any suggestions?

@fedelibre
Copy link
Author

Ok, I got it:

$ find ~/.local/lib64 -name libappstream.so.4
/var/home/fede/.local/lib64/libappstream.so.4

$ env LD_LIBRARY_PATH="/var/home/fede/.local/lib64" appstreamcli --version
AppStream version: 0.14.6

@fedelibre
Copy link
Author

It's not working. You can use this branch for testing:
https://gitlab.gnome.org/fedelibre/gthumb/-/tree/metainfo-automate-releases

The error I get is:

$ ninja -C build
ninja: Entering directory `build'
[1/2] Generating gen-output with a custom command
FAILED: data/appdata/nol10n_withrelinfo_org.gnome.gThumb.appdata.xml 
/var/home/fede/.local/bin/appstreamcli news-to-metainfo --limit=6 ../data/appdata/../../NEWS ../data/appdata/org.gnome.gThumb.appdata.xml.in data/appdata/nol10n_withrelinfo_org.gnome.gThumb.appdata.xml

(appstreamcli:25507): GLib-WARNING **: 13:02:55.117: GError set over the top of a previous GError or uninitialized memory.
This indicates a bug in someone's code. You must ensure an error is NULL before it's set.
The overwriting error message was: Unable to detect input data format.
Failed to detect section 'version 3.9.1
-------------'
ninja: build stopped: subcommand failed.

Version 3.9.1 is the 7th-last release and I'm using --limit=6, so it shouldn't be parsed.

@fedelibre
Copy link
Author

If I change to --limit=5 it builds correctly.
So currently the copied releases are N, but the parsing is made on N+1 releases.

@ximion
Copy link
Owner

ximion commented Oct 7, 2021

If I change to --limit=5 it builds correctly.
So currently the copied releases are N, but the parsing is made on N+1 releases.

Yes, this is expected behavior. In order to know when a previous release block has been completed (and to add the data as release) we need to be able to parse the next header, which signals termination of the release entry. There really is no good way around that...
You did find a nice GError override programming error though, which is also fixed now (so parsing errors should actually be shown properly now).

@fedelibre
Copy link
Author

Ok, it makes sense.

Do you know if you'll make a new release soon?

@ximion
Copy link
Owner

ximion commented Oct 7, 2021

Possibly tomorrow.

@ximion
Copy link
Owner

ximion commented Oct 8, 2021

It may actually also make sense to give appstreamcli a pkg-config file, so people can require a certain minimum version in their Meson/Automake/CMake/whatever build scripts...

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

No branches or pull requests

2 participants