<table><tr><td style="">kossebau added subscribers: whiting, kossebau.<br />kossebau added a comment.
</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/D7194" rel="noreferrer">View Revision</a></tr></table><br /><div><div><p><a href="https://phabricator.kde.org/p/apol/" style="
              border-color: #f1f7ff;
              color: #19558d;
              background-color: #f1f7ff;
                border: 1px solid transparent;
                border-radius: 3px;
                font-weight: bold;
                padding: 0 4px;" rel="noreferrer">@apol</a>  <a href="https://phabricator.kde.org/p/whiting/" style="
              border-color: #f1f7ff;
              color: #19558d;
              background-color: #f1f7ff;
                border: 1px solid transparent;
                border-radius: 3px;
                font-weight: bold;
                padding: 0 4px;" rel="noreferrer">@whiting</a> <a href="https://phabricator.kde.org/p/leinir/" style="
              border-color: #f1f7ff;
              color: #19558d;
              background-color: #f1f7ff;
                border: 1px solid transparent;
                border-radius: 3px;
                font-weight: bold;
                padding: 0 4px;" rel="noreferrer">@leinir</a> This change might have broken quite some things: EntryInternal has been an explicitly shared data object for some years (cmp. <tt style="background: #ebebeb; font-size: 13px;">QExplicitlySharedDataPointer<Private> d;</tt>), and quite some logic seems to depend on that. E.g <tt style="background: #ebebeb; font-size: 13px;">ItemsModel</tt>, which keeps a separate copy of the list of EntryInternal objects, does not update those objects on changes, but relies on this to happen automagically due to the explicitely shared data, <a href="https://phabricator.kde.org/R304:c32c8d002e1216373560c94738841a7a5e1b976b" style="background-color: #e7e7e7;
          border-color: #e7e7e7;
          border-radius: 3px;
          padding: 0 4px;
          font-weight: bold;
          color: black;text-decoration: none;" rel="noreferrer">c32c8d002e12</a> tries to work-around that, but actually makes things more convoluted.<br />
Same with <tt style="background: #ebebeb; font-size: 13px;">DownloadWidgetPrivate</tt> and <tt style="background: #ebebeb; font-size: 13px;">Cache</tt>, which both keep a set of changed entries as a <tt style="background: #ebebeb; font-size: 13px;">QSet<EntryInternal></tt>, which they maintain by <tt style="background: #ebebeb; font-size: 13px;">QSet::insert()</tt>, just that this will due to same uniqueId not update the contained item to the latest version. Still those (potentially outdated) objects are then passed via signals to e.g. library consumers, which get outdated or incomplete data (like installedFiles values).<br />
Similar issue with <tt style="background: #ebebeb; font-size: 13px;">AtticaProvider::entryFromAtticaContent()</tt> which also assumes in the logic explicitly shared data so the cache object gets all the updated data as well.</p>

<p>Given this is a public class, this is quite some behavioural change. One could assume this breaks 3rd-party code in a similar way. There is no specification of the behaviour in the API dox though, so unsure myself what is the best solution: <a href="https://api.kde.org/frameworks/knewstuff/html/classKNSCore_1_1EntryInternal.html" class="remarkup-link" target="_blank" rel="noreferrer">https://api.kde.org/frameworks/knewstuff/html/classKNSCore_1_1EntryInternal.html</a></p>

<p>One fallout is at least this bug: <a href="https://bugs.kde.org/show_bug.cgi?id=386156" class="remarkup-link" target="_blank" rel="noreferrer">https://bugs.kde.org/show_bug.cgi?id=386156</a></p>

<p>I had started a patch to fix this, but it has grown a lot to make up for the now no longer explicitely shared nature of this class, and there are still issues in some places. So I wonder if it is better to simply revert this commit instead, document the old behaviour properly in the API and fix the places which rely on non-explicitely shared data objects. That might be only some newer code, given the old behaviour has stayed around for some years.</p>

<p>What do you think?</p></div></div><br /><div><strong>REPOSITORY</strong><div><div>R304 KNewStuff</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D7194" rel="noreferrer">https://phabricator.kde.org/D7194</a></div></div><br /><div><strong>To: </strong>apol, leinir<br /><strong>Cc: </strong>kossebau, whiting, mutlaqja, broulik, Frameworks<br /></div>