Bug 463598

Summary: External plugins are not found, findPlugins used by findExtractors only searches for files/libraries
Product: [Frameworks and Libraries] frameworks-kfilemetadata Reporter: Gabriel Marcano <gabemarcano>
Component: generalAssignee: Alexander Lohnau <alexander.lohnau>
Status: RESOLVED FIXED    
Severity: normal CC: alexander.lohnau, asturm, gabemarcano
Priority: NOR    
Version First Reported In: 5.101.0   
Target Milestone: ---   
Platform: Gentoo Packages   
OS: Linux   
URL: https://invent.kde.org/frameworks/kfilemetadata/-/merge_requests/46
Latest Commit: Version Fixed/Implemented In:
Sentry Crash Report:
Attachments: external_extractor_plugin.patch
add_external_plugins.patch

Description Gabriel Marcano 2022-12-29 18:48:49 UTC
SUMMARY
***
External plugins are not found by kfilemetadata. I did some digging, and it turns out that the current implementation in `ExtractorCollectionPrivate::findExtractors` for external plugins relies on `KPluginMetaData::findPlugins`, which uses `KPluginMetaDataPrivate::forEachPlugin` which only searches for files, and these files must be libraries.
***


STEPS TO REPRODUCE
1. Compile kfilemetadata and kcoreaddons with debugging symbols and install them
2. Install an external plugin at libexec/kf5/kfilemetadata/externalextractors/test/ (on my system, this is at `/usr/lib64/libexec/kf5/kfilemetadata/externalextractors/test/`)
2. gdb --args /usr/bin/baloo_filemetadata_temp_extractor [some-audio-file-as-an-example]
3. Place a breakpoint at `ExtractorCollectionPrivate::findExtractors`  and `KPluginMetaDataPrivate::forEachPlugin` 

OBSERVED RESULT
When trying to find external plugins, `KPluginMetaDataPrivate::forEachPlugin` skips over these are they aren't files (`QDirIterator it(dir, QDir::Files);`) nor libraries (`if (QLibrary::isLibrary(it.fileName())) {`.

EXPECTED RESULT
The kfilemetadata implementation uses something else to find external plugins so that they are found by `ExtractorCollectionPrivate::findExtractors`

SOFTWARE/OS VERSIONS
Linux/KDE Plasma: Gentoo Linux
(available in About System)
KDE Plasma Version: 5.26.4
KDE Frameworks Version: 5.101.0
Qt Version: 5.15.7

ADDITIONAL INFORMATION
Let me know if you need additional information, but this should be fairly easy to reproduce. The new-ish `KPluginMetaData::findPlugins` support from KPluginMetaData doesn't have a concept for external plugins-- not sure if that's intentional or a bug. In any case, currently, it assumes all plugins are files and libraries, which explains why the current kfilemetadata implementation fails to find any external libraries, as these are in directories, and are not libraries.
Comment 1 Gabriel Marcano 2022-12-30 06:00:23 UTC
Created attachment 154894 [details]
external_extractor_plugin.patch

After looking at the more recent changes to the code that manages the external plugins, I reverted the changes only for the external plugin handling, yielding this patch. With this in place I can successfully use an external plugin with kfilemetadata.

I took a brief look at the writer collection class, and it looks like the external plugins code isn't affected... but might have the same bug reported in bug 438186.
Comment 2 Bug Janitor Service 2023-01-05 20:04:40 UTC
A possibly relevant merge request was started @ https://invent.kde.org/frameworks/kfilemetadata/-/merge_requests/78
Comment 3 Andreas Sturmlechner 2023-01-06 10:27:44 UTC
Gabriel, thanks for your report. Could you please test the linked patch provided by upstream?
Comment 4 Alexander Lohnau 2023-01-10 17:54:52 UTC
Git commit 5bd67e925db7db2f9619139f7e8000ce0321d52d by Alexander Lohnau.
Committed on 10/01/2023 at 17:13.
Pushed by alex into branch 'master'.

Fix loading of external extractors and writers

In 5bbdfc4749f92c510cc82dbd66f5d3b2bc3763ac, the external extractors were mistakenly ported to KPluginMetaData::findPlugins.

However, the logic was broken before, because the dir entries must *not* be plugins, but normal files.
This was introduced in 73da9a53cb500da000c50b164dd5693ef7474041.

Considering that we tell QDir to only list dirs, the check if we have a plugin or not doesn't make sense anyways.

M  +12   -11   src/extractorcollection.cpp
M  +0    -3    src/writercollection.cpp

https://invent.kde.org/frameworks/kfilemetadata/commit/5bd67e925db7db2f9619139f7e8000ce0321d52d
Comment 5 Nicolas Fella 2023-01-11 12:35:47 UTC
Git commit 543abf86e6747e406e176ac4e8fc52090bae373b by Nicolas Fella, on behalf of Alexander Lohnau.
Committed on 11/01/2023 at 12:35.
Pushed by nicolasfella into branch 'kf5'.

Fix loading of external extractors and writers

In 5bbdfc4749f92c510cc82dbd66f5d3b2bc3763ac, the external extractors were mistakenly ported to KPluginMetaData::findPlugins.

However, the logic was broken before, because the dir entries must *not* be plugins, but normal files.
This was introduced in 73da9a53cb500da000c50b164dd5693ef7474041.

Considering that we tell QDir to only list dirs, the check if we have a plugin or not doesn't make sense anyways.
(cherry picked from commit 5bd67e925db7db2f9619139f7e8000ce0321d52d)

M  +12   -11   src/extractorcollection.cpp
M  +0    -3    src/writercollection.cpp

https://invent.kde.org/frameworks/kfilemetadata/commit/543abf86e6747e406e176ac4e8fc52090bae373b
Comment 6 Gabriel Marcano 2023-01-11 22:16:46 UTC
(In reply to Andreas Sturmlechner from comment #3)
> Gabriel, thanks for your report. Could you please test the linked patch
> provided by upstream?

Sorry, was traveling. No, the patch does not fix the problem. In particular, the problem is that it fails to add the external plugins found to the `m_allExtractors` list. I don't see a similar issue in the writercollection class.

I'll attach a rough patch that fixes the issue for me.
Comment 7 Gabriel Marcano 2023-01-11 22:18:36 UTC
Created attachment 155224 [details]
add_external_plugins.patch

Commit 5bd67e925db7db2f9619139f7e8000ce0321d52d almost fixed the bug, it just missed adding the found external plugins to the list of all extract plugins. This patch, once applied on top of 5bd67e925db7db2f9619139f7e8000ce0321d52d, fixes the problem for me.
Comment 8 Bug Janitor Service 2023-01-12 14:16:42 UTC
A possibly relevant merge request was started @ https://invent.kde.org/frameworks/kfilemetadata/-/merge_requests/79
Comment 9 Alexander Lohnau 2023-01-14 17:58:09 UTC
Git commit 34637beb6bb4dbaf80377f12142c799ed9e000d5 by Alexander Lohnau.
Committed on 12/01/2023 at 14:15.
Pushed by alex into branch 'master'.

Also add external extractors to vector of all plugins

Amends 5bd67e925db7db2f9619139f7e8000ce0321d52d

M  +10   -0    src/extractorcollection.cpp

https://invent.kde.org/frameworks/kfilemetadata/commit/34637beb6bb4dbaf80377f12142c799ed9e000d5
Comment 10 Alexander Lohnau 2023-01-14 18:06:22 UTC
Git commit 36efc6c632ad88bf6b1f16649ec28cc78dad5c15 by Alexander Lohnau.
Committed on 14/01/2023 at 18:06.
Pushed by alex into branch 'kf5'.

Also add external extractors to vector of all plugins

Amends 5bd67e925db7db2f9619139f7e8000ce0321d52d


(cherry picked from commit 34637beb6bb4dbaf80377f12142c799ed9e000d5)

M  +10   -0    src/extractorcollection.cpp

https://invent.kde.org/frameworks/kfilemetadata/commit/36efc6c632ad88bf6b1f16649ec28cc78dad5c15