Re: Threadweaver compilation failure: Windows

2017-04-17 Thread Aleix Pol
On Mon, Apr 17, 2017 at 6:09 AM, Ben Cooksley  wrote:
> Hi all,
>
> If someone could take a look at the following build log that would be
> appreciated:
> https://paste.kde.org/pzyhxydjw/xxx39x/raw

Include KDE on Windows.

Aleix


D5394: KAuth integration in document saving - vol. 2

2017-04-17 Thread Martin Kostolný
martinkostolny marked 2 inline comments as done.

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

2017-04-17 Thread Martin Kostolný
martinkostolny updated this revision to Diff 13539.
martinkostolny added a comment.


  Thanks for noticing all these security issues!
  
  Both issues should now be fixed.
  
  Regarding the KAuth dialog, I agree, but couldn't find a simple way of 
propagating additional info to the dialog. I didn't dig in KAuth code much but 
so far I didn't find a way. It looks like it isn't supported. 
KAuth::Action.setDetails(...) doesn't really add anything in the dialog.

REPOSITORY
  R39 KTextEditor

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

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


KTextEditor : best way to add background vertical guide lines?

2017-04-17 Thread René J . V . Bertin
Hi,

What's the best way to add render a couple of vertical lines in a widget 
derived from KTextEditor, for instance to provide margin indicators?

Thanks,
René


D5138: Fill UDS_CREATION_TIME with the value of st_birthtime on FreeBSD

2017-04-17 Thread Adriaan de Groot
This revision was automatically updated to reflect the committed changes.
Closed by commit R241:16f69a5d0d55: Fill UDS_CREATION_TIME with the value of 
st_birthtime on FreeBSD (authored by adridg).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5138?vs=13529=13530

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

AFFECTED FILES
  src/ioslaves/file/file.cpp

To: adridg, rakuco, dfaure, arrowdodger, tcberner
Cc: aacid, kfunk, emmanuelp, #frameworks


D5138: Fill UDS_CREATION_TIME with the value of st_birthtime on FreeBSD

2017-04-17 Thread Adriaan de Groot
adridg updated this revision to Diff 13529.
adridg added a comment.


  - Brain-o on the OpenBSD code
  - Reduce whitespace changes
  - Update comment

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5138?vs=13528=13529

BRANCH
  arcpatch-D5138

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

AFFECTED FILES
  src/ioslaves/file/file.cpp

To: adridg, rakuco, dfaure, arrowdodger, tcberner
Cc: aacid, kfunk, emmanuelp, #frameworks


D5138: Fill UDS_CREATION_TIME with the value of st_birthtime on FreeBSD

2017-04-17 Thread Adriaan de Groot
adridg commandeered this revision.
adridg edited reviewers, added: tcberner; removed: adridg.

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D5138

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

To: adridg, rakuco, dfaure, arrowdodger, tcberner
Cc: aacid, kfunk, emmanuelp, #frameworks


D5138: Fill UDS_CREATION_TIME with the value of st_birthtime on FreeBSD

2017-04-17 Thread Adriaan de Groot
adridg updated this revision to Diff 13528.
adridg added a comment.


  Take over the review so as to land it, based on tcberner's comments
  from april 6th, and Albert's reminder.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5138?vs=12745=13528

BRANCH
  arcpatch-D5138

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

AFFECTED FILES
  src/ioslaves/file/config-kioslave-file.h.cmake
  src/ioslaves/file/file.cpp

To: tcberner, rakuco, adridg, dfaure, arrowdodger
Cc: aacid, kfunk, emmanuelp, #frameworks


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

2017-04-17 Thread no-reply

GENERAL INFO

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

CHANGE SET
Revision 1740dae897ad8f78192093788f25290ef74e205a by Albert Astals Cid: (http 
slave: send error page after authorization failure)
  change: edit src/ioslaves/http/http.cpp


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 30008/52023 
(58%)CONDITIONAL 16434/39077 (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 8131/14183 
(57%)CONDITIONAL 4449/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 3470/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/3781 
(47%)CONDITIONAL 1266/3462 (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 32/64 (50%)CLASSES 32/64 (50%)LINE 3665/11070 
(33%)CONDITIONAL 1763/7138 (25%)

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

2017-04-17 Thread no-reply

GENERAL INFO

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

CHANGE SET
Revision 1740dae897ad8f78192093788f25290ef74e205a by Albert Astals Cid: (http 
slave: send error page after authorization failure)
  change: edit src/ioslaves/http/http.cpp


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 30008/52023 
(58%)CONDITIONAL 16434/39077 (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 8131/14183 
(57%)CONDITIONAL 4449/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 3470/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/3781 
(47%)CONDITIONAL 1266/3462 (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 32/64 (50%)CLASSES 32/64 (50%)LINE 3665/11070 
(33%)CONDITIONAL 1763/7138 (25%)

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

2017-04-17 Thread KJ Tsanaktsidis

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

(Updated April 17, 2017, 11:27 a.m.)


Review request for KDE Frameworks, Oswald Buddenhagen and Thiago Macieira.


Changes
---

Removed the Job API changes; fsync is now always used when the target is on 
removeable media


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 
the appropriate places in Dolphin. I would think that at least 
FsyncCrossFilesystem should be the default, but Fsync always might be a little 
heavy handed. At the least fsync'ing cross-filesystem copies ensures that the 
unmount won't take forever.


Diffs (updated)
-

  src/ioslaves/file/CMakeLists.txt b9132ced9d4a08b2cf9f9bbbaa3bd43f026c6469 
  src/ioslaves/file/ConfigureChecks.cmake 
5a83d1b9fbe90c851c774e3b467468d93b5a2bd4 
  src/ioslaves/file/config-kioslave-file.h.cmake 
372f79d01ad4597aae0b2ae62627648fe7680b64 
  src/ioslaves/file/file_unix.cpp 3c1b9927e3dd2d0134f77caec6e6b24a0356d26f 

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


Testing
---

Tested the patch with KDE/Dolphin on Arch Linux, which is version 5.32.0. The 
diff applies cleanly to master so I assume there shouldn't be any issues there, 
but I've not actually checked that. As advertised, copying a file to USB flash 
storage now displays an accurate progress bar.

I experimented with how often fsync should be called on my hardware, and I 
found calling it every ~1M copied caused no decrease in copy performance whilst 
still providing accurate progress info. That is the setting I've gone with in 
this patch. I'm open to suggestions on how this could be tuned better though.


Thanks,

KJ Tsanaktsidis



D5138: Fill UDS_CREATION_TIME with the value of st_birthtime on FreeBSD

2017-04-17 Thread Albert Astals Cid
aacid added a comment.


  @tcberner Can you mark this as "Request Changes" then so it doens't show up 
in the list of "this has been approved to land but still noone has commited it 
yet" list?

REPOSITORY
  R241 KIO

BRANCH
  master

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

To: tcberner, rakuco, adridg, dfaure, arrowdodger
Cc: aacid, kfunk, emmanuelp, #frameworks


D5368: http slave: send error page after authorization failure

2017-04-17 Thread Albert Astals Cid
This revision was automatically updated to reflect the committed changes.
Closed by commit R241:1740dae897ad: http slave: send error page after 
authorization failure (authored by schwab, committed by aacid).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5368?vs=13266=13526

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

AFFECTED FILES
  src/ioslaves/http/http.cpp

To: schwab, #frameworks, dfaure
Cc: dfaure, ltoscano


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

2017-04-17 Thread no-reply

GENERAL INFO

BUILD SUCCESS
Build URL: 
https://build.kde.org/job/kxmlrpcclient%20master%20stable-kf5-qt5/PLATFORM=Linux,compiler=gcc/492/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Mon, 17 Apr 2017 10:01:48 +
Build duration: 6 min 32 sec

CHANGE SET
No changes


JUNIT RESULTS

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

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 2/2 (100%)FILES 7/7 (100%)CLASSES 7/7 (100%)LINE 303/422 
(72%)CONDITIONAL 109/208 (52%)

By packages
  
autotests
FILES 4/4 (100%)CLASSES 4/4 (100%)LINE 149/149 
(100%)CONDITIONAL 32/62 (52%)
src
FILES 3/3 (100%)CLASSES 3/3 (100%)LINE 154/273 (56%)CONDITIONAL 
77/146 (53%)

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

2017-04-17 Thread no-reply

GENERAL INFO

BUILD SUCCESS
Build URL: 
https://build.kde.org/job/kxmlrpcclient%20master%20stable-kf5-qt5/PLATFORM=Linux,compiler=gcc/492/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Mon, 17 Apr 2017 10:01:48 +
Build duration: 6 min 32 sec

CHANGE SET
No changes


JUNIT RESULTS

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

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 2/2 (100%)FILES 7/7 (100%)CLASSES 7/7 (100%)LINE 303/422 
(72%)CONDITIONAL 109/208 (52%)

By packages
  
autotests
FILES 4/4 (100%)CLASSES 4/4 (100%)LINE 149/149 
(100%)CONDITIONAL 32/62 (52%)
src
FILES 3/3 (100%)CLASSES 3/3 (100%)LINE 154/273 (56%)CONDITIONAL 
77/146 (53%)

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

2017-04-17 Thread KJ Tsanaktsidis


> 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?
> 
> David Faure wrote:
> 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)

OK - that seems reasonable enough. I guess I should be able to search Solid's 
disks for the major/minor numbers from `fstat`'s `st_dev`. Whilst doing this I 
ran into a bug with Solid, which I've proposed a patch for here: 
https://git.reviewboard.kde.org/r/130090/. If that gets OK'd I'll come back to 
finishing this off :)


- KJ


---
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 

Review Request 130090: Fix incorrect definition of major(3)/minor(3) macros

2017-04-17 Thread KJ Tsanaktsidis

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

Review request for KDE Frameworks.


Repository: solid


Description
---

Previously, udesksblock.cpp was attempting to find a definition for
major/minor on Linux in  by checking Q_OS_LINUX before
importing the header. Q_OS_LINUX is however only set when
qsystemdetection.h is included, and the macro was being checked first.

Even had this check worked, it would still be wrong. On a modern version
of the userspace linux-headers,  includes definitions for
major and minor that assume each is limited to 8 bits and that dev_t is
16 bits. This is no longer true anymore; on Linux, major numbers can be
up to 12 bits at present and minor numbers up to 20. Calling these
macros with dev_t values > 2^16 would give incorrect results.

Because the Q_OS_LINUX check failed, a fallback version of the macros
were defined for use on all platforms. The code is allegedly copied from
kdev_t.h, except it is copied from the *kernel* version of the header,
not the userspace version. Linux internally uses a different
representation of dev_t than it exposes to userspace - the kernelspace
version is 20 bits of minor/12 bits of major contiguously, but the
userspace version packs the bits in a different order to maintain
compatability with old 16-bit device numbers. Thus, this code also does
not work for dev_t values > 2^16.

To fix this, we add CMake rules to search for a system-provided
definition of the major/minor macros - on various systems, these can be
in a few different places. As a fallback, we assume old-style 16-bit
dev_t (although I suspect that is only used for Windows, where
major/minor numbers are pretty meaningless anyway).


Diffs
-

  src/solid/devices/backends/udisks2/CMakeLists.txt 
34390064af29ace07cbb3470945be098cc606d04 
  src/solid/devices/backends/udisks2/udisksblock.cpp 
0622ec77fcf670a2005d34b7a6c31ca8b53a18d8 

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


Testing
---

I've written a little snippet to iterate through block devices, print their 
major/minor number, and their device properties. It was previously incorrectly 
labeling all my disks with major 0 and minor == device_number (since it was 
using the first 20 bits for the minor). It now correctly identifies their 
major/minor number.


Thanks,

KJ Tsanaktsidis



Re: KAuth buildability: new CI architecture

2017-04-17 Thread Harald Sitter
On Mon, Apr 17, 2017 at 9:30 AM, Martin Gräßlin  wrote:
>> Would it be possible to use relative-to-calling-binary paths?
>
>
> Simply put: no. That would require quite some engineering effort especially
> considering that distros do have the libexec paths different with some
> adding the arch into it. This makes it almost impossible to get it right.

```
QString path;
char *bin_path = realpath("/proc/self/exe", NULL);
if (bin_path == NULL) {
path = QString::fromLatin1(KDE_INSTALL_FULL_LIBEXEC);
} else {
QFileInfo info(QString::fromLatin1(bin_path));
path = info.dir().relativeFilePath(QLatin1String(KDE_INSTALL_FULL_LIBEXEC));
}
auto helper = relativePath + QLatin1String("/magicExecutable");
```

We always know where stuff is installed to... we install it.

Not wanting to add the relative resolution code is, of course, a
different story.

HS


Re: KAuth buildability: new CI architecture

2017-04-17 Thread Martin Gräßlin

Am 2017-04-17 06:49, schrieb 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?


Simply put: no. That would require quite some engineering effort 
especially considering that distros do have the libexec paths different 
with some adding the arch into it. This makes it almost impossible to 
get it right.


So in the case of kscreenlocker I have absolute zero interest in 
re-engineering it to support relative paths. The current code works and 
we can just assume that it is there. Anything else requires error 
handling and if something goes wrong users have a system which cannot be 
unlocked. The risk is not worth the effort.


Cheers
Martin