Re: KF5 Tags tag tags, not commits

2016-04-24 Thread Stephen Kelly 
David Faure wrote:

> On Sunday 24 April 2016 13:54:02 Stephen Kelly wrote:
>> 
>> Why doesn't v5.21.0 tag the d2e7c78c94e6 commit directly instead?
> 
> Don't know.
> 
> I just call git tag, then git does its magic.
> 
> cat $here/modules.git | while read repo branch; do
> echo $repo
> . $here/version
> tagname=v$version
> versionfile=$here/versions/$repo
> if [ ! -f $versionfile ]; then echo "$versionfile not found"; exit 1;
> fi b=`sed '2q;d' $versionfile`
> echo $b
> checkout=$(findCheckout $repo)
> cd $checkout || exit 2
> echo $PWD
> $cmd git fetch --tags || exit 2
> $cmd git tag -a $tagname $b -m "Create tag for $version"  # ignore
> error ("already exists") $cmd git push --tags || exit 5
> done
> 
> A previous script (make_rc_tag.sh) tagged rc similarly, in that same
> checkout.
> 
> You can see it all in the release-tools.git repo, branch frameworks/5.0

Thanks, I had looked around for it but didn't find it. I can't seem to 
access that though.

$ git clone kde:release-tools
Cloning into 'release-tools'...
fatal: remote error: access denied or repository not exported: /release-
tools


> Why is this an actual problem btw?

It just seems like a bug.

Thanks,

Steve.


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: [KIO] cookie service and installing in a parallel prefix (/opt/local)

2016-04-24 Thread David Faure 
On Friday 22 April 2016 23:51:51 René J.V. Bertin wrote:
> On Friday April 22 2016 23:33:27 René J.V. Bertin wrote:
> 
> > Looks like I'm going to have to add output to kio_http and see if the error 
> > message doesn't come from an error==ENOENT returned from a failed function 
> > call. Seems like something that's going to be fun ...
> 
> Looking at when http.cpp returns ERR_DOES_NOT_EXIST I had a hunch and turned 
> off the web cache. Lo and behold, pages started to load, so the error must be 
> in the caching .

Caching done by kio_http, or caching done at the system level ?

> I keep getting a crash on exist though:
> 
> frame #3: 0x00010ca5bde9 
> libKF5Crash.5.dylib`KCrash::defaultCrashHandler(sig=) + 1049 at 
> kcrash.cpp:527
> frame #4: 0x7fff8b9645aa libsystem_platform.dylib`_sigtramp + 26
> frame #5: 0x00010dc6db50 QtCore`QCache bool>::insert(this=0x7f85eb6f3090, akey=, 
> aobject=, acost=) + 368 at qcache.h:173
> frame #6: 0x00010dc6c6d8 QtCore`setNativeLocks(QString const&, int) 
> [inlined] QStringBuilder::operator 
> QString(fn=0x7f85eb41a500, a=0x7f85eb41a3b0, b=0x7f85eb44e350, 
> rc=) const + 218 at qlockfile_unix.cpp:142

Already fixed in Qt, branch 5.6 (and 5.7)
See comimt b6eea89b67e0d3bb4f8f888fff21257eff0b65a5

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: KF5 Tags tag tags, not objects

2016-04-24 Thread David Faure 
On Sunday 24 April 2016 13:54:02 Stephen Kelly wrote:
> 
> Why doesn't v5.21.0 tag the d2e7c78c94e6 commit directly instead?

Don't know.

I just call git tag, then git does its magic.

cat $here/modules.git | while read repo branch; do
echo $repo
. $here/version
tagname=v$version
versionfile=$here/versions/$repo
if [ ! -f $versionfile ]; then echo "$versionfile not found"; exit 1; fi
b=`sed '2q;d' $versionfile`
echo $b
checkout=$(findCheckout $repo)
cd $checkout || exit 2
echo $PWD
$cmd git fetch --tags || exit 2
$cmd git tag -a $tagname $b -m "Create tag for $version"  # ignore error 
("already exists")
$cmd git push --tags || exit 5
done

A previous script (make_rc_tag.sh) tagged rc similarly, in that same checkout.

You can see it all in the release-tools.git repo, branch frameworks/5.0

Why is this an actual problem btw?

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Jenkins-kde-ci: kdbusaddons master kf5-qt5 » Linux, gcc - Build # 11 - Still Unstable!

2016-04-24 Thread Martin Graesslin 
On Saturday, April 23, 2016 8:48:17 PM CEST Ben Cooksley wrote:
> On Sat, Apr 23, 2016 at 8:35 PM, Martin Graesslin  
wrote:
> > On Saturday, April 23, 2016 1:09:00 PM CEST Ben Cooksley wrote:
> >> On Sat, Apr 23, 2016 at 1:04 PM, Daniel Vrátil  wrote:
> >> > On Saturday, April 23, 2016 1:44:16 AM CEST Ben Cooksley wrote:
> >> >> On Fri, Apr 22, 2016 at 11:11 PM, David Faure  wrote:
> >> >> > On Thursday 21 April 2016 14:57:52 no-re...@kde.org wrote:
> >> >> >> GENERAL INFO
> >> >> >> 
> >> >> >> BUILD UNSTABLE
> >> >> >> Build URL:
> >> >> >> https://build.kde.org/job/kdbusaddons%20master%20kf5-qt5/PLATFORM=L
> >> >> >> inu
> >> >> >> x,
> >> >> >> compiler=gcc/11/>
> >> >> > 
> >> >> > Wow, ASAN detected a real bug in Qt here.
> >> >> > 
> >> >> > Fixed in https://codereview.qt-project.org/156728
> >> >> 
> >> >> As this issue essentially breaks any test on the CI system that
> >> >> depends on Qt I suggest all developers affected by this indicate this
> >> >> on the relevant reviews.
> >> > 
> >> > Hey guys,
> >> 
> >> Hi David,
> >> 
> >> > any chance to get this patch into our Qt build on the CI since it has
> >> > been
> >> > integrated upstream now? As it affects all tests that interact with
> >> > DBus
> >> > it
> >> > would be nice to resolve this on the CI.
> >> 
> >> We'll need upstream to update qt5.git (I think that's still required
> >> for changes to fully flow through) I think.
> >> We could try to hack the patch in, but then we'll end up having to
> >> undo that in a few weeks time.
> > 
> > Could we disable that ASAN check please[1]? I have two of my projects
> > failing due to the DBus problem and one further failing due to a similar
> > problem in QQuickStyleItem [2]. It's nice that we find errors, but I
> > rather not have my tests fail due to errors in Qt.
> 
> That would just cover up the issue.

Yes it would and that's exactly what I want.

To me the CI is currently producing false positives. It started to break from 
one moment to the other. It costs time to look into why it started to fail. It 
means that till that problem is fixed in Qt I cannot trust the CI system. It 
results in nobody noticing real breakage, because the build is broken anyway.

Then there is the problem of responsibility: I look at the output, I have no 
idea why that thing is broken, I have no idea how to build a minimal example 
which would reproduce it, even less on how to fix it. That's just a really bad 
situation to have a CI which can be trusted.

In summary: I don't want spend time to investigate false positives. If an 
option results in false positives, I think that we should disable it.

Of course Qt should get those bugs fixed, but for that Qt should enable ASAN on 
their CI system. If we get libraries which are free of ASAN problems, then we 
can enable it on our side. But false positives are just bad.

Cheers
Martin

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Jenkins-kde-ci: kdbusaddons master kf5-qt5 » Linux, gcc - Build # 11 - Still Unstable!

2016-04-24 Thread Ben Cooksley 
On Mon, Apr 25, 2016 at 10:36 AM, Albert Astals Cid  wrote:
> El dilluns, 25 d’abril de 2016, a les 9:05:58 CEST, Ben Cooksley va escriure:
>> On Sun, Apr 24, 2016 at 11:46 AM, Michael Pyne  wrote:
>> > On Sat, April 23, 2016 13:09:00 Ben Cooksley wrote:
>> >> On Sat, Apr 23, 2016 at 1:04 PM, Daniel Vrátil  wrote:
>> >> > any chance to get this patch into our Qt build on the CI since it has
>> >> > been
>> >> > integrated upstream now? As it affects all tests that interact with
>> >> > DBus
>> >> > it
>> >> > would be nice to resolve this on the CI.
>> >>
>> >> We'll need upstream to update qt5.git (I think that's still required
>> >> for changes to fully flow through) I think.
>> >> We could try to hack the patch in, but then we'll end up having to
>> >> undo that in a few weeks time.
>> >
>> > If we're talking about Qt commit 1b9d082b, that appears to already be in
>> > the 5.6 branch (though not dev). I'm not sure which the CI uses though,
>> > if it's not 5.6 or less then it is probably more work than its worth to
>> > try to downgrade (something I've run into too much myself).
>>
>> The CI system uses the 5.6 branch, from qt5.git.
>> I've now triggered a build, but as qt5.git hasn't been updated since
>> our last build I have no idea whether this will pull the relevant
>> commit in.
>
> Is there any special reason we inflict ourselves the pain of using qt5.git
> instead of the standalone repos?

Primarily to keep everything in sync, so others can reproduce the
version of Qt 5 we're using.
We could try to do everything repo by repo but it would be quite a bit
of time to setup.

Also, I'm not sure if Qt's build system is up to having each module in
it's own installation prefix (haven't tried though).

>
> Cheers,
>   Albert

Regards,
Ben

>
>>
>> > Regards,
>> >
>> >  - Michael Pyne
>>
>> Cheers,
>> Ben
>> ___
>> Kde-frameworks-devel mailing list
>> Kde-frameworks-devel@kde.org
>> https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
>
>
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Jenkins-kde-ci: kdbusaddons master kf5-qt5 » Linux, gcc - Build # 11 - Still Unstable!

2016-04-24 Thread Albert Astals Cid 
El dilluns, 25 d’abril de 2016, a les 9:05:58 CEST, Ben Cooksley va escriure:
> On Sun, Apr 24, 2016 at 11:46 AM, Michael Pyne  wrote:
> > On Sat, April 23, 2016 13:09:00 Ben Cooksley wrote:
> >> On Sat, Apr 23, 2016 at 1:04 PM, Daniel Vrátil  wrote:
> >> > any chance to get this patch into our Qt build on the CI since it has
> >> > been
> >> > integrated upstream now? As it affects all tests that interact with
> >> > DBus
> >> > it
> >> > would be nice to resolve this on the CI.
> >> 
> >> We'll need upstream to update qt5.git (I think that's still required
> >> for changes to fully flow through) I think.
> >> We could try to hack the patch in, but then we'll end up having to
> >> undo that in a few weeks time.
> > 
> > If we're talking about Qt commit 1b9d082b, that appears to already be in
> > the 5.6 branch (though not dev). I'm not sure which the CI uses though,
> > if it's not 5.6 or less then it is probably more work than its worth to
> > try to downgrade (something I've run into too much myself).
> 
> The CI system uses the 5.6 branch, from qt5.git.
> I've now triggered a build, but as qt5.git hasn't been updated since
> our last build I have no idea whether this will pull the relevant
> commit in.

Is there any special reason we inflict ourselves the pain of using qt5.git 
instead of the standalone repos?

Cheers,
  Albert

> 
> > Regards,
> > 
> >  - Michael Pyne
> 
> Cheers,
> Ben
> ___
> Kde-frameworks-devel mailing list
> Kde-frameworks-devel@kde.org
> https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

[Differential] [Updated] D1465: KFileItem: remove separate storage of times, use UDSEntry instead.

2016-04-24 Thread dfaure (David Faure) 
dfaure added a reviewer: Frameworks.

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

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: dfaure, Frameworks
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: ECM metainfo path

2016-04-24 Thread Matthias Klumpp 
2016-04-22 12:45 GMT+02:00 Harald Sitter :
> Lost mailing list CC, I am assuming that was intentional? (:

Oops, no - I didn't see the CC and didn't hit reply-all then...
Adding the list back (hope your reply isn't private ;-) ), the mail I
sent first is below.

> On Thu, Apr 21, 2016 at 4:21 PM, Matthias Klumpp  
> wrote:
>> 2016-04-21 12:26 GMT+02:00 Harald Sitter :
>>> ahoy ahoy!
>>>
>>> http://commits.kde.org/extra-cmake-modules/4b7a90bfe7a3e2eb3ae83c946c182a79fabc51e3
>>>
>>> doesn't that break compatibility with older appstreams for everything
>>> that uses ECM? if so, I am not sure that is appropriate TBH
>>
>> It shouldn't, since all tools which handle this have been adapted
>> already - I expect this to be only enabled for the next generation of
>> KDE apps, so this change shouldn't hit distributions right now (I hope
>> I got e-c-m's release schedule right here...).
>
> ECM gets released monthly, so this would hit anyone that wishes to
> push the new ECM onto an existing stack that has an older appstream.

Hmm, that could indeed be a problem. The main component which should
not be out of date is the server-side module which generates the
distro metadata. AFAIK all distros are up-to-date on that, but we
could also wait a few more months to be sure everyone has updated.
The client tools not being updated shouldn't be a big issue, they
rarely read /usr/share/appdata or /usr/share/metainfo.

> And technically we expect distributions to do that which is why we
> technically aren't breaking backwards compat.

Hehe, does Debian know that? ;-)

> Now if you were to say
> that the appstream tools already looked in /metainfo as well as
> /appdata for a year or two

True for AppStream itself (~6months), but the GNOME world switched
only recently.

> but we are simply late to the party, then I
> guess the change might be done just as well even if technically
> breaking compat.
>
> Whether or not we are aware of an ECM user that would have a problem
> with this or not doesn't really factor into it though, just like we
> don't break ABI because we think that no one is using a specific
> interface.

Okay, that's an entirely different thing then - so, when are we
allowed to do such breaking changes?
Moving away from appdata/ is something I care much about, because as
the recent discussion at KDE has shown, people still seem to think
this metadata is only for applications (which it isn't).

I think to not break assumptions, reverting that commit is fine now,
but knowing some time when changing it is okay would also be good.
Generally, the critical factor is distributions upgrading their
AppStream generator, which happened at Debian and Ubuntu, should be
done at Fedora (I need to ask again). I have no data on OpenSUSE and
Arch yet, but I can ask them too.

Cheers,
Matthias

2016-04-21 12:26 GMT+02:00 Harald Sitter :
> ahoy ahoy!
>
> http://commits.kde.org/extra-cmake-modules/4b7a90bfe7a3e2eb3ae83c946c182a79fabc51e3
>
> doesn't that break compatibility with older appstreams for everything
> that uses ECM? if so, I am not sure that is appropriate TBH

It shouldn't, since all tools which handle this have been adapted
already - I expect this to be only enabled for the next generation of
KDE apps, so this change shouldn't hit distributions right now (I hope
I got e-c-m's release schedule right here...).
This might be an issue with backports on Fedora, but that would be the
only problem I am aware of. Ubuntu/Debian are fine, and AppStream,
libappstream-qt, libappstream, appstream-glib appstream-builder and
appstream-generator are all adapted for this change.

So it should be safe ^^
Honestly, I am very eager to get rid of the old misleading name too -
if you notice anything that breaks, please let me know and we can
revert that change.
(One thing required in particular would be updating the validator at
the CI system (which would only validate the new path then, though))
Cheers,
Matthias


-- 
Debian Developer | Freedesktop-Developer
I welcome VSRE emails. See http://vsre.info/
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Jenkins-kde-ci: kdbusaddons master kf5-qt5 » Linux, gcc - Build # 11 - Still Unstable!

2016-04-24 Thread Ben Cooksley 
On Sun, Apr 24, 2016 at 11:46 AM, Michael Pyne  wrote:
> On Sat, April 23, 2016 13:09:00 Ben Cooksley wrote:
>> On Sat, Apr 23, 2016 at 1:04 PM, Daniel Vrátil  wrote:
>> > any chance to get this patch into our Qt build on the CI since it has been
>> > integrated upstream now? As it affects all tests that interact with DBus
>> > it
>> > would be nice to resolve this on the CI.
>>
>> We'll need upstream to update qt5.git (I think that's still required
>> for changes to fully flow through) I think.
>> We could try to hack the patch in, but then we'll end up having to
>> undo that in a few weeks time.
>
> If we're talking about Qt commit 1b9d082b, that appears to already be in the
> 5.6 branch (though not dev). I'm not sure which the CI uses though, if it's
> not 5.6 or less then it is probably more work than its worth to try to
> downgrade (something I've run into too much myself).

The CI system uses the 5.6 branch, from qt5.git.
I've now triggered a build, but as qt5.git hasn't been updated since
our last build I have no idea whether this will pull the relevant
commit in.

>
> Regards,
>  - Michael Pyne

Cheers,
Ben
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127251: [KUnitConversion] Fix downloading currency exchange rates

2016-04-24 Thread Kai Uwe Broulik 


> On April 24, 2016, 1:57 nachm., David Faure wrote:
> > Better, but I'm still wary of the reentrancy due to the nested event loop 
> > usage.
> > 
> > I bet this leads to a complete deadlock:
> > 
> > QTimer::singleShot(0, this, SLOT(launchConversion()));
> > launchConversion();
> > 
> > where launchConversion() triggers the KUnitConversion code that this patch 
> > is about.
> > 
> > While waiting for the first conversion, the timer will fire (given the 
> > nested QEventLoop usage), and we'll enter this code again, and hit 
> > mutex.lock(), and then wait forever, since we'll never go back to the event 
> > loop to finish the first conversion and unlock the mutex.
> > 
> > Maybe the mutex can be removed, actually. All that needs to be done is for 
> > m_update to be turned into a local variable (there's no reason for it to be 
> > a member variable, right?). The QSaveFile usage fixes the case where two 
> > threads would write to m_cache at the same time, or the case where one 
> > writes and one reads. Or is the mutex needed for the if() at the top? In 
> > any case I recommend not holding the mutex while being in the event loop.
> > 
> > Sorry for not realizing this earlier.

>From what I can tell the m_update is so it updates it either on first run or 
>when the cache file is outdated/missing. I don't know the internals of 
>KUnitConversion, ie. if an instance of Currency is shared or not, and how to 
>ensure that for subsequent runs of convert() I don't needlessly probe the 
>cache file again.

The m_cache variable could also be made local to the convert() function.


- Kai Uwe


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127251/#review94808
---


On April 24, 2016, 1:26 nachm., Kai Uwe Broulik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127251/
> ---
> 
> (Updated April 24, 2016, 1:26 nachm.)
> 
> 
> Review request for KDE Frameworks and Vishesh Handa.
> 
> 
> Bugs: 345750
> https://bugs.kde.org/show_bug.cgi?id=345750
> 
> 
> Repository: kunitconversion
> 
> 
> Description
> ---
> 
> QNetworkReply does not implement waitForReadyRead
> 
> Also, the code never actually created the cache dir it was trying to create a 
> file in.
> 
> 
> Diffs
> -
> 
>   src/currency.cpp ad325d8 
> 
> Diff: https://git.reviewboard.kde.org/r/127251/diff/
> 
> 
> Testing
> ---
> 
> Works now. It's downloaded once and then taken from cache file in 
> ~/.local/share/libkunitconversion/currency.xml
> 
> Given it's a Tier 2 framework doesn't make sense to add KIO now, also failed 
> to reproduce the crashes mentioned in the code.
> 
> Tests pass (only if I run them on English locale btw)
> 
> Obviously not happy with this being sync but alas that's how the API works.
> 
> Not sure if this is a KRunner bug or KUnitConverison but if I enter "5 USD" 
> it converts fine, if I enter "5 usd" it returns zero for all the currencies 
> it converted to.
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Jenkins-kde-ci: kservice master kf5-qt5 » Linux,gcc - Build # 18 - Failure!

2016-04-24 Thread no-reply 

GENERAL INFO

BUILD FAILURE
Build URL: 
https://build.kde.org/job/kservice%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/18/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Sun, 24 Apr 2016 16:56:40 +
Build duration: 1.9 sec

CHANGE SET
No changes
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Jenkins-kde-ci: kglobalaccel master kf5-qt5 » Linux,gcc - Build # 16 - Still Unstable!

2016-04-24 Thread no-reply 

GENERAL INFO

BUILD UNSTABLE
Build URL: 
https://build.kde.org/job/kglobalaccel%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/16/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Sun, 24 Apr 2016 16:56:40 +
Build duration: 1 min 9 sec

CHANGE SET
No changes


JUNIT RESULTS

Name: (root) Failed: 1 test(s), Passed: 0 test(s), Skipped: 0 test(s), Total: 1 
test(s)Failed: TestSuite.kglobalshortcuttest

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 3/3 (100%)FILES 8/18 (44%)CLASSES 8/18 (44%)LINE 126/1278 
(10%)CONDITIONAL 52/1075 (5%)

By packages
  
src
FILES 1/7 (14%)CLASSES 1/7 (14%)LINE 1/400 (0%)CONDITIONAL 
2/278 (1%)
src.runtime
FILES 6/10 (60%)CLASSES 6/10 (60%)LINE 103/751 (14%)CONDITIONAL 
38/688 (6%)
src.runtime.plugins.xcb
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 22/127 (17%)CONDITIONAL 
12/109 (11%)___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127251: [KUnitConversion] Fix downloading currency exchange rates

2016-04-24 Thread David Faure 

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127251/#review94808
---



Better, but I'm still wary of the reentrancy due to the nested event loop usage.

I bet this leads to a complete deadlock:

QTimer::singleShot(0, this, SLOT(launchConversion()));
launchConversion();

where launchConversion() triggers the KUnitConversion code that this patch is 
about.

While waiting for the first conversion, the timer will fire (given the nested 
QEventLoop usage), and we'll enter this code again, and hit mutex.lock(), and 
then wait forever, since we'll never go back to the event loop to finish the 
first conversion and unlock the mutex.

Maybe the mutex can be removed, actually. All that needs to be done is for 
m_update to be turned into a local variable (there's no reason for it to be a 
member variable, right?). The QSaveFile usage fixes the case where two threads 
would write to m_cache at the same time, or the case where one writes and one 
reads. Or is the mutex needed for the if() at the top? In any case I recommend 
not holding the mutex while being in the event loop.

Sorry for not realizing this earlier.

- David Faure


On April 24, 2016, 1:26 p.m., Kai Uwe Broulik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127251/
> ---
> 
> (Updated April 24, 2016, 1:26 p.m.)
> 
> 
> Review request for KDE Frameworks and Vishesh Handa.
> 
> 
> Bugs: 345750
> https://bugs.kde.org/show_bug.cgi?id=345750
> 
> 
> Repository: kunitconversion
> 
> 
> Description
> ---
> 
> QNetworkReply does not implement waitForReadyRead
> 
> Also, the code never actually created the cache dir it was trying to create a 
> file in.
> 
> 
> Diffs
> -
> 
>   src/currency.cpp ad325d8 
> 
> Diff: https://git.reviewboard.kde.org/r/127251/diff/
> 
> 
> Testing
> ---
> 
> Works now. It's downloaded once and then taken from cache file in 
> ~/.local/share/libkunitconversion/currency.xml
> 
> Given it's a Tier 2 framework doesn't make sense to add KIO now, also failed 
> to reproduce the crashes mentioned in the code.
> 
> Tests pass (only if I run them on English locale btw)
> 
> Obviously not happy with this being sync but alas that's how the API works.
> 
> Not sure if this is a KRunner bug or KUnitConverison but if I enter "5 USD" 
> it converts fine, if I enter "5 usd" it returns zero for all the currencies 
> it converted to.
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127251: [KUnitConversion] Fix downloading currency exchange rates

2016-04-24 Thread Kai Uwe Broulik 

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127251/
---

(Updated April 24, 2016, 1:26 nachm.)


Review request for KDE Frameworks and Vishesh Handa.


Changes
---

Use QSaveFile


Bugs: 345750
https://bugs.kde.org/show_bug.cgi?id=345750


Repository: kunitconversion


Description
---

QNetworkReply does not implement waitForReadyRead

Also, the code never actually created the cache dir it was trying to create a 
file in.


Diffs (updated)
-

  src/currency.cpp ad325d8 

Diff: https://git.reviewboard.kde.org/r/127251/diff/


Testing
---

Works now. It's downloaded once and then taken from cache file in 
~/.local/share/libkunitconversion/currency.xml

Given it's a Tier 2 framework doesn't make sense to add KIO now, also failed to 
reproduce the crashes mentioned in the code.

Tests pass (only if I run them on English locale btw)

Obviously not happy with this being sync but alas that's how the API works.

Not sure if this is a KRunner bug or KUnitConverison but if I enter "5 USD" it 
converts fine, if I enter "5 usd" it returns zero for all the currencies it 
converted to.


Thanks,

Kai Uwe Broulik

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

KF5 Tags tag tags, not objects

2016-04-24 Thread Stephen Kelly 
If you look in gitk at karchive (or any other repo), you can see that commit 
d2e7c78c94e6 is tagged with both v5.21.0-rc2 and v5.21.0.

If you click on the v5.21.0-rc2 tag, you can see that it tags the commit. 
That is the usual thing to do.


Tag: v5.21.0-rc2
object d2e7c78c94e629a60bcff7b5c1abaf43e8ac7d75
type commit   # << GOOD
tag v5.21.0-rc2
tagger l10n daemon script  1459716237 +

It says it tags the d2e7c78c94e629a60bcff7b5c1abaf43e8ac7d75 object, which 
is the commit.

However, click on the v5.21.0 to see that it tags a tag

Tag: v5.21.0
object 6122c6149595f5f915c99ca4c7bd82e6415f1482
type tag # << BAD
tag v5.21.0
tagger l10n daemon script  1460196978 +

Create tag for 5.21.0

We can see what the 6122c6149595 object is:

$ git cat-file -p 6122c6149595f5f915c99ca4c7bd82e6415f1482
object d2e7c78c94e629a60bcff7b5c1abaf43e8ac7d75
type commit
tag v5.21.0-rc2
tagger l10n daemon script  1459716237 +

Create tag for 5.21.0


So, v5.21.0 is a tag on the 6122c6149595 object, which is a tag on the 
d2e7c78c94e6 commit.

Why doesn't v5.21.0 tag the d2e7c78c94e6 commit directly instead?

Thanks,

Steve.


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel