Jenkins-kde-ci: kxmlrpcclient master stable-kf5-qt5 » Linux,gcc - Build # 491 - Failure!

[no-reply]

GENERAL INFO

BUILD FAILURE
Build URL: 
https://build.kde.org/job/kxmlrpcclient%20master%20stable-kf5-qt5/PLATFORM=Linux,compiler=gcc/491/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Mon, 17 Apr 2017 03:09:43 +
Build duration: 1 min 18 sec

CHANGE SET
No changes

Re: KAuth buildability: new CI architecture

[Ben Cooksley]

On Mon, Apr 17, 2017 at 12:48 AM, Martin Gräßlin  wrote:
> Am 2017-04-16 13:52, schrieb Ben Cooksley:
>>
>> On Sun, Apr 16, 2017 at 11:09 PM, Harald Sitter  wrote:
>>>
>>> Not particularly related to the issue at hand (which is probably
>>> polkitqt having meh cmake files), but relocating stuff in general is
>>> sper unreliable and I would absolutely advise against it as it can
>>> easily screw up test results and builds alike, often in unobvious ways
>>> (all it takes is a bit of libexec use). Instead, as a general
>>> principle, I would suggest that you get stuff mounted in suitably
>>> stable/consistent/generic paths inside the build containers.
>>> Ultimately what things look like natively on the host file system
>>> shouldn't factor into what they look like in the build environment.
>>
>>
>> While I have thought of using a consistent path, this would simply
>> workaround the fact that our binaries are frail and have hardcoded
>> paths baked into them.
>
>
> note that this is partially intended for security reasons. E.g.
> kscreenlocker starts kcheckpass through a hardcoded path for security
> reasons (we want to be sure it's the kcheckpass we created and not a
> fakekcheckpass injected by a malicious tool). So overall especially in
> plasma lots of Wayland stuff is hardcoding paths for this reason and
> partially they are used in testing.
>
> In the case of kscreenlocker this can become a problem when KWin tests are
> run. We have tests verify that locking the screen works on Wayland. For that
> kscreenlocker_greet gets started and that has a hardcoded path as well. So
> if the paths is relocated we test an unsupported setup.

Would it be possible to use relative-to-calling-binary paths? The new
CI scripts do this to find their configuration and other resources
they need so it should be doable (note that the new CI environment
unpacks everything into the same directory when assembling an install
prefix so one can run with that assumption - and it's one that should
be true on 99% of systems out there)

This should meet the security requirements (if someone is running
their own kscreenlocker / kwin / etc binaries then chances are they
can modify the executable directly anyway)

>
> Cheers
> Martin

Cheers,
Ben

Threadweaver compilation failure: Windows

[Ben Cooksley]

Hi all,

If someone could take a look at the following build log that would be
appreciated:
https://paste.kde.org/pzyhxydjw/xxx39x/raw

Cheers,
Ben

Jenkins-kde-ci: kio master stable-kf5-qt5 » Linux,gcc - Build # 519 - Fixed!

[no-reply]

GENERAL INFO

BUILD SUCCESS
Build URL: 
https://build.kde.org/job/kio%20master%20stable-kf5-qt5/PLATFORM=Linux,compiler=gcc/519/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Mon, 17 Apr 2017 02:00:38 +
Build duration: 29 min

CHANGE SET
Revision 45bff508278963b4e19da9222e0562a84e590f1d by scripty: (SVN_SILENT made 
messages (.desktop file) - always resolve ours)
  change: edit src/kioexec/kioexecd.json


JUNIT RESULTS

Name: (root) Failed: 0 test(s), Passed: 53 test(s), Skipped: 0 test(s), Total: 
53 test(s)

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 22/22 (100%)FILES 275/344 (80%)CLASSES 275/344 (80%)LINE 30013/52022 
(58%)CONDITIONAL 16430/39075 (42%)

By packages
  
autotests
FILES 66/66 (100%)CLASSES 66/66 (100%)LINE 7968/8288 
(96%)CONDITIONAL 4445/8694 (51%)
autotests.http
FILES 9/9 (100%)CLASSES 9/9 (100%)LINE 543/544 
(100%)CONDITIONAL 200/336 (60%)
autotests.kcookiejar
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 179/198 (90%)CONDITIONAL 
60/90 (67%)
src.core
FILES 97/117 (83%)CLASSES 97/117 (83%)LINE 8130/14183 
(57%)CONDITIONAL 4447/9267 (48%)
src.core.kssl
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 35/93 (38%)CONDITIONAL 
3/6 (50%)
src.filewidgets
FILES 26/36 (72%)CLASSES 26/36 (72%)LINE 3468/7561 
(46%)CONDITIONAL 1292/4381 (29%)
src.gui
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 104/110 (95%)CONDITIONAL 
46/72 (64%)
src.ioslaves.file
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 447/849 (53%)CONDITIONAL 
330/749 (44%)
src.ioslaves.http
FILES 8/8 (100%)CLASSES 8/8 (100%)LINE 1759/3780 
(47%)CONDITIONAL 1266/3460 (37%)
src.ioslaves.http.kcookiejar
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 621/782 (79%)CONDITIONAL 
607/839 (72%)
src.ioslaves.remote
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 70/258 (27%)CONDITIONAL 
14/200 (7%)
src.ioslaves.trash
FILES 8/10 (80%)CLASSES 8/10 (80%)LINE 737/1173 
(63%)CONDITIONAL 420/851 (49%)
src.ioslaves.trash.tests
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 707/785 (90%)CONDITIONAL 
461/970 (48%)
src.kioslave
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 14/27 (52%)CONDITIONAL 
5/10 (50%)
src.kntlm
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 373/385 (97%)CONDITIONAL 
111/138 (80%)
src.kpasswdserver
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 377/594 (63%)CONDITIONAL 
280/580 (48%)
src.kpasswdserver.autotests
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 283/286 (99%)CONDITIONAL 
144/256 (56%)
src.urifilters.fixhost
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 25/34 (74%)CONDITIONAL 
36/54 (67%)
src.urifilters.ikws
FILES 5/10 (50%)CLASSES 5/10 (50%)LINE 242/727 (33%)CONDITIONAL 
150/546 (27%)
src.urifilters.localdomain
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 21/29 (72%)CONDITIONAL 
16/26 (62%)
src.urifilters.shorturi
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 237/266 (89%)CONDITIONAL 
332/412 (81%)
src.widgets
FILES 32/64 (50%)CLASSES 32/64 (50%)LINE 3673/11070 
(33%)CONDITIONAL 1765/7138 (25%)

Jenkins-kde-ci: kio master stable-kf5-qt5 » Linux,gcc - Build # 519 - Fixed!

[no-reply]

GENERAL INFO

BUILD SUCCESS
Build URL: 
https://build.kde.org/job/kio%20master%20stable-kf5-qt5/PLATFORM=Linux,compiler=gcc/519/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Mon, 17 Apr 2017 02:00:38 +
Build duration: 29 min

CHANGE SET
Revision 45bff508278963b4e19da9222e0562a84e590f1d by scripty: (SVN_SILENT made 
messages (.desktop file) - always resolve ours)
  change: edit src/kioexec/kioexecd.json


JUNIT RESULTS

Name: (root) Failed: 0 test(s), Passed: 53 test(s), Skipped: 0 test(s), Total: 
53 test(s)

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 22/22 (100%)FILES 275/344 (80%)CLASSES 275/344 (80%)LINE 30013/52022 
(58%)CONDITIONAL 16430/39075 (42%)

By packages
  
autotests
FILES 66/66 (100%)CLASSES 66/66 (100%)LINE 7968/8288 
(96%)CONDITIONAL 4445/8694 (51%)
autotests.http
FILES 9/9 (100%)CLASSES 9/9 (100%)LINE 543/544 
(100%)CONDITIONAL 200/336 (60%)
autotests.kcookiejar
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 179/198 (90%)CONDITIONAL 
60/90 (67%)
src.core
FILES 97/117 (83%)CLASSES 97/117 (83%)LINE 8130/14183 
(57%)CONDITIONAL 4447/9267 (48%)
src.core.kssl
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 35/93 (38%)CONDITIONAL 
3/6 (50%)
src.filewidgets
FILES 26/36 (72%)CLASSES 26/36 (72%)LINE 3468/7561 
(46%)CONDITIONAL 1292/4381 (29%)
src.gui
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 104/110 (95%)CONDITIONAL 
46/72 (64%)
src.ioslaves.file
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 447/849 (53%)CONDITIONAL 
330/749 (44%)
src.ioslaves.http
FILES 8/8 (100%)CLASSES 8/8 (100%)LINE 1759/3780 
(47%)CONDITIONAL 1266/3460 (37%)
src.ioslaves.http.kcookiejar
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 621/782 (79%)CONDITIONAL 
607/839 (72%)
src.ioslaves.remote
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 70/258 (27%)CONDITIONAL 
14/200 (7%)
src.ioslaves.trash
FILES 8/10 (80%)CLASSES 8/10 (80%)LINE 737/1173 
(63%)CONDITIONAL 420/851 (49%)
src.ioslaves.trash.tests
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 707/785 (90%)CONDITIONAL 
461/970 (48%)
src.kioslave
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 14/27 (52%)CONDITIONAL 
5/10 (50%)
src.kntlm
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 373/385 (97%)CONDITIONAL 
111/138 (80%)
src.kpasswdserver
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 377/594 (63%)CONDITIONAL 
280/580 (48%)
src.kpasswdserver.autotests
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 283/286 (99%)CONDITIONAL 
144/256 (56%)
src.urifilters.fixhost
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 25/34 (74%)CONDITIONAL 
36/54 (67%)
src.urifilters.ikws
FILES 5/10 (50%)CLASSES 5/10 (50%)LINE 242/727 (33%)CONDITIONAL 
150/546 (27%)
src.urifilters.localdomain
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 21/29 (72%)CONDITIONAL 
16/26 (62%)
src.urifilters.shorturi
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 237/266 (89%)CONDITIONAL 
332/412 (81%)
src.widgets
FILES 32/64 (50%)CLASSES 32/64 (50%)LINE 3673/11070 
(33%)CONDITIONAL 1765/7138 (25%)

Jenkins-kde-ci: kio master kf5-qt5 » Linux,gcc - Build # 518 - Unstable!

[no-reply]

GENERAL INFO

BUILD UNSTABLE
Build URL: 
https://build.kde.org/job/kio%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/518/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Mon, 17 Apr 2017 02:00:38 +
Build duration: 24 min

CHANGE SET
Revision 45bff508278963b4e19da9222e0562a84e590f1d by scripty: (SVN_SILENT made 
messages (.desktop file) - always resolve ours)
  change: edit src/kioexec/kioexecd.json


JUNIT RESULTS

Name: (root) Failed: 1 test(s), Passed: 52 test(s), Skipped: 0 test(s), Total: 
53 test(s)Failed: TestSuite.kiocore-threadtest

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 22/22 (100%)FILES 275/343 (80%)CLASSES 275/343 (80%)LINE 30094/51987 
(58%)CONDITIONAL 16466/39053 (42%)

By packages
  
autotests
FILES 65/65 (100%)CLASSES 65/65 (100%)LINE 7933/8253 
(96%)CONDITIONAL 4431/8672 (51%)
autotests.http
FILES 9/9 (100%)CLASSES 9/9 (100%)LINE 543/544 
(100%)CONDITIONAL 200/336 (60%)
autotests.kcookiejar
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 179/198 (90%)CONDITIONAL 
60/90 (67%)
src.core
FILES 97/117 (83%)CLASSES 97/117 (83%)LINE 8136/14183 
(57%)CONDITIONAL 4448/9267 (48%)
src.core.kssl
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 35/93 (38%)CONDITIONAL 
3/6 (50%)
src.filewidgets
FILES 26/36 (72%)CLASSES 26/36 (72%)LINE 3465/7561 
(46%)CONDITIONAL 1294/4381 (30%)
src.gui
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 104/110 (95%)CONDITIONAL 
46/72 (64%)
src.ioslaves.file
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 447/849 (53%)CONDITIONAL 
330/749 (44%)
src.ioslaves.http
FILES 8/8 (100%)CLASSES 8/8 (100%)LINE 1759/3780 
(47%)CONDITIONAL 1266/3460 (37%)
src.ioslaves.http.kcookiejar
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 621/782 (79%)CONDITIONAL 
607/839 (72%)
src.ioslaves.remote
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 70/258 (27%)CONDITIONAL 
14/200 (7%)
src.ioslaves.trash
FILES 8/10 (80%)CLASSES 8/10 (80%)LINE 737/1173 
(63%)CONDITIONAL 420/851 (49%)
src.ioslaves.trash.tests
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 707/785 (90%)CONDITIONAL 
461/970 (48%)
src.kioslave
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 14/27 (52%)CONDITIONAL 
5/10 (50%)
src.kntlm
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 373/385 (97%)CONDITIONAL 
111/138 (80%)
src.kpasswdserver
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 377/594 (63%)CONDITIONAL 
280/580 (48%)
src.kpasswdserver.autotests
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 283/286 (99%)CONDITIONAL 
146/256 (57%)
src.urifilters.fixhost
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 25/34 (74%)CONDITIONAL 
36/54 (67%)
src.urifilters.ikws
FILES 5/10 (50%)CLASSES 5/10 (50%)LINE 242/727 (33%)CONDITIONAL 
150/546 (27%)
src.urifilters.localdomain
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 21/29 (72%)CONDITIONAL 
16/26 (62%)
src.urifilters.shorturi
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 237/266 (89%)CONDITIONAL 
332/412 (81%)
src.widgets
FILES 33/64 (52%)CLASSES 33/64 (52%)LINE 3786/11070 
(34%)CONDITIONAL 1810/7138 (25%)

D5394: KAuth integration in document saving - vol. 2

[Fabian Vogt]

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


  Thanks!
  
  So far I only found two issues (see comments).
  Apart from that it would be great to see the application name and the 
target/source file in the polkit dialog, but I assume this is out of scope here.

INLINE COMMENTS

> katesecuretextbuffer.cpp:88
> +// ensure file has the same owner and group as before
> +setOwner(tempFile.fileName(), ownerId, groupId);
>  }

This is racy: If the newly set permissions allow someone to delete the file, it 
can be replaced with a symlink and the chown will take effect on the symlink 
target, which can be literally anything -> escalation.

This is not an issue for the rename call as if the file permissions allow 
deleting, they allow deleting for the destination file as well -> no escalation.

Solution: Use fchown.

> katesecuretextbuffer.cpp:92
> +// rename temporary file to the target file
> +return moveFile(tempFile.fileName(), targetFile);
>  }

The destructor of QTemporaryFile here tries to unlink the temporary file here, 
which fails if the rename was successful.

REPOSITORY
  R39 KTextEditor

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

To: martinkostolny, #ktexteditor, fvogt
Cc: elvisangelaccio, aacid, ivan, lbeltrame, fvogt, apol, anthonyfieroni, 
cullmann, ltoscano, dhaumann, graesslin, davidedmundson, palant, kwrite-devel, 
dfaure, #frameworks, head7, kfunk, sars

D5456: KDoctools: build on Mac with docbook from homebrew

[Allen Winter]

winterz closed this revision.
winterz added a comment.


  committed in 
https://phabricator.kde.org/R238:5c5bfc2d838993f7d4be1885dff822e3794c529f

REPOSITORY
  R238 KDocTools

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

To: winterz, ltoscano, kfunk
Cc: kfunk, #frameworks, #documentation, skadinna

D5456: KDoctools: build on Mac with docbook from homebrew

[Luigi Toscano]

ltoscano accepted this revision.

REPOSITORY
  R238 KDocTools

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

To: winterz, ltoscano, kfunk
Cc: kfunk, #frameworks, #documentation, skadinna

D5456: KDoctools: build on Mac with docbook from homebrew

[Allen Winter]

winterz updated this revision to Diff 13520.
winterz added a comment.


  I added comments.

REPOSITORY
  R238 KDocTools

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5456?vs=13448=13520

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

AFFECTED FILES
  cmake/FindDocBookXML4.cmake
  cmake/FindDocBookXSL.cmake

To: winterz, ltoscano, kfunk
Cc: kfunk, #frameworks, #documentation, skadinna

Re: Review Request 130058: Make kwalletd5 service both org.kde.kwalletd5 and org.kde.kwalletd

[Andreas Sturmlechner]

> On April 1, 2017, 7:59 p.m., David Faure wrote:
> > Makes sense to me, +1.
> 
> Andreas Sturmlechner wrote:
> Thanks, anyone else who wants to +1?
> 
> I've tried to test migration today but it didn't work. May as well have 
> nothing to do with te patch and be caused by the permanently troubled 
> migration agent though... KMail happily gets its IMAP password from kwallet5 
> though after manually export/import via XML files.
> 
> Andreas Sturmlechner wrote:
> As suspected, on my test system migration is broken regardless of with 
> these patches or not.
> 
> David Faure wrote:
> Are you planning on looking into that? ;-)
> 
> These patches are related to migration, it feels a bit wrong to commit 
> changes around migration and still leave it broken.

I don't feel like I'm up to that task. Also, the reason for why it works for 
some, but not all the systems, has afaik never been established.


- Andreas


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


On March 29, 2017, 12:52 p.m., Andreas Sturmlechner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/130058/
> ---
> 
> (Updated March 29, 2017, 12:52 p.m.)
> 
> 
> Review request for KDE Frameworks and Stefan Brüns.
> 
> 
> Repository: kwallet
> 
> 
> Description
> ---
> 
> These are not my own patches, I'm creating this review request after having 
> been made aware of *kwalletd4_dbus_compat* branch in kwallet.git, which I 
> simply rebased on top of current master (author of course preserved) to be 
> able to test it. I think it would be a great improvement over the current 
> situation that is rather confusing to the users.
> 
> The changes are organised in 5 commits:
> 
> - Check for unique applicaton instance as early as possible
> Exit before KWalletD and the MigrationAgent has been initialized.
> The return value is changed, but concurrent instatiation of kwalletd is
> not a fault.
> 
> - Only start timer for migration agent if necessary
> - Whitespace fixup
> - Signal completion of migration agent
> - Replace kwalletd4 after migration has finished
> kwalletd5 can service both org.kde.kwalletd5 and org.kde.kwalletd
> 
> 
> Diffs
> -
> 
>   src/runtime/kwalletd/kwalletd.h 3571535cd8bd78415002795f9b61adf9f6cfb8c1 
>   src/runtime/kwalletd/kwalletd.cpp 18ef9fa7e6ddaeba6e0b32deae3de1dae39df5bb 
>   src/runtime/kwalletd/main.cpp ff9620886fa1959e14b594be6bbb4644b637c000 
>   src/runtime/kwalletd/migrationagent.h 
> 0f6467c1753ef34b7f7f7e282503ec5607927db9 
>   src/runtime/kwalletd/migrationagent.cpp 
> f3da94743ecd83fe406e058f560d4238caec1be8 
> 
> Diff: https://git.reviewboard.kde.org/r/130058/diff/
> 
> 
> Testing
> ---
> 
> Migration itself was not tested so far, but a legacy application like ksirk 
> was able to create a new wallet just fine and can access it as well. I do not 
> have kwalletd4 installed anymore.
> 
> 
> Thanks,
> 
> Andreas Sturmlechner
> 
>

D5394: KAuth integration in document saving - vol. 2

[Martin Kostolný]

martinkostolny updated this revision to Diff 13517.
martinkostolny added a comment.


  Updated diff based on Fabian's advisory. Thanks, Fabian!
  
  I've implemented the first option: checksum -> tempfile1 -> read in 
privileged action -> checksum -> tempfile2 -> permissions -> rename. It works 
with big (e.g. 50MiB) files. I hope I didn't miss something.
  
  Known issues:
  
  - Using std::rename only for unix systems while using racy QFile's remove && 
rename for windows as fallback. I cannot test on windows otherwise I'd 
implemented an atomic rename there as well.
  - I'm now using QBuffer to buffer all file bytes before making the first 
checksum. Probably the best solution (memory-wise) would be to use 
QTemporaryFile directly and capture all written bytes right before they are 
written to the file and make the checksum from it. But I didn't find an easy 
way to do that.

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5394?vs=13314=13517

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

AFFECTED FILES
  src/buffer/katesecuretextbuffer.cpp
  src/buffer/katesecuretextbuffer_p.h
  src/buffer/katetextbuffer.cpp
  src/buffer/katetextbuffer.h

To: martinkostolny, #ktexteditor, fvogt
Cc: elvisangelaccio, aacid, ivan, lbeltrame, fvogt, apol, anthonyfieroni, 
cullmann, ltoscano, dhaumann, graesslin, davidedmundson, palant, kwrite-devel, 
dfaure, #frameworks, head7, kfunk, sars

Re: KAuth buildability: new CI architecture

[Martin Gräßlin]

Am 2017-04-16 13:52, schrieb Ben Cooksley:

On Sun, Apr 16, 2017 at 11:09 PM, Harald Sitter  wrote:

Not particularly related to the issue at hand (which is probably
polkitqt having meh cmake files), but relocating stuff in general is
sper unreliable and I would absolutely advise against it as it can
easily screw up test results and builds alike, often in unobvious ways
(all it takes is a bit of libexec use). Instead, as a general
principle, I would suggest that you get stuff mounted in suitably
stable/consistent/generic paths inside the build containers.
Ultimately what things look like natively on the host file system
shouldn't factor into what they look like in the build environment.


While I have thought of using a consistent path, this would simply
workaround the fact that our binaries are frail and have hardcoded
paths baked into them.


note that this is partially intended for security reasons. E.g. 
kscreenlocker starts kcheckpass through a hardcoded path for security 
reasons (we want to be sure it's the kcheckpass we created and not a 
fakekcheckpass injected by a malicious tool). So overall especially in 
plasma lots of Wayland stuff is hardcoding paths for this reason and 
partially they are used in testing.


In the case of kscreenlocker this can become a problem when KWin tests 
are run. We have tests verify that locking the screen works on Wayland. 
For that kscreenlocker_greet gets started and that has a hardcoded path 
as well. So if the paths is relocated we test an unsupported setup.


Cheers
Martin

Re: KAuth buildability: new CI architecture

[Ben Cooksley]

On Sun, Apr 16, 2017 at 11:09 PM, Harald Sitter  wrote:
> Not particularly related to the issue at hand (which is probably
> polkitqt having meh cmake files), but relocating stuff in general is
> sper unreliable and I would absolutely advise against it as it can
> easily screw up test results and builds alike, often in unobvious ways
> (all it takes is a bit of libexec use). Instead, as a general
> principle, I would suggest that you get stuff mounted in suitably
> stable/consistent/generic paths inside the build containers.
> Ultimately what things look like natively on the host file system
> shouldn't factor into what they look like in the build environment.

While I have thought of using a consistent path, this would simply
workaround the fact that our binaries are frail and have hardcoded
paths baked into them.

The CI has always run an unusual configuration as part of it's job is
to find brittle parts of our codebase and make the breakage visible.

Outside of traditionally provided FOSS packaging, relocatable binaries
are something we'll need - for Windows packages for instance as users
there expect to be able to specify the installation directory (on that
note - log for kcoreaddons on Windows is attached with several test
failures, and it looks like the logic around ASAN on Windows needs
adjustment too).

We'll see how things go.

>
> For example, in neon we simply mount everything into /workspace of our
> containers.
>
> You could have
>
> /workspace
> |_ src/  [on host: /home/jenkins/workspace/polkit-qt-1 kf5-qt5 
> XenialQt5.7/
> |_ install/  [on host: /home/jenkins/workspace/polkit-qt-1 kf5-qt5
> XenialQt5.7/install-prefix/]

The workspaces are, on the Linux systems at least, thrown away at the
completion of a job. The only thing that survives is a tarball archive
of the files installed by the job.

>
> Notably advantage is that relocation issue get entirely eliminated as
> the install prefix is always the same, and as an added bonus paths
> become much shorter and easier to read in the build logs.
>
> It also detaches the build tooling from the host tooling. e.g. the
> build code at this point no longer needs to care about which path
> Jenkins was installed to, or where it was configured to put
> workspaces, or what the jenkins job name is, or if jenkins is even
> still used.
>
> Food for thought :)
>
> HS

Cheers,
Ben


kcoreaddons-windows-build.log
Description: Binary data

Re: KAuth buildability: new CI architecture

[Harald Sitter]

Not particularly related to the issue at hand (which is probably
polkitqt having meh cmake files), but relocating stuff in general is
sper unreliable and I would absolutely advise against it as it can
easily screw up test results and builds alike, often in unobvious ways
(all it takes is a bit of libexec use). Instead, as a general
principle, I would suggest that you get stuff mounted in suitably
stable/consistent/generic paths inside the build containers.
Ultimately what things look like natively on the host file system
shouldn't factor into what they look like in the build environment.

For example, in neon we simply mount everything into /workspace of our
containers.

You could have

/workspace
|_ src/  [on host: /home/jenkins/workspace/polkit-qt-1 kf5-qt5 XenialQt5.7/
|_ install/  [on host: /home/jenkins/workspace/polkit-qt-1 kf5-qt5
XenialQt5.7/install-prefix/]

Notably advantage is that relocation issue get entirely eliminated as
the install prefix is always the same, and as an added bonus paths
become much shorter and easier to read in the build logs.

It also detaches the build tooling from the host tooling. e.g. the
build code at this point no longer needs to care about which path
Jenkins was installed to, or where it was configured to put
workspaces, or what the jenkins job name is, or if jenkins is even
still used.

Food for thought :)

HS

D5463: Pug/Jade Syntax

[Yunhe Guo]

guoyunhe added a comment.


  In https://phabricator.kde.org/D5463#102409, @dhaumann wrote:
  
  > Do you also agree to release this highlighting file under MIT license? Same 
for the test case you attached? Also, can we use this test case for unit 
testing?
  
  
  It is okay. I can release the syntax file under LGPL, MIT, BSD ( 
https://community.kde.org/Policies/Licensing_Policy )
  
  The test file is copied from pugjs.org , I am not the author. It is MIT 
licensed 
https://github.com/pugjs/pug-www/blob/39257987117795a4c2d133618649bb89f1cc81e4/package.json

REPOSITORY
  R216 Syntax Highlighting

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

To: guoyunhe, #framework_syntax_hightlighting, #ktexteditor, #kate
Cc: dhaumann, #frameworks

D5394: KAuth integration in document saving - vol. 2

[Fabian Vogt]

fvogt added a comment.


  In https://phabricator.kde.org/D5394#102605, @dfaure wrote:
  
  > Would it help if QSaveFile had an API to set more restrictive permissions 
on the temp file?
  
  
  No, the fix is simple: Revert 
https://code.qt.io/cgit/qt/qtbase.git/commit/?id=a60571b3700e80f44705ebc4bab9628cf852891c

REPOSITORY
  R39 KTextEditor

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

To: martinkostolny, #ktexteditor, fvogt
Cc: elvisangelaccio, aacid, ivan, lbeltrame, fvogt, apol, anthonyfieroni, 
cullmann, ltoscano, dhaumann, graesslin, davidedmundson, palant, kwrite-devel, 
dfaure, #frameworks, head7, kfunk, sars

D5394: KAuth integration in document saving - vol. 2

[David Faure]

dfaure added a comment.


  Would it help if QSaveFile had an API to set more restrictive permissions on 
the temp file?

REPOSITORY
  R39 KTextEditor

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

To: martinkostolny, #ktexteditor, fvogt
Cc: elvisangelaccio, aacid, ivan, lbeltrame, fvogt, apol, anthonyfieroni, 
cullmann, ltoscano, dhaumann, graesslin, davidedmundson, palant, kwrite-devel, 
dfaure, #frameworks, head7, kfunk, sars

D5413: makes KArchive be optional and do not build extractors needing it

[Matthieu Gallien]

This revision was automatically updated to reflect the committed changes.
Closed by commit R286:599e9b6489fe: makes KArchive be optional and do not build 
extractors needing it (authored by mgallien).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D5413?vs=13496=13507#toc

REPOSITORY
  R286 KFileMetaData

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5413?vs=13496=13507

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

AFFECTED FILES
  CMakeLists.txt
  autotests/CMakeLists.txt
  src/extractors/CMakeLists.txt

To: mgallien, kfunk, dfaure
Cc: kfunk, heikobecker, dfaure, #frameworks

Re: Review Request 130084: Add a pair of flags forcing fsync during copy loop

[David Faure]

> On April 15, 2017, 8:30 a.m., David Faure wrote:
> > But doesn't this make copying much slower in the normal case? (copying onto 
> > a non-removable harddisk partition).
> > 
> > It sounds to me like this should be
> > 1) done internally in kio_file (no Job API for this)
> > 2) only when the destination is a removable device
> 
> Oswald Buddenhagen wrote:
> i've read multiple times that fsync isn't a performance problem on modern 
> file systems any more. whatever that may mean.
> limiting this to cross-device isn't really sensible imo - a) one can have 
> multiple internal disks and b) even if the disk stays in, at some point the 
> flushing will commence and will be a major slowdown for subsequent operations.
> in fact, this problem is bad enough that the linux kernel community 
> realized it (which, in the area of disk i/o, never ceases to amaze) - 
> https://lwn.net/Articles/682582/ (obvious followup question: what kernel do 
> you use? this code seems to have landed in 4.10)
> 
> KJ Tsanaktsidis wrote:
> I'm using kernel `Linux kj-hedt-arch 4.10.8-1-ARCH #1 SMP PREEMPT Fri Mar 
> 31 16:50:19 CEST 2017 x86_64 GNU/Linux`. My understanding is that the 
> patchset you're talking about will not allow synchronous reads, such as 
> faulting in application code, to get stuck behind a full writeback queue, by 
> limiting how much work the MM subsystem can send to the disk - not by 
> throttling how fast applications can dirty the device. I've not noticed any 
> problems using the disk whilst writing with or without my patch to KIO here 
> but I haven't really stressed it.
> 
> As to what the performance implications of fsync - I guess it depends how 
> much you care about what you were planning to do with the file after you copy 
> it. I implemented the "fsync if source/dest are on different filesystems" 
> logic because in that case, one of the things you might be wanting to do is 
> unmount the disk. If you wanted to interact with the file on the destination 
> system instead, this patch would make it take (much) longer. The reason I 
> implemented this with a job flag is that I was envisiging making it an option 
> in Dolphin - like the "move/copy" menu when you drop, you could also get 
> "copy with fsync" perhaps for this reason - we don't know what the user plans 
> to do with the file afterwards.
> 
> I'm happy enough to use "is the device removeable?" as a heuristic for 
> "the users next desired operation on this file is probably to unmount it" 
> instead and delete the Job API - this would address my use case at least. How 
> do you suggest I get this information in `kio_file`? On Linux it looks like I 
> can get this from sysfs `/sys/dev/block/maj:min/removeable`, but I don't know 
> how to do this for other platforms and don't have them available to test. 
> Would the patch be OK if I just added this for linux?

Solid has portable API for this. 
Something like this (completely untested, I'm no Solid expert, these are just 
old notes from a TODO)

const QString query = QLatin1String("[StorageAccess.accessible == true]");
const QList lst = Solid::Device::listFromQuery(query);
iterating and then using Solid::StorageDrive::isRemovable() || 
Solid::StorageDrive::isHotpluggable() (these checks can probably be integrated 
into the query string)


- David


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


On April 15, 2017, 8:28 a.m., KJ Tsanaktsidis wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/130084/
> ---
> 
> (Updated April 15, 2017, 8:28 a.m.)
> 
> 
> Review request for KDE Frameworks, Oswald Buddenhagen and Thiago Macieira.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> When copying a large-ish file (~1-2GB) from very fast storage to very slow 
> storage (e.g. an NVME SSD to a cheap USB flash drive) on a machine with lots 
> of RAM, Dolphin displays a progress bar which finishes in a fraction of a 
> second (i.e. as fast as it takes to read the source file into the Linux page 
> cache). Unmounting the drive then of course takes a long time, with only an 
> indeterminate spinner.
> 
> This patch adds an option to force fsync during copy jobs, so that the copy 
> progress bar measures how long it will take to actually copy the file to the 
> destination.
> 
> I've added two flags - Fsync and FsyncCrossFilesystem - to the JobEnum flag. 
> The former will cause all copy operations to fsync during the copy loop, 
> whilst the latter will only fsync copies that are across different 
> filesystems.
> 
> If this patch gets OK'd, I have another patch which adds support for this 
> into 

D5413: makes KArchive be optional and do not build extractors needing it

[David Faure]

dfaure accepted this revision.

REPOSITORY
  R286 KFileMetaData

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

To: mgallien, kfunk, dfaure
Cc: kfunk, heikobecker, dfaure, #frameworks