[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-frameworks-devel
Subject:    D24979: KRunner: port away from deprecated KF5 API
From:       "Friedrich W. H. Kossebau" <noreply () phabricator ! kde ! org>
Date:       2019-10-27 18:05:13
Message-ID: d51ad4ee661dd0ff1ecebcf9c98b473c () localhost ! localdomain
[Download RAW message or body]

[Attachment #2 (text/plain)]

kossebau requested changes to this revision.
kossebau added a comment.
This revision now requires changes to proceed.


  One of the Interesting challenges with all the cross-library deprecation visibility \
control setup.  
  Let me try to collect requirements:
  
  - Plasma::Package is part of ABI of Plasma API, cannot be removed in normal builds, \
                only made optionally invisible in API when building against
  - Plasma::Package is part of ABI of KRunner API, cannot be removed in normal \
                builds, only made optionally invisible in API when building against
  - if Plasma::Package is made invisible with Plasma API, it also needs to be \
invisible with KRunner API  
  Current patch as proposed  has an issue for build time of KRunner itself, because \
KF_DISABLE_DEPRECATED_BEFORE_AND_AT=0x053f00 results in \
PLASMA_ENABLE_DEPRECATED_SINCE(5, 28) evaluating to FALSE during the build of \
KRunner. Which means,the symbol for AbstractRunner::package() will be missing and \
thus ABI being broken -> not allowed.  
  At the same time #if PLASMA_ENABLE_DEPRECATED_SINCE(5, 28) with the declaration in \
the header is needed indeed, as Plasma::Package is only visible to the compiler with \
this condition, and given consumers of KRunner API have control over that we need to \
handle it.  
  So, KF_DISABLE_DEPRECATED_BEFORE_AND_AT=0x053f00 is not possibly in total with \
KRunner, as its required implementation code has a hard dependency on Plasma API \
deprecated before that.  Two options:
  a) set the module specific PLASMA_DISABLE_DEPRECATED_BEFORE_AND_AT=0x050500 to \
override the default from the group KF one  b) remove visibility wrappers around  \
originalPlasma::Package API  
  I personally favour a) here.
  
  Given we have officially deprecated API in KRunner, it should indeed also get \
ported to ecm_generate_export_header. I would have now done this myself, but actually \
would like to see other people are easily able to use that macro API, so it's not \
just something compatible to my state of brain ;)

REPOSITORY
  R308 KRunner

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

To: dfaure, kossebau, mart, apol
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


[Attachment #3 (text/html)]

<table><tr><td style="">kossebau requested changes to this revision.<br />kossebau \
added a comment.<br />This revision now requires changes to proceed. </td><a \
style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; \
color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; \
background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; \
border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D24979">View \
Revision</a></tr></table><br /><div><div><p>One of the Interesting challenges with \
all the cross-library deprecation visibility control setup.</p>

<p>Let me try to collect requirements:</p>

<ul class="remarkup-list">
<li class="remarkup-list-item">Plasma::Package is part of ABI of Plasma API, cannot \
be removed in normal builds, only made optionally invisible in API when building \
against</li> <li class="remarkup-list-item">Plasma::Package is part of ABI of KRunner \
API, cannot be removed in normal builds, only made optionally invisible in API when \
building against</li> <li class="remarkup-list-item">if Plasma::Package is made \
invisible with Plasma API, it also needs to be invisible with KRunner API</li> </ul>

<p>Current patch as proposed  has an issue for build time of KRunner itself, because \
KF_DISABLE_DEPRECATED_BEFORE_AND_AT=0x053f00 results in \
PLASMA_ENABLE_DEPRECATED_SINCE(5, 28) evaluating to FALSE during the build of \
KRunner. Which means,the symbol for AbstractRunner::package() will be missing and \
thus ABI being broken -&gt; not allowed.</p>

<p>At the same time #if PLASMA_ENABLE_DEPRECATED_SINCE(5, 28) with the declaration in \
the header is needed indeed, as Plasma::Package is only visible to the compiler with \
this condition, and given consumers of KRunner API have control over that we need to \
handle it.</p>

<p>So, KF_DISABLE_DEPRECATED_BEFORE_AND_AT=0x053f00 is not possibly in total with \
KRunner, as its required implementation code has a hard dependency on Plasma API \
deprecated before that.<br /> Two options:<br />
a) set the module specific PLASMA_DISABLE_DEPRECATED_BEFORE_AND_AT=0x050500 to \
override the default from the group KF one<br /> b) remove visibility wrappers around \
originalPlasma::Package API</p>

<p>I personally favour a) here.</p>

<p>Given we have officially deprecated API in KRunner, it should indeed also get \
ported to ecm_generate_export_header. I would have now done this myself, but actually \
would like to see other people are easily able to use that macro API, so it&#039;s \
not just something compatible to my state of brain ;)</p></div></div><br \
/><div><strong>REPOSITORY</strong><div><div>R308 KRunner</div></div></div><br \
/><div><strong>REVISION DETAIL</strong><div><a \
href="https://phabricator.kde.org/D24979">https://phabricator.kde.org/D24979</a></div></div><br \
/><div><strong>To: </strong>dfaure, kossebau, mart, apol<br /><strong>Cc: \
</strong>kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns<br /></div>



[prev in list] [next in list] [prev in thread] [next in thread]