D17816: Support for xattrs on kio copy/move

2021-03-02 Thread Gleb Popov
arrowd added a comment.


  In D17816#677446 , @kdudka wrote:
  
  > I was wondering why copying files in Krusader or Dolphin from a 
vfat-formatted memory card stopped working for me after update.  After copying 
the first file, the transfer stopped progressing and the process ended up 
consuming 100% CPU forever.  Later I discovered that the cause of the breakage 
was this patch: If fgetxattr() fails with ENOTSUP, the code loops indefinitely 
calling fgetxattr().
  >
  > The following hotfix made copying of files in Krusader work again for me:
  >  ...
  >
  > However, the code might need bigger changes to cover all the cases.  I do 
not fully understand the (IMO over-complicated) loops around flistxattr() and 
fgetxattr().  Note that the fact that one of them uses `while (true) { ... }` 
whereas the other one uses `do { ... } while (true)` does not improve code 
readability either.
  
  
  I think, the correct fix is something like
  
diff --git a/src/ioslaves/file/file_unix.cpp 
b/src/ioslaves/file/file_unix.cpp
index 74767123..4bc87c57 100644
--- a/src/ioslaves/file/file_unix.cpp
+++ b/src/ioslaves/file/file_unix.cpp
@@ -614,8 +614,8 @@ bool FileProtocol::copyXattrs(const int src_fd, const 
int dest_fd)
 valuelen = 0;
 continue;
 }
-// happens when attr value is an empty string
-if (valuelen == 0) {
+// valuelen=0 happens when attr value is an empty string
+if (valuelen == 0 || valuelen == -1) {
 break;
 }
 } while (true);
  
  Can you check if this works for you too?

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

To: arrowd, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta, cochise
Cc: kdudka, usta, scheirle, tmarshall, arrowd, cfeck, bruns, phidrho, dhaumann, 
funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, spoorun, 
nicolasfella, kde-frameworks-devel, LeGast00n, cblack, michaelh


KACL from KIO isn't really POSIX-compliant

2020-12-02 Thread Gleb Popov
Hello everyone.

I tried compiling kio/src/core/kacl.cpp on FreeBSD, which does support
POSIX ACLs, and failed. This is because KACL's code uses non-standard
Linux-specific acl_* functions. I tried implementing them using standard
ones and it turned out to be impossible, mainly because types like acl_t
are opaque to the user of the library.

For instance, there is no standard acl_get_perm() function and it is
impossible to implement it without getting into acl_permset_t.

Now I'm a bit unsure how to solve this. I can implement non-standard
functions in FreeBSD's libc without touching KACL code, or I can rewrite
the KACL class to be truly POSIX-compliant. The latter seems to be a better
idea on the first look, but it'd require keeping track of all the
permission flags set (again, because there is no acl_get_perm()) inside.
This will turn KACL class from being a tiny acl_* wrapper into a beefy
chunk of code, but at least we won't lie that it is POSIX-compliant.

Any thoughts?

P.S. Please CC me, as I'm not subscribed.


D17816: Support for xattrs on kio copy/move

2020-11-16 Thread Gleb Popov
arrowd added inline comments.

INLINE COMMENTS

> dfaure wrote in ConfigureChecks.cmake:12
> I'm talking about the implementation of fileSystemSupportsACL in 
> kpropertiesdialog.cpp, which uses getxattr.
> But now that I take another look at it, I see that it's already implemented 
> on FreeBSD, without the use of extattr, it uses statfs instead.
> So I think I was wrong, this part is fine.
> 
> I guess the only thing that's a bit weird is that kio_file's attr code 
> (unrelated to ACLs) relies on FindACL linking to the right lib (on Linux). It 
> would be cleaner if the attr stuff was separate. But no big deal I guess.

There are other parts of code that are guarded with `HAVE_POSIX_ACL`s, and 
these aren't enabled ATM for FreeBSD. So, the change is needed and I'm willing 
to implement it.

I plan to move `set(HAVE_POSIX_ACL ...)` part into `FindACL.cmake` and use this 
module everywhere. Does that sound OK?

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

To: arrowd, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta, cochise
Cc: usta, scheirle, tmarshall, arrowd, cfeck, bruns, phidrho, dhaumann, 
funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, spoorun, 
nicolasfella, kde-frameworks-devel, LeGast00n, cblack, michaelh


D17816: Support for xattrs on kio copy/move

2020-10-29 Thread Gleb Popov
arrowd added inline comments.

INLINE COMMENTS

> dfaure wrote in ConfigureChecks.cmake:12
> Ah, I see, OK. No extra lib needed makes it simple.
> 
> The FindACL.cmake stuff is a bit of a mess now, with the need for extended 
> attributes outside the ACL related code.
> Maybe this could be split up into "find extended attribute stuff" and "find 
> ACL stuff", the latter relying on the former.
> But this merge request has been pending for long enough, let's do buildsystem 
> refactoring as part of it.
> 
> Let's leave this part as is for now.
> 
> If you feel like it, I suggest followup commits to 1) enable the ACL stuff on 
> systems with extattr, see the little bit of code in kpropertiesdialog.cpp, 
> and 2) separate the cmake stuff for ACLs from the cmake stuff for extended 
> attributes.

> 1. enable the ACL stuff on systems with extattr, see the little bit of code 
> in kpropertiesdialog.cpp

By that you mean that I should edit the CMake module to define `HAVE_POSIX_ACL` 
when extattr headers are found?

Or should I change checks in kpropertiesdialog.cpp from `HAVE_POSIX_ACL` to 
`HAVE_*ATTR_H`?

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

To: arrowd, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta, cochise
Cc: usta, scheirle, tmarshall, arrowd, cfeck, bruns, phidrho, dhaumann, 
funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, spoorun, 
nicolasfella, kde-frameworks-devel, LeGast00n, cblack, michaelh


D17816: Support for xattrs on kio copy/move

2020-10-29 Thread Gleb Popov
arrowd added inline comments.

INLINE COMMENTS

> dfaure wrote in ConfigureChecks.cmake:12
> This is already done by `cmake/FindACL.cmake`, why not add the other one 
> (extattr.h) to that file as well?
> 
> The CMakeLists.txt in this directory calls `find_package(ACL)` so it'll be 
> used.
> 
> Even though this isn't technically about ACLs, I'm guessing it all links 
> *because* FindACL.cmake links to libattr, right?
> And then this might also mean that the condition in FindACL.cmake needs to be 
> updated, it currently *requires* HAVE_SYS_XATTR_H instead of allowing both 
> variants.
> 
> But then I guess this means kpropertiesdialog.cpp needs to be ported to the 
> sys/extattr.h API.
> 
> Has this been tested on a system with sys/extattr.h? I'm wondering if what 
> will happen is: FindACL didn't find sys/attr.h so it doesn't link to libattr, 
> and then the new code here fails to link. Or am I missing something?

> Has this been tested on a system with sys/extattr.h?

I was working on this revision on such system all the time. FreeBSD contains 
extattr bits in its `libc`, so no extra libraries are required.

So, what should be done here, then? Just move `HAVE_SYS_EXTATTR_H` check to 
`FindACL.cmake` module?

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

To: arrowd, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta, cochise
Cc: usta, scheirle, tmarshall, arrowd, cfeck, bruns, phidrho, dhaumann, 
funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, spoorun, 
nicolasfella, kde-frameworks-devel, LeGast00n, cblack, michaelh


D17816: Support for xattrs on kio copy/move

2020-10-29 Thread Gleb Popov
arrowd updated this revision to Diff 83375.
arrowd marked an inline comment as done.
arrowd added a comment.


  - Put FileProtocol::copyXattrs inside a #if

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17816?vs=83366=83375

BRANCH
  arcpatch-D17816

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

AFFECTED FILES
  autotests/jobtest.cpp
  autotests/jobtest.h
  src/ioslaves/file/ConfigureChecks.cmake
  src/ioslaves/file/config-kioslave-file.h.cmake
  src/ioslaves/file/file.h
  src/ioslaves/file/file_unix.cpp

To: arrowd, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta, cochise
Cc: usta, scheirle, tmarshall, arrowd, cfeck, bruns, phidrho, dhaumann, 
funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, spoorun, 
nicolasfella, kde-frameworks-devel, LeGast00n, cblack, michaelh


D17816: Support for xattrs on kio copy/move

2020-10-17 Thread Gleb Popov
arrowd added a comment.


  Another bump.

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

To: arrowd, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta, cochise
Cc: usta, scheirle, tmarshall, arrowd, cfeck, bruns, phidrho, dhaumann, 
funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, spoorun, 
nicolasfella, kde-frameworks-devel, LeGast00n, cblack, michaelh


D17816: Support for xattrs on kio copy/move

2020-09-21 Thread Gleb Popov
arrowd updated this revision to Diff 83366.
arrowd marked 5 inline comments as done.
arrowd added a comment.


  - Address comments.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17816?vs=83365=83366

BRANCH
  arcpatch-D17816

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

AFFECTED FILES
  autotests/jobtest.cpp
  autotests/jobtest.h
  src/ioslaves/file/ConfigureChecks.cmake
  src/ioslaves/file/config-kioslave-file.h.cmake
  src/ioslaves/file/file.h
  src/ioslaves/file/file_unix.cpp

To: arrowd, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta, cochise
Cc: usta, scheirle, tmarshall, arrowd, cfeck, bruns, phidrho, dhaumann, 
funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, spoorun, 
nicolasfella, kde-frameworks-devel, LeGast00n, cblack, michaelh


D17816: Support for xattrs on kio copy/move

2020-09-16 Thread Gleb Popov
arrowd added inline comments.

INLINE COMMENTS

> usta wrote in file_unix.cpp:1517
> isnt this ignoring acl_from_mode part ? I mean not sure but i think we need 
> to check if it is nullptr or not before assigning it otherwise we will ignore 
> the acl_from_mode part.

You seem to be right. I blamed the code down to svn->git import and this bug 
was present all the time.

To properly fix this I'd start with a test for `KIO::chmod` job that changes 
ACL permissions, but I can't test this on FreeBSD as we don't have 
`acl_from_mode`.

Anyways, it is out of scope of this review.

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

To: arrowd, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta, cochise
Cc: usta, scheirle, tmarshall, arrowd, cfeck, bruns, phidrho, dhaumann, 
funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, spoorun, 
nicolasfella, kde-frameworks-devel, LeGast00n, cblack, michaelh


D17816: Support for xattrs on kio copy/move

2020-09-16 Thread Gleb Popov
arrowd updated this revision to Diff 83365.
arrowd marked an inline comment as done.
arrowd added a comment.


  - More const QString.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17816?vs=83364=83365

BRANCH
  arcpatch-D17816

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

AFFECTED FILES
  autotests/jobtest.cpp
  autotests/jobtest.h
  src/ioslaves/file/ConfigureChecks.cmake
  src/ioslaves/file/config-kioslave-file.h.cmake
  src/ioslaves/file/file.h
  src/ioslaves/file/file_unix.cpp

To: arrowd, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta, cochise
Cc: usta, scheirle, tmarshall, arrowd, cfeck, bruns, phidrho, dhaumann, 
funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, spoorun, 
nicolasfella, kde-frameworks-devel, LeGast00n, cblack, michaelh


D17816: Support for xattrs on kio copy/move

2020-09-16 Thread Gleb Popov
arrowd added inline comments.

INLINE COMMENTS

> bruns wrote in jobtest.cpp:780
> And now you **only** check the source FS, but no longer the destination FS.

To be honest, I was confused by your first comment. What was wrong with the 
first version of this code?

My understanding was that we want to skip xattr stuff if either source or 
destination doesn't support it

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

To: arrowd, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta, cochise
Cc: usta, scheirle, tmarshall, arrowd, cfeck, bruns, phidrho, dhaumann, 
funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, spoorun, 
nicolasfella, kde-frameworks-devel, LeGast00n, cblack, michaelh


D17816: Support for xattrs on kio copy/move

2020-09-10 Thread Gleb Popov
arrowd updated this revision to Diff 83364.
arrowd marked 2 inline comments as done.
arrowd added a comment.


  - Make checkXattrFsSupport accept directory as a parameter.
  - Consitify QStrings.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17816?vs=83361=83364

BRANCH
  arcpatch-D17816

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

AFFECTED FILES
  autotests/jobtest.cpp
  autotests/jobtest.h
  src/ioslaves/file/ConfigureChecks.cmake
  src/ioslaves/file/config-kioslave-file.h.cmake
  src/ioslaves/file/file.h
  src/ioslaves/file/file_unix.cpp

To: arrowd, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta, cochise
Cc: usta, scheirle, tmarshall, arrowd, cfeck, bruns, phidrho, dhaumann, 
funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, spoorun, 
nicolasfella, kde-frameworks-devel, LeGast00n, cblack, michaelh


D17816: Support for xattrs on kio copy/move

2020-09-06 Thread Gleb Popov
arrowd updated this revision to Diff 83361.
arrowd marked an inline comment as done.
arrowd added a comment.


  - Return "true" if the file doesn't have any xattrs.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17816?vs=83334=83361

BRANCH
  arcpatch-D17816

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

AFFECTED FILES
  autotests/jobtest.cpp
  autotests/jobtest.h
  src/ioslaves/file/ConfigureChecks.cmake
  src/ioslaves/file/config-kioslave-file.h.cmake
  src/ioslaves/file/file.h
  src/ioslaves/file/file_unix.cpp

To: arrowd, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta, cochise
Cc: usta, scheirle, tmarshall, arrowd, cfeck, bruns, phidrho, dhaumann, 
funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, spoorun, 
nicolasfella, kde-frameworks-devel, LeGast00n, cblack, michaelh


D17816: Support for xattrs on kio copy/move

2020-08-25 Thread Gleb Popov
arrowd added a comment.


  Bump.

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

To: arrowd, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta, cochise
Cc: usta, scheirle, tmarshall, arrowd, cfeck, bruns, phidrho, dhaumann, 
funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, spoorun, 
nicolasfella, kde-frameworks-devel, LeGast00n, cblack, michaelh


D17816: Support for xattrs on kio copy/move

2020-07-30 Thread Gleb Popov
arrowd updated this revision to Diff 83334.
arrowd added a comment.


  Rebase on master.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17816?vs=83319=83334

BRANCH
  arcpatch-D17816

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

AFFECTED FILES
  autotests/jobtest.cpp
  autotests/jobtest.h
  src/ioslaves/file/ConfigureChecks.cmake
  src/ioslaves/file/config-kioslave-file.h.cmake
  src/ioslaves/file/file.h
  src/ioslaves/file/file_unix.cpp

To: arrowd, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta, cochise
Cc: usta, scheirle, tmarshall, arrowd, cfeck, bruns, phidrho, dhaumann, 
funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, spoorun, 
nicolasfella, kde-frameworks-devel, LeGast00n, cblack, michaelh


D17816: Support for xattrs on kio copy/move

2020-06-30 Thread Gleb Popov
arrowd added inline comments.

INLINE COMMENTS

> bruns wrote in jobtest.cpp:487
> not done ...

There are no arrays of `512` size anymore. Or am I missing something?

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

To: arrowd, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta, cochise
Cc: usta, scheirle, tmarshall, arrowd, cfeck, bruns, phidrho, dhaumann, 
funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, spoorun, 
nicolasfella, kde-frameworks-devel, LeGast00n, cblack, michaelh


D17816: Support for xattrs on kio copy/move

2020-06-30 Thread Gleb Popov
arrowd updated this revision to Diff 83319.
arrowd marked an inline comment as done.
arrowd added a comment.


  - Remove extraneous debugging output.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17816?vs=83318=83319

BRANCH
  arcpatch-D17816

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

AFFECTED FILES
  autotests/jobtest.cpp
  autotests/jobtest.h
  src/ioslaves/file/ConfigureChecks.cmake
  src/ioslaves/file/config-kioslave-file.h.cmake
  src/ioslaves/file/file.h
  src/ioslaves/file/file_unix.cpp

To: arrowd, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta, cochise
Cc: usta, scheirle, tmarshall, arrowd, cfeck, bruns, phidrho, dhaumann, 
funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, spoorun, 
nicolasfella, kde-frameworks-devel, LeGast00n, cblack, michaelh


D17816: Support for xattrs on kio copy/move

2020-06-29 Thread Gleb Popov
arrowd updated this revision to Diff 83318.
arrowd marked 3 inline comments as done.
arrowd added a comment.


  - Handle attrs with empty values.
  - Add test for it.
  - Fix syscalls for FreeBSD case.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17816?vs=83309=83318

BRANCH
  arcpatch-D17816

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

AFFECTED FILES
  autotests/jobtest.cpp
  autotests/jobtest.h
  src/ioslaves/file/ConfigureChecks.cmake
  src/ioslaves/file/config-kioslave-file.h.cmake
  src/ioslaves/file/file.h
  src/ioslaves/file/file_unix.cpp

To: arrowd, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta, cochise
Cc: usta, scheirle, tmarshall, arrowd, cfeck, bruns, phidrho, dhaumann, 
funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, spoorun, 
nicolasfella, kde-frameworks-devel, LeGast00n, cblack, michaelh


D17816: Support for xattrs on kio copy/move

2020-06-23 Thread Gleb Popov
arrowd added inline comments.

INLINE COMMENTS

> bruns wrote in file_unix.cpp:625
> Infinite loop on valuelen == 0

Why? `ERANGE` means we need to come up with new value for `valuelen`, so we set 
it to zero and start over. On the new iteration it gets passed into 
`fgetxattr`/`extattr_get_fd` to find out sufficient value.

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

To: arrowd, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta, cochise
Cc: usta, scheirle, tmarshall, arrowd, cfeck, bruns, phidrho, dhaumann, 
funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, spoorun, 
nicolasfella, kde-frameworks-devel, LeGast00n, cblack, michaelh


D17816: Support for xattrs on kio copy/move

2020-06-23 Thread Gleb Popov
arrowd updated this revision to Diff 83309.
arrowd marked an inline comment as done.
arrowd added a comment.


  - Improve comment.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17816?vs=83302=83309

BRANCH
  arcpatch-D17816

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

AFFECTED FILES
  autotests/jobtest.cpp
  autotests/jobtest.h
  src/ioslaves/file/ConfigureChecks.cmake
  src/ioslaves/file/config-kioslave-file.h.cmake
  src/ioslaves/file/file.h
  src/ioslaves/file/file_unix.cpp

To: arrowd, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta, cochise
Cc: usta, scheirle, tmarshall, arrowd, cfeck, bruns, phidrho, dhaumann, 
funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, spoorun, 
nicolasfella, kde-frameworks-devel, LeGast00n, cblack, michaelh


D17816: Support for xattrs on kio copy/move

2020-06-20 Thread Gleb Popov
arrowd updated this revision to Diff 83302.
arrowd added a comment.


  - Rebase on master.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17816?vs=83207=83302

BRANCH
  arcpatch-D17816

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

AFFECTED FILES
  autotests/jobtest.cpp
  autotests/jobtest.h
  src/ioslaves/file/ConfigureChecks.cmake
  src/ioslaves/file/config-kioslave-file.h.cmake
  src/ioslaves/file/file.h
  src/ioslaves/file/file_unix.cpp

To: arrowd, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta, cochise
Cc: usta, scheirle, tmarshall, arrowd, cfeck, bruns, phidrho, dhaumann, 
funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, spoorun, 
nicolasfella, kde-frameworks-devel, LeGast00n, cblack, michaelh


D17816: Support for xattrs on kio copy/move

2020-06-03 Thread Gleb Popov
arrowd added a comment.


  I'd appreciate if someone test this on Linux and MacOS. I got FreeBSD covered.

REPOSITORY
  R241 KIO

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

To: arrowd, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta, cochise
Cc: usta, scheirle, tmarshall, arrowd, cfeck, bruns, phidrho, dhaumann, 
funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, spoorun, 
nicolasfella, kde-frameworks-devel, LeGast00n, cblack, michaelh


D17816: Support for xattrs on kio copy/move

2020-06-03 Thread Gleb Popov
arrowd updated this revision to Diff 83207.
arrowd marked 8 inline comments as done.
arrowd added a comment.


  - Address comments.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17816?vs=83203=83207

BRANCH
  arcpatch-D17816

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

AFFECTED FILES
  autotests/jobtest.cpp
  autotests/jobtest.h
  src/ioslaves/file/ConfigureChecks.cmake
  src/ioslaves/file/config-kioslave-file.h.cmake
  src/ioslaves/file/file.h
  src/ioslaves/file/file_unix.cpp

To: arrowd, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta, cochise
Cc: usta, scheirle, tmarshall, arrowd, cfeck, bruns, phidrho, dhaumann, 
funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, spoorun, 
nicolasfella, kde-frameworks-devel, LeGast00n, cblack, michaelh


D17816: Support for xattrs on kio copy/move

2020-06-02 Thread Gleb Popov
arrowd added a comment.


  Almost every comment have been addressed. Please, give this another look.

REPOSITORY
  R241 KIO

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

To: arrowd, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta, cochise
Cc: usta, scheirle, tmarshall, arrowd, cfeck, bruns, phidrho, dhaumann, 
funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, spoorun, 
nicolasfella, kde-frameworks-devel, LeGast00n, cblack, michaelh


D17816: Support for xattrs on kio copy/move

2020-06-02 Thread Gleb Popov
arrowd updated this revision to Diff 83203.
arrowd marked 17 inline comments as done.
arrowd added a comment.


  - Address comments.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17816?vs=83192=83203

BRANCH
  arcpatch-D17816

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

AFFECTED FILES
  autotests/jobtest.cpp
  autotests/jobtest.h
  src/ioslaves/file/ConfigureChecks.cmake
  src/ioslaves/file/config-kioslave-file.h.cmake
  src/ioslaves/file/file.h
  src/ioslaves/file/file_unix.cpp

To: arrowd, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta, cochise
Cc: usta, scheirle, tmarshall, arrowd, cfeck, bruns, phidrho, dhaumann, 
funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, spoorun, 
nicolasfella, kde-frameworks-devel, LeGast00n, cblack, michaelh


D17816: Support for xattrs on kio copy/move

2020-06-01 Thread Gleb Popov
arrowd marked an inline comment as done.

REPOSITORY
  R241 KIO

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

To: arrowd, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta, cochise
Cc: usta, scheirle, tmarshall, arrowd, cfeck, bruns, phidrho, dhaumann, 
funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, spoorun, 
nicolasfella, kde-frameworks-devel, LeGast00n, cblack, michaelh


D17816: Support for xattrs on kio copy/move

2020-06-01 Thread Gleb Popov
arrowd updated this revision to Diff 83192.
arrowd marked 2 inline comments as done.
arrowd added a comment.


  - Change the algorithm in FileProtocol::copyXattrs()

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17816?vs=83166=83192

BRANCH
  arcpatch-D17816

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

AFFECTED FILES
  autotests/jobtest.cpp
  autotests/jobtest.h
  src/ioslaves/file/ConfigureChecks.cmake
  src/ioslaves/file/config-kioslave-file.h.cmake
  src/ioslaves/file/file.h
  src/ioslaves/file/file_unix.cpp

To: arrowd, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta, cochise
Cc: usta, scheirle, tmarshall, arrowd, cfeck, bruns, phidrho, dhaumann, 
funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, spoorun, 
nicolasfella, kde-frameworks-devel, LeGast00n, cblack, michaelh


D17816: Support for xattrs on kio copy/move

2020-05-27 Thread Gleb Popov
arrowd marked an inline comment as done.
arrowd added a comment.


  Mark stale command as done.

REPOSITORY
  R241 KIO

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

To: arrowd, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta, cochise
Cc: usta, scheirle, tmarshall, arrowd, cfeck, bruns, phidrho, dhaumann, 
funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, spoorun, 
nicolasfella, kde-frameworks-devel, LeGast00n, cblack, michaelh


D17816: Support for xattrs on kio copy/move

2020-05-27 Thread Gleb Popov
arrowd updated this revision to Diff 83166.
arrowd marked an inline comment as done.
arrowd added a comment.


  - Use std::function to store a generator function for m_setXattrCmd's 
arguments.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17816?vs=83143=83166

BRANCH
  arcpatch-D17816

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

AFFECTED FILES
  autotests/jobtest.cpp
  autotests/jobtest.h
  src/ioslaves/file/ConfigureChecks.cmake
  src/ioslaves/file/config-kioslave-file.h.cmake
  src/ioslaves/file/file.h
  src/ioslaves/file/file_unix.cpp

To: arrowd, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta, cochise
Cc: usta, scheirle, tmarshall, arrowd, cfeck, bruns, phidrho, dhaumann, 
funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, spoorun, 
nicolasfella, kde-frameworks-devel, LeGast00n, cblack, michaelh


D17816: Support for xattrs on kio copy/move

2020-05-24 Thread Gleb Popov
arrowd updated this revision to Diff 83143.
arrowd marked 5 inline comments as done.
arrowd added a comment.


  - Refactor common code from compareXattr() into readXattr() as requested in 
comments.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17816?vs=83068=83143

BRANCH
  arcpatch-D17816

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

AFFECTED FILES
  autotests/jobtest.cpp
  autotests/jobtest.h
  src/ioslaves/file/ConfigureChecks.cmake
  src/ioslaves/file/config-kioslave-file.h.cmake
  src/ioslaves/file/file.h
  src/ioslaves/file/file_unix.cpp

To: arrowd, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta, cochise
Cc: usta, scheirle, tmarshall, arrowd, cfeck, bruns, phidrho, dhaumann, 
funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, spoorun, 
nicolasfella, kde-frameworks-devel, LeGast00n, cblack, michaelh


D17816: Support for xattrs on kio copy/move

2020-05-19 Thread Gleb Popov
arrowd updated this revision to Diff 83068.
arrowd marked 18 inline comments as done.
arrowd added a comment.


  - Use full paths to command line utilities and pass xattr args correctly.
  - Mark some stale comments as done.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17816?vs=83016=83068

BRANCH
  arcpatch-D17816

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

AFFECTED FILES
  autotests/jobtest.cpp
  autotests/jobtest.h
  src/ioslaves/file/ConfigureChecks.cmake
  src/ioslaves/file/config-kioslave-file.h.cmake
  src/ioslaves/file/file.h
  src/ioslaves/file/file_unix.cpp

To: arrowd, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta, cochise
Cc: usta, scheirle, tmarshall, arrowd, cfeck, bruns, phidrho, dhaumann, 
funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, spoorun, 
nicolasfella, kde-frameworks-devel, LeGast00n, cblack, michaelh


D17816: Support for xattrs on kio copy/move

2020-05-17 Thread Gleb Popov
arrowd updated this revision to Diff 83016.
arrowd added a comment.


  - Fix tests on FreeBSD.
  - Fix bug: Move `keylist.squeeze()` call before acquiring an interator.
  
  At this stage, all tests in `jobtest` pass on FreeBSD.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17816?vs=82960=83016

BRANCH
  arcpatch-D17816

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

AFFECTED FILES
  autotests/jobtest.cpp
  autotests/jobtest.h
  src/ioslaves/file/ConfigureChecks.cmake
  src/ioslaves/file/config-kioslave-file.h.cmake
  src/ioslaves/file/file.h
  src/ioslaves/file/file_unix.cpp

To: arrowd, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta, cochise
Cc: usta, scheirle, tmarshall, arrowd, cfeck, bruns, phidrho, dhaumann, 
funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, spoorun, 
nicolasfella, kde-frameworks-devel, LeGast00n, cblack, michaelh


D17816: Support for xattrs on kio copy/move

2020-05-15 Thread Gleb Popov
arrowd updated this revision to Diff 82960.
arrowd added a comment.


  I decided to help with this a bit.
  
  - Fix detection of  header. It requires  to be 
included too.
  - Implement test helpers for BSD (extattr).
  
  At the moment, some tests fail for me, because xattrs don't seem to be 
copied. I'll look into that later.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17816?vs=80336=82960

BRANCH
  arcpatch-D17816

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

AFFECTED FILES
  autotests/jobtest.cpp
  autotests/jobtest.h
  src/ioslaves/file/ConfigureChecks.cmake
  src/ioslaves/file/config-kioslave-file.h.cmake
  src/ioslaves/file/file.h
  src/ioslaves/file/file_unix.cpp

To: arrowd, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta, cochise
Cc: usta, scheirle, anthonyfieroni, tmarshall, arrowd, cfeck, bruns, phidrho, 
dhaumann, funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, 
spoorun, nicolasfella, kde-frameworks-devel, LeGast00n, cblack, michaelh


D17816: Support for xattrs on kio copy/move

2020-05-15 Thread Gleb Popov
arrowd commandeered this revision.
arrowd added a reviewer: cochise.

REPOSITORY
  R241 KIO

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

To: arrowd, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta, cochise
Cc: usta, scheirle, anthonyfieroni, tmarshall, arrowd, cfeck, bruns, phidrho, 
dhaumann, funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, 
spoorun, nicolasfella, kde-frameworks-devel, LeGast00n, cblack, michaelh


Re: Does FreeBSD have HAL?

2020-01-05 Thread Gleb Popov
On Sun, Jan 5, 2020 at 3:37 PM Tijl Coosemans  wrote:

> On Sun, 5 Jan 2020 10:06:08 +0100 "Tobias C. Berner"
>  wrote:
> > Moin moin
> >
> > We will replace (ancient) hal with Gleb's implementation of usdisks2
> > "bsdisks2" in the near future by default.
> >
> > Hald is likely not enabled on the CI hosts -- I could enable it :) --
> > or we could push the switch and make bsdisks2 the default in master,
> > which I would prefer.
> > What is your thought on this Gleb?
>
> Does it support cdrom/dvd?
>

Depends on what's meant by support. It should recognize it, probably
doesn't mount it.


Re: Does FreeBSD have HAL?

2020-01-05 Thread Gleb Popov
On Sun, Jan 5, 2020 at 1:06 PM Tobias C. Berner  wrote:

> Moin moin
>
> We will replace (ancient) hal with Gleb's implementation of usdisks2
> "bsdisks2" in the near future by default.
>
> Hald is likely not enabled on the CI hosts -- I could enable it :) --
> or we could push the switch and make bsdisks2 the default in master,
> which I would prefer.
> What is your thought on this Gleb?
>

I'm using bsdisks for a long time and have no problems with it. However,
tcberner was having busy-loop hangs, and I wasn't able to reproduce them
myself.

I'd say let's make a switch given that CI is green with bsdisks.


>
> mfg Tobias
>
> On Sat, 4 Jan 2020 at 18:16, David Faure  wrote:
> >
> > solid/src/CMakeLists.txt offers the option to use "UDisks2/bsdisks
> backend instead of HAL to manage disk devices" on FreeBSD, but OFF by
> default.
> >
> > So the default is the HAL backend, which however completely fails on CI:
> >
> https://build.kde.org/job/Frameworks/view/Platform%20-%20FreeBSDQt5.13/job/solid/job/kf5-qt5%20FreeBSDQt5.13/52/testReport/projectroot/autotests/halbasictest/
> > basically says that org.freedesktop.Hal is not running (on the system
> bus)
> >
> > FreeBSD users: does `qdbus --system org.freedesktop.HalManager` work for
> you?
> > If it does, any idea what should be done on the CI to make that work
> there?
> >
> > I'm really hoping for fully-green unittests one day, but FreeBSD isn't
> really helping with that :-)
>

I was working on improving tests on FreeBSD long ago, but got distracted by
$WORK. It is sad to hear that things aren't improving. I'll try my best
again in near future.

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


D22525: kioclient: Don't convert `:x:y` to `?line=x=y` for URLs starting with remote schemes.

2019-07-20 Thread Gleb Popov
arrowd added a comment.


  I'm not familiar with KDE release engineering, so I'm asking to do merging 
someone else.

REPOSITORY
  R126 KDE CLI Utilities

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

To: arrowd, #frameworks, dfaure
Cc: wbauer, kwrite-devel, dfaure, cfeck, plasma-devel, #frameworks, LeGast00n, 
jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart


D22525: kioclient: Don't convert `:x:y` to `?line=x=y` for URLs starting with remote schemes.

2019-07-20 Thread Gleb Popov
This revision was automatically updated to reflect the committed changes.
Closed by commit R126:6d86fd453417: kioclient: Dont convert `:x:y` to 
`?line=xcolumn=y` for URLs starting with… (authored by arrowd).

REPOSITORY
  R126 KDE CLI Utilities

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22525?vs=61957=62126

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

AFFECTED FILES
  kioclient/urlinfo.h

To: arrowd, #frameworks, dfaure
Cc: kwrite-devel, dfaure, cfeck, plasma-devel, #frameworks, LeGast00n, 
jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart


D22525: kioclient: Don't convert `:x:y` to `?line=x=y` for URLs starting with remote schemes.

2019-07-20 Thread Gleb Popov
arrowd added a comment.


  In D22525#498393 , @dfaure wrote:
  
  > I suppose the kate developers like the fact that this currently works over 
FTP, SFTP, FISH, SMB, etc.
  >  So maybe only HTTP[S]/WEBDAV should be blacklisted (because there queries 
have a different meaning, one that we can't know client-side).
  
  
  I'm confused. I just tried SFTP and SMB without the patch and it also doesn't 
work. As I said, it is invalid to rewrite URLs for remote schemas.

REPOSITORY
  R126 KDE CLI Utilities

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

To: arrowd, #frameworks, dfaure
Cc: dfaure, cfeck, plasma-devel, #frameworks, LeGast00n, jraleigh, 
fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart


D22525: kioclient: Don't convert `:x:y` to `?line=x=y` for URLs starting with remote schemes.

2019-07-19 Thread Gleb Popov
arrowd added a reviewer: Frameworks.

REPOSITORY
  R126 KDE CLI Utilities

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

To: arrowd, #frameworks
Cc: cfeck, plasma-devel, #frameworks, LeGast00n, jraleigh, fbampaloukas, GB_2, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart


D22525: kioclient: Don't convert `:x:y` to `?line=x=y` for URLs starting with remote schemes.

2019-07-19 Thread Gleb Popov
arrowd added a comment.


  In D22525#497556 , @cfeck wrote:
  
  > Oh, if the latter syntax also works, then you are right.
  
  
  Not sure what you mean by "also works". Current code checks if the URL ends 
with `:x:y` and turns it into `?line=x=y`. This is used to open local 
text files at the given cursor position. However, for URL's like 
`http://localhost:3000` it is invalid to perform such transformation. My patch 
checks URL scheme and applies the transformation only if it is `file://`.

REPOSITORY
  R126 KDE CLI Utilities

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

To: arrowd
Cc: cfeck, plasma-devel, #frameworks, LeGast00n, jraleigh, fbampaloukas, GB_2, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart


D22525: kioclient: Don't convert `:x:y` to `?line=x=y` for URLs starting with remote schemes.

2019-07-18 Thread Gleb Popov
arrowd added a comment.


  In D22525#497332 , @cfeck wrote:
  
  > Could we only apply the `:xx` check on the filename part, not on the server 
part? Someone might expect that `http://path.to/file.txt:99` also works for 
remote files.
  
  
  I don't think that rewriting `http://path.to/file.txt:99` to 
`http://path.to/file.txt?line=99` is a good idea. I'd better not touch 
non`file://` URLs at all.

REPOSITORY
  R126 KDE CLI Utilities

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

To: arrowd
Cc: cfeck, plasma-devel, #frameworks, LeGast00n, jraleigh, fbampaloukas, GB_2, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart


D22525: kioclient: Don't convert `:x:y` to `?line=x=y` for URLs starting with remote schemes.

2019-07-18 Thread Gleb Popov
arrowd created this revision.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
arrowd requested review of this revision.

REVISION SUMMARY
  BUG: 408632

TEST PLAN
  `kioclient5 exec http://localhost:9000` now works correctly.

REPOSITORY
  R126 KDE CLI Utilities

BRANCH
  master

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

AFFECTED FILES
  kioclient/urlinfo.h

To: arrowd
Cc: plasma-devel, #frameworks, LeGast00n, jraleigh, fbampaloukas, GB_2, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart


D20967: [UserMetaData] Add method to query which attributes are set

2019-06-10 Thread Gleb Popov
arrowd added a comment.


  Now the test passes!

INLINE COMMENTS

> xattr_p.h:194
> +{
> +int pos;
> +QList entries;

variable 'pos' is uninitialized

REPOSITORY
  R286 KFileMetaData

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

To: bruns, #baloo, #frameworks, astippich, ngraham
Cc: arrowd, #freebsd, #windows, kde-frameworks-devel, LeGast00n, domson, 
ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams


D21720: Enable usermetadatawritertest for all UNIXes, not only Linux.

2019-06-10 Thread Gleb Popov
This revision was automatically updated to reflect the committed changes.
Closed by commit R286:346918916474: Enable usermetadatawritertest for all 
UNIXes, not only Linux. (authored by arrowd).

REPOSITORY
  R286 KFileMetaData

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21720?vs=59503=59504

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

AFFECTED FILES
  autotests/CMakeLists.txt

To: arrowd, #frameworks, bruns
Cc: kde-frameworks-devel, #baloo, LeGast00n, domson, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, bruns, abrahams


D21720: Enable usermetadatawritertest for all UNIXes, not only Linux.

2019-06-10 Thread Gleb Popov
arrowd created this revision.
arrowd added reviewers: Frameworks, bruns.
Herald added projects: Frameworks, Baloo.
Herald added subscribers: Baloo, kde-frameworks-devel.
arrowd requested review of this revision.

TEST PLAN
  The test passes on FreeBSD.

REPOSITORY
  R286 KFileMetaData

BRANCH
  master

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

AFFECTED FILES
  autotests/CMakeLists.txt

To: arrowd, #frameworks, bruns
Cc: kde-frameworks-devel, #baloo, LeGast00n, domson, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, bruns, abrahams


D20967: [UserMetaData] Add method to query which attributes are set

2019-06-10 Thread Gleb Popov
arrowd added a comment.


  In D20967#477314 , @bruns wrote:
  
  > Apparently, the usermetadatawritertest was not executed on *BSD before, can 
you check if the test succeeds on the old code?
  
  
  Yep, it works on master.
  
  The output with `qDebug()`s:
  
PASS   : UserMetaDataWriterTest::initTestCase()
QDEBUG : UserMetaDataWriterTest::test() extattr_list size: 14
QDEBUG : UserMetaDataWriterTest::test() extattr_list size: 14
QDEBUG : UserMetaDataWriterTest::test() extattr_list r: 14 "\ruser.xdg.tags"
FAIL!  : UserMetaDataWriterTest::test() 
'md.queryAttributes(UserMetaData::Attribute::All) & 
UserMetaData::Attribute::Tags' returned FALSE. ()
   Loc: 
[/home/arr/projects/kfilemetadata/autotests/usermetadatawritertest.cpp(55)]

REPOSITORY
  R286 KFileMetaData

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

To: bruns, #baloo, #frameworks, astippich, ngraham
Cc: arrowd, #freebsd, #windows, kde-frameworks-devel, LeGast00n, domson, 
ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams


D20967: [UserMetaData] Add method to query which attributes are set

2019-06-10 Thread Gleb Popov
arrowd added a comment.


  It compiles now, but the test fails:
  
* Start testing of UserMetaDataWriterTest *
Config: Using QtTest library 5.12.2, Qt 5.12.2 (x86_64-little_endian-lp64 
shared (dynamic) release build; by Clang 8.0.0 (tags/RELEASE_800/final 356365))
PASS   : UserMetaDataWriterTest::initTestCase()
FAIL!  : UserMetaDataWriterTest::test() 
'md.queryAttributes(UserMetaData::Attribute::All) & 
UserMetaData::Attribute::Tags' returned FALSE. ()
   Loc: 
[/home/arr/projects/kfilemetadata/autotests/usermetadatawritertest.cpp(55)]
PASS   : UserMetaDataWriterTest::cleanupTestCase()
Totals: 2 passed, 1 failed, 0 skipped, 0 blacklisted, 1ms
* Finished testing of UserMetaDataWriterTest *
  
  I also had to do this to enable the test:
  
diff --git a/autotests/CMakeLists.txt b/autotests/CMakeLists.txt
index 49824ba..994c7bc 100644
--- a/autotests/CMakeLists.txt
+++ b/autotests/CMakeLists.txt
@@ -220,7 +220,7 @@ endif()
 #
 # UserMetaData
 #
-if(CMAKE_SYSTEM_NAME MATCHES "Linux")
+if(UNIX)
 kde_enable_exceptions()
 ecm_add_test(usermetadatawritertest.cpp ../src/usermetadata.cpp
 TEST_NAME "usermetadatawritertest"

INLINE COMMENTS

> bruns wrote in xattr_p.h:231
> Why ditto?

Right, no need to change there.

REPOSITORY
  R286 KFileMetaData

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

To: bruns, #baloo, #frameworks, astippich, ngraham
Cc: arrowd, #freebsd, #windows, kde-frameworks-devel, LeGast00n, domson, 
ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams


D20967: [UserMetaData] Add method to query which attributes are set

2019-06-10 Thread Gleb Popov
arrowd added inline comments.

INLINE COMMENTS

> xattr_p.h:204
> +#elif defined(Q_OS_FREEBSD) || defined(Q_OS_NETBSD)
> +const ssize_t size = extattr_list_file(encodedPath, 
> EXTATTR_NAMESPACE_USER, attributeName, nullptr, 0);
> +#endif

error: use of undeclared identifier 'attributeName'

> xattr_p.h:231
> +#elif defined(Q_OS_FREEBSD) || defined(Q_OS_NETBSD)
> +const ssize_t size = extattr_list_file(encodedPath, 
> EXTATTR_NAMESPACE_USER, data.data(), data.size());
> +#endif

Ditto.

Also, `size` should be `r`, probably, see below.

> xattr_p.h:234
> +
> +if (r == 0) {
> +return UserMetaData::Attribute::None;

error: use of undeclared identifier 'r'

REPOSITORY
  R286 KFileMetaData

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

To: bruns, #baloo, #frameworks, astippich, ngraham
Cc: arrowd, #freebsd, #windows, kde-frameworks-devel, LeGast00n, domson, 
ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams


D21380: show static word wrap marker always if requested

2019-05-24 Thread Gleb Popov
arrowd accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R39 KTextEditor

BRANCH
  master

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

To: cullmann, #ktexteditor, #frameworks, arrowd
Cc: kwrite-devel, kde-frameworks-devel, domson, michaelh, ngraham, bruns, 
demsking, cullmann, sars, dhaumann


D18296: Add support for passing cursor information via URL parameters when running kioclient exec.

2019-04-11 Thread Gleb Popov
arrowd marked an inline comment as done.

REPOSITORY
  R126 KDE CLI Utilities

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

To: arrowd, #plasma, #ktexteditor, broulik, #frameworks, pino, cfeck, dfaure, 
elvisangelaccio
Cc: apol, cullmann, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, mart


D18296: Add support for passing cursor information via URL parameters when running kioclient exec.

2019-04-11 Thread Gleb Popov
This revision was automatically updated to reflect the committed changes.
Closed by commit R126:8072a6acf221: Add support for passing cursor information 
via URL parameters when running… (authored by arrowd).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D18296?vs=55879=55973#toc

REPOSITORY
  R126 KDE CLI Utilities

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18296?vs=55879=55973

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

AFFECTED FILES
  kioclient/kioclient.cpp
  kioclient/kioclient.h
  kioclient/urlinfo.h

To: arrowd, #plasma, #ktexteditor, broulik, #frameworks, pino, cfeck, dfaure, 
elvisangelaccio
Cc: apol, cullmann, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, mart


D18296: Add support for passing cursor information via URL parameters when running kioclient exec.

2019-04-10 Thread Gleb Popov
arrowd added inline comments.

INLINE COMMENTS

> dfaure wrote in urlinfo.h:77
> But then you can't do `kde-open5 www.google.fr` anymore, right?
> 
> I see what you mean with typo handling, but there is no perfect solution. 
> Either we treat typos as URLs (but it means we also treat actual URLs as 
> such), or we treat everything non-existing as a local file (breaking any use 
> of short URLs). The latter is OK for kwrite, but not for the more general 
> purpose kioclient / kde-open5.

> But then you can't do kde-open5 www.google.fr anymore, right?

Ah, right. Ok, then.

REPOSITORY
  R126 KDE CLI Utilities

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

To: arrowd, #plasma, #ktexteditor, broulik, #frameworks, pino, cfeck, dfaure, 
elvisangelaccio
Cc: apol, cullmann, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, mart


D18296: Add support for passing cursor information via URL parameters when running kioclient exec.

2019-04-10 Thread Gleb Popov
arrowd updated this revision to Diff 55879.
arrowd marked 5 inline comments as done.
arrowd added a comment.


  Address comments.

REPOSITORY
  R126 KDE CLI Utilities

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18296?vs=54774=55879

BRANCH
  cursor

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

AFFECTED FILES
  kioclient/kioclient.cpp
  kioclient/kioclient.h
  kioclient/urlinfo.h

To: arrowd, #plasma, #ktexteditor, broulik, #frameworks, pino, cfeck, dfaure, 
elvisangelaccio
Cc: apol, cullmann, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, mart


D18296: Add support for passing cursor information via URL parameters when running kioclient exec.

2019-04-08 Thread Gleb Popov
arrowd added a comment.


  In D18296#445829 , @dfaure wrote:
  
  > Is the makeURL function still used, or should it be removed now?
  
  
  Yep, it is still used in a bunch of other places.

INLINE COMMENTS

> dfaure wrote in urlinfo.h:39
> const QString &
> 
> And what if it's a URL? At this point this string is pathOrUrl.

> const QString &

There is `path.chop(match.capturedLength());`, which requires non-const 
`QString`.

> And what if it's a URL? At this point this string is pathOrUrl.

Well, `if (QFile::exists(path))` will return false in this case, and `url` 
would get populated by `url = QUrl::fromUserInput()`. What's wrong with that?

> dfaure wrote in urlinfo.h:77
> I'm not sure about AssumeLocalFile, in the context of kde-open.
> This is about opening existing files, not creating new files.
> So it should be removed.

It is not about creating missing files, but reaction to user typos. If I try to 
open `fiel.txt` instead of a `file.txt`, I want to get a "no such file or 
directory error message" instead of popping browser trying to open 
"http://fiel.txt;.

REPOSITORY
  R126 KDE CLI Utilities

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

To: arrowd, #plasma, #ktexteditor, broulik, #frameworks, pino, cfeck, dfaure, 
elvisangelaccio
Cc: apol, cullmann, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, mart


D18296: Add support for passing cursor information via URL parameters when running kioclient exec.

2019-04-06 Thread Gleb Popov
arrowd added inline comments.

INLINE COMMENTS

> elvisangelaccio wrote in urlinfo.h:45-52
> Why doesn't it parse the URL if the file already exists?

If the file is named `foo:123`, it refuses to search for cursor infomation and 
just open it.

REPOSITORY
  R126 KDE CLI Utilities

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

To: arrowd, #plasma, #ktexteditor, broulik, #frameworks, pino, cfeck, dfaure, 
elvisangelaccio
Cc: apol, cullmann, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, mart


D18296: Add support for passing cursor information via URL parameters when running kioclient exec.

2019-04-06 Thread Gleb Popov
arrowd added a comment.


  In D18296#444642 , 
@elvisangelaccio wrote:
  
  > Is it really necessary to copy `urlinfo.h` here? Wouldn't be enough to just 
create a static function and put the URL parsing logic there?
  
  
  I just copied this from Kate project. KDevelop also done this.

REPOSITORY
  R126 KDE CLI Utilities

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

To: arrowd, #plasma, #ktexteditor, broulik, #frameworks, pino, cfeck, dfaure, 
elvisangelaccio
Cc: apol, cullmann, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, mart


D17938: Add support for passing cursor information to KRun.

2019-01-15 Thread Gleb Popov
arrowd abandoned this revision.
arrowd added a comment.


  Right.

REPOSITORY
  R241 KIO

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

To: arrowd, #frameworks
Cc: aacid, elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, bruns


D17938: Add support for passing cursor information to KRun.

2019-01-05 Thread Gleb Popov
arrowd added a comment.


  In D17938#386985 , 
@elvisangelaccio wrote:
  
  > I don't know, "cursorRow" and "cursorColumn" are pretty editor-specific. 
I'm not sure they make sense in the KRun API.
  
  
  I'd say they are both filetype-specific and service-specific. But I haven't 
found any other approach.

REPOSITORY
  R241 KIO

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

To: arrowd, #frameworks
Cc: elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, bruns


D17938: Add support for passing cursor information to KRun.

2019-01-03 Thread Gleb Popov
arrowd created this revision.
arrowd added a reviewer: Frameworks.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
arrowd requested review of this revision.

REVISION SUMMARY
  This is the KIO part of https://bugs.kde.org/show_bug.cgi?id=398998

TEST PLAN
  With another change to `kioclient5` (not submitted yet) running `kioclient5 
exec textfile:12:34` works fine.
  Corner cases also tested.

REPOSITORY
  R241 KIO

BRANCH
  master

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

AFFECTED FILES
  src/widgets/krun.cpp
  src/widgets/krun.h
  src/widgets/krun_p.h

To: arrowd, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns


D6430: Add syntax definition for julius language.

2017-06-29 Thread Gleb Popov
arrowdodger abandoned this revision.

REPOSITORY
  R216 Syntax Highlighting

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

To: arrowdodger
Cc: #frameworks


D6430: Add syntax definition for julius language.

2017-06-29 Thread Gleb Popov
arrowdodger created this revision.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  master

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

AFFECTED FILES
  data/syntax/julius.xml

To: arrowdodger
Cc: #frameworks


D6087: Haskell: Add all language pragmas as keywords.

2017-06-29 Thread Gleb Popov
This revision was automatically updated to reflect the committed changes.
Closed by commit R216:d2a38d790914: Haskell: Add all language pragmas as 
keywords. (authored by arrowdodger).

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D6087?vs=15982=15984

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

AFFECTED FILES
  autotests/input/highlight.hs
  autotests/reference/highlight.hs.ref
  data/syntax/haskell.xml

To: arrowdodger, #framework_syntax_hightlighting, dhaumann, cullmann
Cc: cullmann, dhaumann, #frameworks


D6087: Haskell: Add all language pragmas as keywords.

2017-06-29 Thread Gleb Popov
arrowdodger updated this revision to Diff 15982.
arrowdodger added a comment.


  Updated reference file for autotest as well.

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D6087?vs=15981=15982

BRANCH
  master

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

AFFECTED FILES
  autotests/input/highlight.hs
  autotests/reference/highlight.hs.ref
  data/syntax/haskell.xml

To: arrowdodger, #framework_syntax_hightlighting, dhaumann
Cc: cullmann, dhaumann, #frameworks


D6087: Haskell: Add all language pragmas as keywords.

2017-06-29 Thread Gleb Popov
arrowdodger updated this revision to Diff 15981.
arrowdodger added a comment.


  Added autotests. Not sure if .ref file should be updated, as the tests pass 
as they are now.

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D6087?vs=15772=15981

BRANCH
  master

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

AFFECTED FILES
  autotests/input/highlight.hs
  data/syntax/haskell.xml

To: arrowdodger, #framework_syntax_hightlighting, dhaumann
Cc: cullmann, dhaumann, #frameworks


D6087: Haskell: Add all language pragmas as keywords.

2017-06-29 Thread Gleb Popov
arrowdodger added a comment.


  So, can I push this?

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  master

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

To: arrowdodger, #framework_syntax_hightlighting, dhaumann
Cc: dhaumann, #frameworks


D6087: Haskell: Add all language pragmas as keywords.

2017-06-23 Thread Gleb Popov
arrowdodger added a comment.


  In https://phabricator.kde.org/D6087#118963, @dhaumann wrote:
  
  > Demo for the highlighting rule - I don't think I can say anything to the 
auto completion issue. I am pretty sure it is unrelated to your xml file.
  
  
  Sure. Pragmas have following form:
  
{-# blabla #-}
  
  Specifically, pragmas that enable language extensions looks like
  
{-# LANGUAGE OverlappingInstances, BangPatterns #-}
  
  As I've found today, pragmas can span onto multiple lines:
  
{-# LANGUAGE OverlappingInstances, 
 BangPatterns
#-}

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  master

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

To: arrowdodger, #framework_syntax_hightlighting, dhaumann
Cc: dhaumann, #frameworks


D6087: Haskell: Add all language pragmas as keywords.

2017-06-23 Thread Gleb Popov
arrowdodger added a comment.


  In https://phabricator.kde.org/D6087#117019, @dhaumann wrote:
  
  > Also: could you provide us with a small demo code so that we can extend our 
unit test?
  
  
  The demo for added highlighting rule, or the problem with autocompletion?

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  master

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

To: arrowdodger, #framework_syntax_hightlighting, dhaumann
Cc: dhaumann, #frameworks


D6087: Haskell: Add all language pragmas as keywords.

2017-06-23 Thread Gleb Popov
arrowdodger updated this revision to Diff 15772.
arrowdodger added a comment.


  - Use WordDetect instead of StringDetect for `omport` keyword.
  - Pop `Pragma` context on `#-}` string, not newline.

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D6087?vs=15129=15772

BRANCH
  master

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

AFFECTED FILES
  data/syntax/haskell.xml

To: arrowdodger, #framework_syntax_hightlighting, dhaumann
Cc: dhaumann, #frameworks


D6102: KUrlRequester: Set NOTIFY signal to textChanged() for text property.

2017-06-12 Thread Gleb Popov
This revision was automatically updated to reflect the committed changes.
Closed by commit R241:f7dfb713a852: KUrlRequester: Set NOTIFY signal to 
textChanged() for text property. (authored by arrowdodger).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D6102?vs=15177=15371

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

AFFECTED FILES
  src/widgets/kurlrequester.h

To: arrowdodger, #frameworks, dfaure
Cc: apol, elvisangelaccio, kfunk


D6087: Haskell: Add all language pragmas as keywords.

2017-06-07 Thread Gleb Popov
arrowdodger added a comment.


  I just found that the same problem exists for `import_keywords` too - when I 
type `import qual`, the completion doesn't show up, and deleteting `qual` and 
typing it again causes it to show.
  
  So, it seems to be a coding bug.

REPOSITORY
  R216 Syntax Highlighting

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

To: arrowdodger, #framework_syntax_hightlighting
Cc: #frameworks


D6101: Work-around MSVC2017 compiler bug

2017-06-05 Thread Gleb Popov
arrowdodger accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R279 ThreadWeaver

BRANCH
  master

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

To: kfunk, arrowdodger, bcooksley
Cc: #frameworks


D6102: KUrlRequester: Set NOTIFY signal to textChanged() for text property.

2017-06-05 Thread Gleb Popov
arrowdodger created this revision.
Restricted Application added a project: Frameworks.

REVISION SUMMARY
  Having `NOTIFY textChanged` on `text` property may come useful when using 
KUrlRequester with KConfig. KUrlRequester field may hold paths for executables 
and sometimes it is more convenient to specify only executable name, so that it 
would be looked in PATH. But KConfig uses `setUrl()` method for saving 
KUrlRequester values, which turns `foo` into 
`/absolute/path/to/current/working/dir/foo`, which is obviouslt wrong.

REPOSITORY
  R241 KIO

BRANCH
  master

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

AFFECTED FILES
  src/widgets/kurlrequester.h

To: arrowdodger, #frameworks


D6087: Haskell: Add all language pragmas as keywords.

2017-06-04 Thread Gleb Popov
arrowdodger edited the summary of this revision.
arrowdodger edited the test plan for this revision.
arrowdodger added a reviewer: Framework: Syntax Hightlighting.

REPOSITORY
  R216 Syntax Highlighting

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

To: arrowdodger, #framework_syntax_hightlighting
Cc: #frameworks


D6087: Haskell: Add all language pragmas as keywords.

2017-06-04 Thread Gleb Popov
arrowdodger created this revision.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  master

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

AFFECTED FILES
  data/syntax/haskell.xml

To: arrowdodger
Cc: #frameworks


D5890: Pass `--key-positions=*` to gperf instead of `-k *` because on Windows this symbol seems to have a special meaning.

2017-05-18 Thread Gleb Popov
This revision was automatically updated to reflect the committed changes.
Closed by commit R313:07a8a4e6985c: Pass `--key-positions=*` to gperf instead 
of `-k *` because on Windows this… (authored by arrowdodger).

REPOSITORY
  R313 KHtml

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5890?vs=14606=14663

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

AFFECTED FILES
  src/CMakeLists.txt

To: arrowdodger, #frameworks, bcooksley
Cc: bcooksley


D5890: Pass `--key-positions=*` to gperf instead of `-k *` because on Windows this symbol seems to have a special meaning.

2017-05-18 Thread Gleb Popov
arrowdodger added a comment.


  In https://phabricator.kde.org/D5890#110586, @bcooksley wrote:
  
  > This looks good to me. Would this also resolve 
https://build-sandbox.kde.org/view/Frameworks/job/Frameworks%20kcodecs%20kf5-qt5%20WindowsQt5.7/6/console
 ?
  
  
  
  
  > Invalid position value or range, use 1,2,3-255,'$' or '*'.
  
  Yep, looks like it.

REPOSITORY
  R313 KHtml

BRANCH
  master

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

To: arrowdodger, #frameworks, bcooksley
Cc: bcooksley


D5890: Pass `--key-positions=*` to gperf instead of `-k *` because on Windows this symbol seems to have a special meaning.

2017-05-16 Thread Gleb Popov
arrowdodger created this revision.
Restricted Application added a project: Frameworks.

REVISION SUMMARY
  Pass `--key-positions=*` to gperf instead of `-k *` because on Windows this 
symbol seems to have a special meaning.

TEST PLAN
  Successfully compiled with MSVC.

REPOSITORY
  R313 KHtml

BRANCH
  master

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

AFFECTED FILES
  src/CMakeLists.txt

To: arrowdodger, #frameworks


D5138: Fill UDS_CREATION_TIME with the value of st_birthtime on FreeBSD

2017-03-23 Thread Gleb Popov
arrowdodger accepted this revision.

REPOSITORY
  R241 KIO

BRANCH
  master

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

To: tcberner, adridg, arrowdodger, rakuco, dfaure
Cc: #frameworks


Re: breeze-icons and the symlink business

2017-02-02 Thread Gleb Popov
On Thu, Feb 2, 2017 at 2:49 AM, Harald Sitter  wrote:

> hola
>
> breeze-icons uses lots of symlinks. Unfortunately, ever so often our
> icon designers make a mistake and create a bad symlink. To mitigate
> this I added a bunch of tests making sure everything is nice and
> dandy.
>
> In the mean parts of the build were changed to not tolerate broken
> symlinks. While I don't really have a problem with that in of itself,
> the code largely simply ignores the possibility of broken symlinks and
> fails with the most shitty error messages you could think of. Given
> our artists are not software engineers they are having a hard time
> figuring out what is going on. And TBH, I too had to stat files to get
> to the bottom of the errors. This is a fairly shit situation as on the
> one hand we want lovely icons and on the other hand the people working
> on the icons can't understand what needs fixing without having to find
> a developer they aren't too afraid of to talk to.
>
> This really needs fixing.
>
> Notably offenders I had a fight with today:
>
>
> # breeze-validate-svg (introduced by Jos)
>
> This is a bash script running xmllint.
>
> ## Problem 1: Sources
>
> The custom target sets `SOURCES ${SVGS}` this has no notable advantage
> other than making the svgs show up in an IDE, it does however mean
> that cmake will try to inspect them (as a build tool does when they
> get told something is a source) and then falls flat on the face when
> it encounters a broken symlink as it now can't determine the source
> type resulting in this lovely error
>
> ```
> 21:39:07 CMake Error at CMakeLists.txt:70 (add_custom_target):
> 21:39:07   Cannot find source file:
> 21:39:07
> 21:39:07 /home/jenkins/sources/breeze-icons/kf5-qt5/icons-dark/
> categories/32/applications-other.svg
> ```
>
> ## Problem 2: The code
>
> The bash code in of itself runs find with -exec on xmllint. Problem
> being that if the symlink is broken xmllint will (rightfully) complain
>
> ```
> warning: failed to load external entity
> "./icons-dark/categories/32/applications-other.svg"
> ```
>
> While that is much better than the earlier problem it's still plenty
> unobvious what the underlying cause for this is. Supposedly the script
> should skip bogus symlinks.
>
> ## Problem 3: Oh but really
>
> This isn't really related to the issue of bad symlink handling:
>
> - apparently this didn't get a review. why ever would this not get a
> review?
> - this should be a test and only run when testing is enabled
> - when xmllint is not present it should report this in some form or
> fashion during the test run so one knows linting is not done
> - it should record its complaints via ctest so jenkins can display them
> properly
> - I fail to appreciate the reason this needs to depend on bash (versus
> sh, or well, neither)
>
> ## Fix Suggestion
>
> - don't needlesly set SOURCE
> - don't pass bad paths to xmllint
> - deal with stuff in problem 3
>
>
> # RCC generation (introduced by Gleb, enabled by Jaroslaw)
>
> This is a bunch of cmake rigging and helper binaries to assemble the
> icons into an rcc.
>
> ## Problem
>
> `cmake -E copy_directory` is used to copy the src tree of the themes
> into the build dir from which the resource file gets assembled. I am
> guessing copy_directory does not preserver, but resolve symlinks
> because it greets us with the ever so opaque error:
>
> ```
> Error copying directory from
> "/home/me/src/git/breeze-icons/icons-dark" to
> "/home/me/src/git/breeze-icons/build/icon
> s-dark/res".
> ```
>
> ## Fix Suggestion
>
> This is slightly less trivial since the rcc/qrc helpers seem to depend
> on resolved symlinks, so I am guessing a way to deal with this would
> be to use cmake's `file(COPY...)` instead of copy_directory and then
> have another helper run through the directories to flatten out the
> symlinks (dropping broken symlinks).
>
>
> Kindly fix this to empower the artists to know what's going on again :(
> HS
>

This PR might be relevant: https://git.reviewboard.kde.org/r/128112/
I intended to use it on Windows only, but it well may be used on *nix too.


Review Request 129590: KAuth: Make D-Bus dependency optional.

2016-11-30 Thread Gleb Popov

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

Review request for KDE Frameworks and kdewin.


Repository: kauth


Description
---

This adds an `ENABLE_DBUS` CMake option defaulting to `ON`, which can be used 
to make KAuth not depend on DBus. This is useful for some apps that are running 
on Windows.

https://www.mail-archive.com/kde-frameworks-devel@kde.org/msg34246.html


Diffs
-

  CMakeLists.txt b8b7ba7 
  autotests/CMakeLists.txt b53d760 
  src/CMakeLists.txt 1b6930d 

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


Testing
---


Thanks,

Gleb Popov



Re: Review Request 128112: New module: ecm_win_resolve_symlinks

2016-11-05 Thread Gleb Popov

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

(Updated Nov. 5, 2016, 2:40 p.m.)


Status
--

This change has been marked as submitted.


Review request for Extra Cmake Modules and KDE Frameworks.


Changes
---

Submitted with commit 632b8868de38e2f85fb4969bcae3dd89a2a0695c by Gleb Popov to 
branch master.


Repository: extra-cmake-modules


Description
---

When git is checking out repositories with UNIX symbolic links inside on 
Windows machine, it writes them as plain text files, containing relative path 
to the real file. This is the case for breeze-icons framework, for instance, 
and this breaks some icons that are symlinked.

This macro is intended to fix that. There is some room for performance 
improvement, but i wanted to get the feedback early.


Diffs
-

  modules/ECMWinResolveSymlinks.cmake PRE-CREATION 

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


Testing
---


Thanks,

Gleb Popov



Re: Review Request 129244: Make UDisks2 backend compile on FreeBSD (and, possibly, other UNIXes).

2016-10-31 Thread Gleb Popov

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

(Updated Oct. 31, 2016, 11:49 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Solid.


Changes
---

Submitted with commit 9bca1fd1993665a2233e0155a1baf9973c7a4b6f by Gleb Popov to 
branch master.


Repository: solid


Description
---

This patch makes Solid UDisks2 backend compile on FreeBSD.


Diffs
-

  src/solid/devices/CMakeLists.txt e1f3f6e 
  src/solid/devices/backends/udisks2/udisksblock.h 123e204 
  src/solid/devices/backends/udisks2/udisksblock.cpp 76cfc21 
  src/solid/devices/backends/udisks2/udisksopticaldisc.h f8dd59a 
  src/solid/devices/backends/udisks2/udisksopticaldisc.cpp a9d9c6b 
  src/solid/devices/backends/udisks2/udisksstoragedrive.h be24c7f 
  src/solid/devices/backends/udisks2/udisksstoragedrive.cpp af49063 

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


Testing
---


Thanks,

Gleb Popov



Re: Review Request 129197: Fix tests on FreeBSD

2016-10-31 Thread Gleb Popov


> On Oct. 16, 2016, 4:12 p.m., Tobias Berner wrote:
> > I'm kind of unsure if this is right. Yes, the tests run now, but isn't the 
> > issue rather in the way kpty works (or fails to work on FreeBSD)?
> 
> Gleb Popov wrote:
> From what i've understood, this boils down to 
> `KPtyDevicePrivate::_k_canRead()` method in kptydevice.cpp. The line 284
> 
> if (!::ioctl(q->masterFd(), PTY_BYTES_AVAILABLE, (char *) ))
> 
> returns 0 in `available` and this makes method return `false`. This, in 
> turn, make `waitFor*` methods return false too.
> 
> Now you mention it, i'm also unsure if this `ioctl` behaves different on 
> Linux.
> 
> Oswald Buddenhagen wrote:
> i've been trying to make sense of the freebsd pts layer, and utterly 
> failed. the documentation is anything between abysmal and non-existing. i 
> suggest you find and invite an actual expert.
> 
> kptyprocess keeps both ends of the pty open, so whether the child process 
> is running or not should be irrelevant. apparently, it's not. maybe this is 
> somehow related to whether the tty is the process' controlling terminal.
> 
> you can try the following:
> - remove the freebsd case from the PTY_BYTES_AVAILABLE definition near 
> the top. this was introduced 2008, before freebsd 8's release, which got an 
> entirely new pts layer. it may just work with the generic code now.
> - try replacing the masterFd() with slaveFd() in the above ioctl. that 
> would be kinda broken, but at least it would be a data point.

This turned out to be a bug in Qt itself. `QProcess` unit tests are also 
failing with the same `.waitForFinished()` error. So, we'll first deal with it 
and then i'll get back on this.


- Gleb


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


On Oct. 16, 2016, 2:44 p.m., Gleb Popov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129197/
> ---
> 
> (Updated Oct. 16, 2016, 2:44 p.m.)
> 
> 
> Review request for KDE Frameworks, Adriaan de Groot, Tobias Berner, Oswald 
> Buddenhagen, and Martin Tobias Holmedahl Sandsmark.
> 
> 
> Repository: kpty
> 
> 
> Description
> ---
> 
> Apparently, KPtyDevice can't be operated on after KPtyProcess finishes. Tweak 
> tests accordingly, so they actually test things while the process is still 
> running.
> 
> 
> Diffs
> -----
> 
>   autotests/kptyprocesstest.cpp 8b0b5b0 
> 
> Diff: https://git.reviewboard.kde.org/r/129197/diff/
> 
> 
> Testing
> ---
> 
> make test on FreeBSD
> 
> 
> Thanks,
> 
> Gleb Popov
> 
>



Re: Review Request 129244: Make UDisks2 backend compile on FreeBSD (and, possibly, other UNIXes).

2016-10-31 Thread Gleb Popov

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

(Updated Oct. 31, 2016, 10:03 a.m.)


Review request for KDE Frameworks and Solid.


Repository: solid


Description
---

This patch makes Solid UDisks2 backend compile on FreeBSD.


Diffs (updated)
-

  src/solid/devices/CMakeLists.txt e1f3f6e 
  src/solid/devices/backends/udisks2/udisksblock.h 123e204 
  src/solid/devices/backends/udisks2/udisksblock.cpp 76cfc21 
  src/solid/devices/backends/udisks2/udisksopticaldisc.h f8dd59a 
  src/solid/devices/backends/udisks2/udisksopticaldisc.cpp a9d9c6b 
  src/solid/devices/backends/udisks2/udisksstoragedrive.h be24c7f 
  src/solid/devices/backends/udisks2/udisksstoragedrive.cpp af49063 

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


Testing
---


Thanks,

Gleb Popov



Re: Review Request 129122: Try to use ulog-helper if utempter does not exist

2016-10-19 Thread Gleb Popov


> On Oct. 19, 2016, 11:07 p.m., Oswald Buddenhagen wrote:
> > an update about the runtime testing would be nice ;)

Here it is: https://git.reviewboard.kde.org/r/129197/

With this and that patch, the test passes on FreeBSD.


- Gleb


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


On Oct. 19, 2016, 10:51 p.m., Tobias Berner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129122/
> ---
> 
> (Updated Oct. 19, 2016, 10:51 p.m.)
> 
> 
> Review request for KDE Frameworks, Adriaan de Groot, Gleb Popov, Oswald 
> Buddenhagen, and Martin Tobias Holmedahl Sandsmark.
> 
> 
> Repository: kpty
> 
> 
> Description
> ---
> 
> FreeBSD does not have `/usr/libexec/*/utempter`. It does however have 
> `/usr/libexec/ulog-helper` [1].
> 
> It uses `login` instead of `add` and `logout` instead of `del`.
> 
> 
> [1] 
> https://svnweb.freebsd.org/base/head/libexec/ulog-helper/ulog-helper.c?revision=234469=markup
> 
> 
> Diffs
> -
> 
>   cmake/FindUTEMPTER.cmake 4921e58 
>   src/kpty.cpp 7bf31c3 
> 
> Diff: https://git.reviewboard.kde.org/r/129122/diff/
> 
> 
> Testing
> ---
> 
> Builds fine. Still need to test if it is actually working.
> 
> 
> Thanks,
> 
> Tobias Berner
> 
>



Re: Review Request 129197: Fix tests on FreeBSD

2016-10-16 Thread Gleb Popov


> On Oct. 16, 2016, 4:12 p.m., Tobias Berner wrote:
> > I'm kind of unsure if this is right. Yes, the tests run now, but isn't the 
> > issue rather in the way kpty works (or fails to work on FreeBSD)?

>From what i've understood, this boils down to 
>`KPtyDevicePrivate::_k_canRead()` method in kptydevice.cpp. The line 284

if (!::ioctl(q->masterFd(), PTY_BYTES_AVAILABLE, (char *) ))

returns 0 in `available` and this makes method return `false`. This, in turn, 
make `waitFor*` methods return false too.

Now you mention it, i'm also unsure if this `ioctl` behaves different on Linux.


- Gleb


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


On Oct. 16, 2016, 2:44 p.m., Gleb Popov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129197/
> ---
> 
> (Updated Oct. 16, 2016, 2:44 p.m.)
> 
> 
> Review request for KDE Frameworks, Adriaan de Groot, Tobias Berner, Oswald 
> Buddenhagen, and Martin Tobias Holmedahl Sandsmark.
> 
> 
> Repository: kpty
> 
> 
> Description
> ---
> 
> Apparently, KPtyDevice can't be operated on after KPtyProcess finishes. Tweak 
> tests accordingly, so they actually test things while the process is still 
> running.
> 
> 
> Diffs
> -
> 
>   autotests/kptyprocesstest.cpp 8b0b5b0 
> 
> Diff: https://git.reviewboard.kde.org/r/129197/diff/
> 
> 
> Testing
> ---
> 
> make test on FreeBSD
> 
> 
> Thanks,
> 
> Gleb Popov
> 
>



Review Request 129197: Fix tests on FreeBSD

2016-10-16 Thread Gleb Popov

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

Review request for KDE Frameworks, Adriaan de Groot, Tobias Berner, Oswald 
Buddenhagen, and Martin Tobias Holmedahl Sandsmark.


Repository: kpty


Description
---

Apparently, KPtyDevice can't be operated on after KPtyProcess finishes. Tweak 
tests accordingly, so they actually test things while the process is still 
running.


Diffs
-

  autotests/kptyprocesstest.cpp 8b0b5b0 

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


Testing
---

make test on FreeBSD


Thanks,

Gleb Popov



Re: Review Request 129122: Try to use ulog-helper if utempter does not exist

2016-10-09 Thread Gleb Popov

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




cmake/FindUTEMPTER.cmake (line 45)
<https://git.reviewboard.kde.org/r/129122/#comment67101>

Why search in `KDE_INSTALL` folders? `ulog-helper` comes with base and 
should always be in `/usr/libexec`, no?


- Gleb Popov


On Oct. 8, 2016, 9:37 p.m., Tobias Berner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129122/
> ---
> 
> (Updated Oct. 8, 2016, 9:37 p.m.)
> 
> 
> Review request for KDE Frameworks, Adriaan de Groot, Gleb Popov, and Martin 
> Tobias Holmedahl Sandsmark.
> 
> 
> Repository: kpty
> 
> 
> Description
> ---
> 
> FreeBSD does not have `/usr/libexec/*/utempter`. It does however have 
> `/usr/libexec/ulog-helper` [1].
> 
> It uses `login` instead of `add` and `logout` instead of `del`.
> 
> 
> [1] 
> https://svnweb.freebsd.org/base/head/libexec/ulog-helper/ulog-helper.c?revision=234469=markup
> 
> 
> Diffs
> -
> 
>   cmake/FindUTEMPTER.cmake 9773963 
>   src/kpty.cpp 7bf31c3 
> 
> Diff: https://git.reviewboard.kde.org/r/129122/diff/
> 
> 
> Testing
> ---
> 
> Builds fine. Still need to test if it is actually working.
> 
> 
> Thanks,
> 
> Tobias Berner
> 
>



Re: Review Request 129020: Fix find invocation in validate_svg.sh on FreeBSD.

2016-09-26 Thread Gleb Popov

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

(Updated Sept. 26, 2016, 11:49 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks.


Changes
---

Submitted with commit 9e0466a1996717e44ca8c3842abd304d69af7ea7 by Gleb Popov to 
branch master.


Repository: breeze-icons


Description
---

Without this patch the build was failing with

```
find: illegal option -- n
usage: find [-H | -L | -P] [-EXdsx] [-f path] path ... [expression]
   find [-H | -L | -P] [-EXdsx] -f path [path ...] [expression]
```


Diffs
-

  validate_svg.sh bbc3076 

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


Testing
---


Thanks,

Gleb Popov



Review Request 129020: Fix find invocation in validate_svg.sh on FreeBSD.

2016-09-26 Thread Gleb Popov

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

Review request for KDE Frameworks.


Repository: breeze-icons


Description
---

Without this patch the build was failing with

```
find: illegal option -- n
usage: find [-H | -L | -P] [-EXdsx] [-f path] path ... [expression]
   find [-H | -L | -P] [-EXdsx] -f path [path ...] [expression]
```


Diffs
-

  validate_svg.sh bbc3076 

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


Testing
---


Thanks,

Gleb Popov



Re: Review Request 128916: kconfig: Make test XFAIL when running as root

2016-09-16 Thread Gleb Popov


> On Sept. 16, 2016, 7:57 p.m., Sune Vuorela wrote:
> > autotests/kconfigtest.cpp, line 1376
> > <https://git.reviewboard.kde.org/r/128916/diff/4/?file=476949#file476949line1376>
> >
> > Isn't a QSKIP if user is root in the beginning of the test a better 
> > approach ?

I think, XFAIL is better idea, because if anyone ever would want to figure out 
why it is expected, it would instantly found out the cause.


- Gleb


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


On Sept. 16, 2016, 10:17 a.m., Evgeniy Sadovnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128916/
> ---
> 
> (Updated Sept. 16, 2016, 10:17 a.m.)
> 
> 
> Review request for KDE Frameworks and Gleb Popov.
> 
> 
> Repository: kconfig
> 
> 
> Description
> ---
> 
> The test checks that saving a read-only config file fails. But because root 
> can write into read-only files, the test is failing when running by root.
> Check for uid when running the test and make it XFAIL if we are running as 
> root.
> 
> 
> Diffs
> -
> 
>   autotests/kconfigtest.cpp 2b905b5 
> 
> Diff: https://git.reviewboard.kde.org/r/128916/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Evgeniy Sadovnik
> 
>



Re: Review Request 128916: kconfig: Make test XFAIL when running as root

2016-09-16 Thread Gleb Popov

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



I'd say ship it, but let's wait for someone else.

- Gleb Popov


On Sept. 16, 2016, 10:17 a.m., Evgeniy Sadovnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128916/
> ---
> 
> (Updated Sept. 16, 2016, 10:17 a.m.)
> 
> 
> Review request for KDE Frameworks and Gleb Popov.
> 
> 
> Repository: kconfig
> 
> 
> Description
> ---
> 
> The test checks that saving a read-only config file fails. But because root 
> can write into read-only files, the test is failing when running by root.
> Check for uid when running the test and make it XFAIL if we are running as 
> root.
> 
> 
> Diffs
> -
> 
>   autotests/kconfigtest.cpp 2b905b5 
> 
> Diff: https://git.reviewboard.kde.org/r/128916/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Evgeniy Sadovnik
> 
>



Re: Review Request 128916: kconfig: Make test XFAIL when running as root

2016-09-15 Thread Gleb Popov


> On Sept. 15, 2016, 3:01 p.m., Gleb Popov wrote:
> > autotests/kconfigtest.cpp, line 1378
> > <https://git.reviewboard.kde.org/r/128916/diff/1/?file=476914#file476914line1378>
> >
> > I'd put both `cgLocal.writeEntry("someLocalString", "whatever2");` and 
> > `QVERIFY(!cgLocal.sync());` inside else clause.

After looking at `QEXPECT_FAIL()` documentation, i've figured you are right.


- Gleb


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


On Sept. 15, 2016, 4:20 p.m., Evgeniy Sadovnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128916/
> ---
> 
> (Updated Sept. 15, 2016, 4:20 p.m.)
> 
> 
> Review request for KDE Frameworks and Gleb Popov.
> 
> 
> Repository: kconfig
> 
> 
> Description
> ---
> 
> The test checks that saving a read-only config file fails. But because root 
> can write into read-only files, the test is failing when running by root.
> Check for uid when running the test and make it XFAIL if we are running as 
> root.
> 
> 
> Diffs
> -
> 
>   autotests/kconfigtest.cpp 2b905b5 
> 
> Diff: https://git.reviewboard.kde.org/r/128916/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Evgeniy Sadovnik
> 
>



Re: Review Request 128916: kconfig: Make test XFAIL when running as root

2016-09-15 Thread Gleb Popov

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




autotests/kconfigtest.cpp (line 1373)
<https://git.reviewboard.kde.org/r/128916/#comment66803>

getuid() isn't present on Windows. Guard this check with `#ifndef Q_OS_WIN`.


- Gleb Popov


On Sept. 15, 2016, 4:20 p.m., Evgeniy Sadovnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128916/
> ---
> 
> (Updated Sept. 15, 2016, 4:20 p.m.)
> 
> 
> Review request for KDE Frameworks and Gleb Popov.
> 
> 
> Repository: kconfig
> 
> 
> Description
> ---
> 
> The test checks that saving a read-only config file fails. But because root 
> can write into read-only files, the test is failing when running by root.
> Check for uid when running the test and make it XFAIL if we are running as 
> root.
> 
> 
> Diffs
> -
> 
>   autotests/kconfigtest.cpp 2b905b5 
> 
> Diff: https://git.reviewboard.kde.org/r/128916/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Evgeniy Sadovnik
> 
>



Re: Review Request 128916: kconfig: Make test XFAIL when running as root

2016-09-15 Thread Gleb Popov

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




autotests/kconfigtest.cpp 
<https://git.reviewboard.kde.org/r/128916/#comment66800>

I'd put both `cgLocal.writeEntry("someLocalString", "whatever2");` and 
`QVERIFY(!cgLocal.sync());` inside else clause.


- Gleb Popov


On Sept. 15, 2016, 2:59 p.m., Evgeniy Sadovnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128916/
> ---
> 
> (Updated Sept. 15, 2016, 2:59 p.m.)
> 
> 
> Review request for KDE Frameworks and Gleb Popov.
> 
> 
> Repository: kconfig
> 
> 
> Description
> ---
> 
> The test checks that saving a read-only config file fails. But because root 
> can write into read-only files, the test is failing when running by root.
> Check for uid when running the test and make it XFAIL if we are running as 
> root.
> 
> 
> Diffs
> -
> 
>   autotests/kconfigtest.cpp 2b905b5 
> 
> Diff: https://git.reviewboard.kde.org/r/128916/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Evgeniy Sadovnik
> 
>



Re: Review Request 128855: Fix build on Windows

2016-09-08 Thread Gleb Popov

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

(Updated Sept. 8, 2016, 4:27 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks.


Changes
---

Submitted with commit 739321ad95994fde874d8e09b698c465cc52b9ec by Gleb Popov to 
branch master.


Repository: kio


Description
---

Windows doesn't have `S_IRUSR` & co defines, so use what it has and stub out 
remainings.


Diffs
-

  src/core/slavebase.cpp 4b8f460 

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


Testing
---


Thanks,

Gleb Popov



Re: Review Request 128855: Fix build on Windows

2016-09-07 Thread Gleb Popov


> On Sept. 7, 2016, 9:24 p.m., Kevin Funk wrote:
> > `kioglobal_p.h` defines those. Just include that one?

Yep, you are right, this fixes build too. Ok to commit this one-liner?

```
+ #include "kioglobal_p.h"
```


- Gleb


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


On Sept. 7, 2016, 6:23 p.m., Gleb Popov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128855/
> ---
> 
> (Updated Sept. 7, 2016, 6:23 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> Windows doesn't have `S_IRUSR` & co defines, so use what it has and stub out 
> remainings.
> 
> 
> Diffs
> -
> 
>   src/core/slavebase.cpp 4b8f460 
> 
> Diff: https://git.reviewboard.kde.org/r/128855/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gleb Popov
> 
>



Review Request 128855: Fix build on Windows

2016-09-07 Thread Gleb Popov

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

Review request for KDE Frameworks.


Repository: kio


Description
---

Windows doesn't have `S_IRUSR` & co defines, so use what it has and stub out 
remainings.


Diffs
-

  src/core/slavebase.cpp 4b8f460 

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


Testing
---


Thanks,

Gleb Popov



[Differential] [Commented On] D2545: Cleanup KSharedUiServerProxy before qApp exists

2016-09-02 Thread arrowdodger (Gleb Popov)
arrowdodger added a comment.


  In https://phabricator.kde.org/D2545#48488, @dfaure wrote:
  
  > Hmm can you check if https://codereview.qt-project.org/161056 fixes it?
  
  
  I've tried both Thiago patches on 5.7 branch and they don't fix this problem.

BRANCH
  master

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

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

To: kfunk, vonreth, dfaure
Cc: arrowdodger, #frameworks


  1   2   >