Bug 352658 - build on master failure due to commit bfa4274f58fcf5c873f20210ce6a88277671ccee
Summary: build on master failure due to commit bfa4274f58fcf5c873f20210ce6a88277671ccee
Status: RESOLVED FIXED
Alias: None
Product: dolphin
Classification: Applications
Component: general (show other bugs)
Version: 16.12.2
Platform: Compiled Sources Linux
: NOR normal
Target Milestone: ---
Assignee: Dolphin Bug Assignee
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-09-13 11:49 UTC by Brice De Bruyne
Modified: 2015-09-26 06:21 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Brice De Bruyne 2015-09-13 11:49:19 UTC
The commit bfa4274f58fcf5c873f20210ce6a88277671ccee
breaks building dolphin-plugins.

The forward declarations of KFileItemList and KFileItem classes
makes them incomplete.

Removing the forward declarations on top and readding the
#include <KFileItem>;
fixes the compilation.

So reverting following change fixes the build for me:

--- a/src/views/versioncontrol/kversioncontrolplugin.h
+++ b/src/views/versioncontrol/kversioncontrolplugin.h
@@ -23,9 +23,9 @@
 #include <dolphin_export.h>
 
 #include <QObject>
-#include <KFileItem>
 #include <QAction>
-
+class KFileItemList;
+class KFileItem;
 /**
  * @brief Base class for version control plugins.
  *


Reproducible: Always
Comment 1 Frank Reininghaus 2015-09-13 14:28:07 UTC
Thanks for the bug report!

On the one hand, one could argue that this should better be fixed in dolphin-plugins, because using forward declarations to reduce dependencies is a good practice. On the other hand, the header with the KFileItem include has already been released in a stable version, and forcing users of this header to adjust their code now is not very nice.
Comment 2 Brice De Bruyne 2015-09-13 21:33:29 UTC
I completely agree with you.
Comment 3 Christoph Feck 2015-09-14 21:05:47 UTC
Relying on indirect inclusion is a bug. Keeping it for convenience is nice, but our goal should be clean headers.
Comment 4 Frank Reininghaus 2015-09-15 20:23:09 UTC
A fix for dolphin-plugins, which seems to be the best long-term solution, is at https://git.reviewboard.kde.org/r/125247/. If I missed anything, please let me know!

(If it is considered necessary to revert the change in kversioncontrolplugin.h in order to not break backwards compatibility, this can still be done.)
Comment 5 Frank Reininghaus 2015-09-26 06:21:01 UTC
This has been fixed in dolphin-plugins in the mean time: https://quickgit.kde.org/?p=dolphin-plugins.git&a=commit&h=9ceea02f9d1903e260a1d45d1ca0330d6c2d47fe