Example screenshot:

There seems to be some bug with the Comment field though, somehow in Dolphin the comment is not shown, where "dump" displays it as existing.

Cool!

Can you add a test please?

In D18450#397920, @kossebau wrote:

There seems to be some bug with the Comment field though, somehow in Dolphin the comment is not shown, where "dump" displays it as existing.

That property "conflicts" with the xattr comment and is excluded in baloo-widgets because of that.

In D18450#398143, @astippich wrote:

Can you add a test please?

Done. The sample file sadly is some 2xx KB big, but the AppImage devs could not help to get it smaller without no longer being a real appimage, the runtime payload brings that weight.

In D18450#397920, @kossebau wrote:

There seems to be some bug with the Comment field though, somehow in Dolphin the comment is not shown, where "dump" displays it as existing.

That property "conflicts" with the xattr comment and is excluded in baloo-widgets because of that.

I see. Guess it still makes sense to extract something for the Comment field, for any other consumers or the future.

I am having troubles getting it to build (Kubuntu 18.10). Unfortunately, I could not find pre-build packages for libappimage. I have overcome two small issues in building libappimage, but now I can't get it to work in KFileMetaData because cmake complains that LIBAPPIMAGE_BINARIES is not set. Is that me doing something stupid? Anyway, I cannot test

In D18450#400865, @astippich wrote:

I am having troubles getting it to build (Kubuntu 18.10). Unfortunately, I could not find pre-build packages for libappimage. I have overcome two small issues in building libappimage, but now I can't get it to work in KFileMetaData because cmake complains that LIBAPPIMAGE_BINARIES is not set. Is that me doing something stupid? Anyway, I cannot test

"LIBAPPIMAGE_BINARIES"? I am not aware of that variable in this patch or libappimage. @astippich what have been your issues exactly you need to overcome, and how did you overcome them?

BTW, if the build system is broken here with the self-built version you tried with, it is also broken in kio-extras, where libappimage is used to extract the icon by the appimage thumbnailer plugin, where the same logic is used.
@broulik As author of the appimage thumbnailer, any hints from your side (or testing :) )?

In D18450#400865, @astippich wrote:

I am having troubles getting it to build (Kubuntu 18.10). Unfortunately, I could not find pre-build packages for libappimage. I have overcome two small issues in building libappimage, but now I can't get it to work in KFileMetaData because cmake complains that LIBAPPIMAGE_BINARIES is not set. Is that me doing something stupid? Anyway, I cannot test

I have no clue what LIBAPPIMAGE_BINARIES is, but it looks like a CMake internal variable. Can you please open an issue on GitHub? Then, the AppImage team can help you.

The first issue is when BUILD_TESTING is not set to false, cmake fails with
CMake Error at lib/CMakeLists.txt:8 (add_subdirectory):

The source directory

  ~/Code/libappimage-0.1.8/lib/gtest

does not contain a CMakeLists.txt file.

Second issue was a missing check for libfuse, otherwise it would fail during compiling (squashfuse I think).

The LIBAPPIMAGE_BINARIES cmake variable is used in the target_link_libraries call in the autotest and extractor cmake file.

I will open issues on github.

@astippich Thanks for the update. I see that myself I built libappimage with USE_SYSTEM_LIBARCHIVE:BOOL=ON, USE_SYSTEM_SQUASHFUSE:BOOL=OFF, USE_SYSTEM_XZ:BOOL=ON, BUILD_TESTING:BOOL=OFF (cannot remember why I configured the build that way when I initially set it up some months ago, possibly ran into issues as well, at least I remember issue when BUILD_TESTING was ON.
Update: and with those settings the build of libappimage works for me on openSUSE TW (do not remember though which packages I had to install explicitly).

You can use a system Google test installation by setting -DUSE_SYSTEM_GTEST=ON, IIRC. But it's easier to simply git submodule --init --recursive.

LIBAPPIMAGE_BINARIES (or, more likely, LIBAPPIMAGE_LIBRARIES) doesn't exist, maybe it existed for a short while but got deleted. @kossebau can you please link to libappimage directly?

In the future Debian packages we'll introduce pkg-config support by installing a .pc file, which makes linking to libappimage even easier.

Given there are now released versions of libappimage which provide cmake config files, I will now update this patch to simply only support those. Distributions will be encouraged to also get the latest version of libappimage anyway.
(I had for the start simply copied the cmake code for finding libappimage from kio-extra, though that was written at a time when the released versions of libappimage had not yet cmake config files with easy to use imported target defined, that's why the complicated logic there)

In D18450#402175, @TheAssassin wrote:

LIBAPPIMAGE_BINARIES (or, more likely, LIBAPPIMAGE_LIBRARIES) doesn't exist, maybe it existed for a short while but got deleted. @kossebau can you please link to libappimage directly?

Oh sorry, just noticed that I wrongly and consistently wrote LIBAPPIMAGE_BINARIES while I meant LIBAPPIMAGE_LIBRARIES, that may have caused some confusion.

I will hopefully find some time tonight or tomorrow to test this again

got it up and running!
One question: Why is the extra desktop file parser necessary? Shouldn't all required information be available from the app data parser?

In D18450#402952, @astippich wrote:

got it up and running!

Yay :) Thanks for keeping trying.

One question: Why is the extra desktop file parser necessary? Shouldn't all required information be available from the app data parser?

Reasons are that for one, the appdata file is optional by the appimage spec, so one needs a fallback. Then other appimage handling tools also give the data from the desktop file a priority over the data from the appdata file (the desktop file being the older metadata container in the history of the spec, so also sometimes completely ignored), so for consistency I followed that priority handling for data from the desktop file.

In D18450#402995, @astippich wrote:

Just noticed, you never use the AppDataParser.name(). Is that intentional?

Good catch, was left over from first code drafts, before I aligned the data mapping with what I have seen in other appimage code. Removing.

Otherwise looks good, but you may want to wait for someone more experienced than me.

Thanks for review. I tried to poke some others, but seems no-one has something negative to point out. So I would go and push soon.
Only blocker I added myself here is to have a released version of libappimage which has fixed version info in the cmake config, so we can reliable build against what is found.

https://github.com/AppImage/libappimage/pull/71 is a recent proposal to get to that.