Reverting Baloo-widgets version change in cmakelists, since this breaks the…

Authored by ngraham on May 17 2018, 2:32 PM.

Description

Reverting Baloo-widgets version change in cmakelists, since this breaks the information panel Let's find another way to do this.

CCMAIL: elvis.angelaccio@kde.org
CCMAIL: kde@privat.broulik.de

@ngraham I'm wondering how changing the required version will fix things? AFAIK 2912894d4f13 was to make Dolphin compile at least (and in a short test I cannot find anything wrong with baloo-widgets' master branch in conjunction with Dolphin's information panel).

CMake is still pretty arcane to me, but apparently it wasn't able to find 18.07.70 as a valid version for baloo-widgets, so it didn't get linked in. Multiple people in Plasma were reporting that Dolphin's information panel broke with the change that introduced this, and was fixed by reverting the version change (and I could confirm), so that's what I did. Hopefully @elvisangelaccio has an idea what went wrong here and what a better fix might be.

rkflx added a comment.May 17 2018, 8:16 PM

Dolphin's information panel broke with the change that introduced this

Could you detail what "broke" means?

Dolphin's information panel broke with the change that introduced this

Could you detail what "broke" means?

It didn't show any information. :) There was just an icon, but no metadata. The baloo-widgets metadata chooser widget in right-click on information panelConfigure... was likewise empty.

rkflx added a comment.May 17 2018, 8:34 PM

That's odd, because I'm seeing things like Comments and Rating there, and I need 18.07.70 to make Dolphin compile at all.

...Weird. FWIW, the CI didn't break after the version change was rolled back: https://build.kde.org/job/Applications%20dolphin%20kf5-qt5%20SUSEQt5.9/107/

(some tests fail, but that's unrelated)

rkflx added a comment.May 17 2018, 8:39 PM

...Weird. FWIW, the CI didn't break after the version change was rolled back: https://build.kde.org/job/Applications%20dolphin%20kf5-qt5%20SUSEQt5.9/107/

(some tests fail, but that's unrelated)

Yeah, because the CI has everything from master, which means it is not affected by any changes to required versions (both yours and Elvis').

Ah, that makes sense. When I pull down baloo-widgets from git master, everything works. So I think I see what happened: because baloo-widgets isn't actually a framework, those of us who were encountering the bug were running Dolphin from git master, but with an older-version of baloo-widgets. Because it's an optional rather than required dependency, Dolphin successfully compiled without it (with a warning in the CMake configuration step that everyone missed), leading to a working Dolphin that had a non-working Information panel.

Does that make sense? If so, I'll revert the reversion...

Does that make sense? If so, I'll revert the reversion...

+1

The only thing missing from your explanation is how folks in Plasma were able to compile Dolphin with baloo-widgets 4.97 after your revert. IIUC it's either no baloo-widgets or baloo-widgets from master, because 4.97 will lead to compilation failure. I guess nobody properly tested your revert.

rkflx added a comment.EditedMay 17 2018, 10:19 PM

One more thing while we are at it: Would it make sense to make the Configure entry in the context menu depend on the availability of KF5BalooWidgets too? If this dialog is only showing Baloo items and nothing else (?), it would always be empty otherwise.

The only thing missing from your explanation is how folks in Plasma were able to compile Dolphin with baloo-widgets 4.97 after your revert. IIUC it's either no baloo-widgets or baloo-widgets from master, because 4.97 will lead to compilation failure. I guess nobody properly tested your revert.

I tested it, and it worked just fine with whatever old version of baloo-widgets I had installed. It also continued working once I upgraded baloo-widgets to master. No compilation errors and a working Information panel.

One more thing while we are at it: Would it make sense to make the Configure entry in the context menu depend on the availability of KF5BalooWidgets too? If this dialog is only showing Baloo items and nothing else (?), it would always be empty otherwise.

I'd need to read the code more deeply, but from what I can tell, it's supposed to fall back to KfileMetaData if baloo-widgets isn't present. The fact that it didn't (or that it did, but functionality was still lost) may be the real bug. I'll investigate further.

Meanwhile, I'll revert the reversion and notify the folks in Plasma regarding what they need to do to get a working information panel with Dolphin from git master.

The only thing missing from your explanation is how folks in Plasma were able to compile Dolphin with baloo-widgets 4.97 after your revert. IIUC it's either no baloo-widgets or baloo-widgets from master, because 4.97 will lead to compilation failure. I guess nobody properly tested your revert.

I tested it, and it worked just fine with whatever old version of baloo-widgets I had installed.

With 4.97 (which in my case is 18.04.1) you should get this error:

Building CXX object src/CMakeFiles/dolphinstatic.dir/panels/places/placespanel.cpp.o
src/panels/information/informationpanelcontent.cpp: In constructor ‘InformationPanelContent::InformationPanelContent(QWidget*)’:
src/panels/information/informationpanelcontent.cpp:115:23: error: ‘class Baloo::FileMetaDataWidget’ has no member named ‘setDateFormat’
     m_metaDataWidget->setDateFormat(static_cast<Baloo::DateFormats>(InformationPanelSettings::dateFormat()));

I'd need to read the code more deeply, but from what I can tell, it's supposed to fall back to KfileMetaData if baloo-widgets isn't present. The fact that it didn't (or that it did, but functionality was still lost) may be the real bug. I'll investigate further.

Is this about compile-time availability of baloo-widgets, or about a running Baloo when using Dolphin?

OK, I can reproduce the compile failure now if I use baloo-widgets from the applications/18.04 branch without Elvis's patch. I can also reproduce the broken information panel at runtime if I re-apply Elvis's complete patch and compile and install a version of baloo-widgets from an older git master that didn't yet have the necessary version bump (so CMake skipped it at compile-time). So I think that's what was going on for me and the other Plasma folks who complained. We just needed to compile and deploy newer versions of baloo-widgets.

I've reverted the reversion. :)

One more thing while we are at it: Would it make sense to make the Configure entry in the context menu depend on the availability of KF5BalooWidgets too?

Yes. There are also other issues, let's track them in T8720.