Add extractor for AppImage files
ClosedPublic

Authored by kossebau on Jan 22 2019, 9:55 AM.

Details

Summary

Only a few properties currently can be mapped more or less to the existing
values of the "Property" enum, ideally for the future can be extended.

Test Plan

"dump" test util and Dolphin both now show the extracted metadata for
AppImage files.

Diff Detail

Repository
R286 KFileMetaData
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
kossebau created this revision.Jan 22 2019, 9:55 AM
Restricted Application added projects: Frameworks, Baloo. · View Herald TranscriptJan 22 2019, 9:55 AM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
kossebau requested review of this revision.Jan 22 2019, 9:55 AM

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.

kossebau updated this revision to Diff 50073.Jan 22 2019, 4:49 PM
  • switch and use any localized versions found matching the current system locale, should be more expected
  • also extract appdata <description> and expose as plain text, even though that can be quite some data, but that's the UI's task to handle that
kossebau updated this revision to Diff 50077.Jan 22 2019, 5:58 PM
  • skip getting the unlocalized description if there is a localized one

Can you add a test please?

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.

kossebau updated this revision to Diff 50146.Jan 23 2019, 9:34 PM

add unit test

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.

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

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 :) )?

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.

kossebau added a comment.EditedJan 29 2019, 6:47 PM

@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).

TheAssassin added a comment.EditedJan 29 2019, 11:00 PM

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.

kossebau added a comment.EditedJan 29 2019, 11:13 PM

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)

kossebau updated this revision to Diff 50535.Jan 29 2019, 11:44 PM

rely completely on working libappimage cmake config files, as avail with recent
releases

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?

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.

astippich accepted this revision.Jan 31 2019, 10:26 PM

Just noticed, you never use the AppDataParser.name(). Is that intentional?
Otherwise looks good, but you may want to wait for someone more experienced than me.

This revision is now accepted and ready to land.Jan 31 2019, 10:26 PM

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.

kossebau updated this revision to Diff 51934.Sun, Feb 17, 11:38 PM
  • update to latest master
  • remove unused name() method
This revision was automatically updated to reflect the committed changes.