D11508: Fix messagelib core includes

Wed Mar 21 19:19:30 GMT 2018

joselema added a comment.

  In D11508#230506 <https://phabricator.kde.org/D11508#230506>, @mlaurent wrote:

  > "... because the KDE CI is like Superman, with X-Ray vision. A god made of bits. And of course if a problem isn't catched by the KDE CI, that problem simply doesn't exist."
  >  We can try all wierd configuration for sure but with KDE CI we use standard configuration.
  >
  > So after that ubuntu likes wierd configuration it's your choice :)

  Well, it's not a "weird configuration", it's just a headers check which reveals a problem with the KDE code which the KDE CI, right now, doesn't detect. Just for the record the headers check is meant to reveal problems in the packaging, however, as a side effect, sometimes it reveals problems in the KDE code like in this case.

  To fix this problem for Kubuntu, we simply commented out temporarily the headers check. But please note that the universe is bigger than KDE and Ubuntu; this test is inherited from Debian, so consider the following hypothetical situation:

  1. The Debian developer packages a newer version of messagelib.
  2. The headers test fail.
  3. One possible solution is patching the code, if the Debian developer patches the code, the patch may be right or wrong (like the first version of the patch I proposed)
  4. Let's supose the patch is wrong, the Debian developer may or may not have time to send the patch here for review.
  5. I wouldn't be surprised if the wrong patch goes unnoticed to the Kubuntu and KDE Neon packaging via merges.

  It's in our best interest to keep to a bare minimum the distribution patching, right? So what's the right solution here? Commenting out the headers check in Kubuntu like we did? Well, I think it's a lot better to be a step ahead and fix the problem in the KDE scope (like you just did this morning) rather than leaving loose ends which may result in an exccesive/wrong distribution patching.

  > For me the second patch seems more logical.
  > 
  > Perhaps you can test it.
  >  Indeed we don't need to install it
  >  so we don't need your first patch where you want to install not necessary headers.

  That's great to know, I'm going to update the patch then.

  > I removed some bad include in 18.04 so perhaps it will help you in your wierd config

  It's not a "weird config"...

  Anyway, I have tested a modified 17.12.3 package with the following commits you did in master:
  https://cgit.kde.org/messagelib.git/commit/?id=feda4e33e9e935384fc322878d8da58e1f08e562
  https://cgit.kde.org/messagelib.git/commit/?id=28e8731a152b98f8f80c896c6b389748d5fdee43
  https://cgit.kde.org/messagelib.git/commit/?id=e068f81e9a6176309301eed7765aa54b2ef120dc
  https://cgit.kde.org/messagelib.git/commit/?id=1d9e28e3aa8bd1d7ad1b7b56720c436cd2dce8b9

  That indeed fixes the problem, avoiding to install extra headers because you did something similar to the first patch, but removing <core/sortorder.h> from widgetbase.h which removes the need to install other extra headers in the pack, and thus, the main mistake from the first patch. Thank you for fixing it.

REPOSITORY
  R94 PIM: Message Library

REVISION DETAIL
  https://phabricator.kde.org/D11508

To: joselema, #kde_pim, #kde_pim_kmail, mlaurent
Cc: mlaurent, joselema, rikmills, dvasin, hrouis, ach, winterz, vkrause, knauss, dvratil
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-pim/attachments/20180321/4012b7a1/attachment.html>