Re: Major CI changes - FreeBSD and Linux

[Gleb Popov]

On Mon, Jan 22, 2024 at 12:09 PM Ben Cooksley  wrote:
>
> Hi all,
>
> Over the past few weeks significant work has been undertaken to develop the 
> ability to make use of containerised builds for FreeBSD.

This is great news! Thank you for all the work done on that front.

Re: Major CI changes - FreeBSD and Linux

[Gleb Popov]

On Mon, Jan 22, 2024 at 12:09 PM Ben Cooksley  wrote:
>
> Hi all,
>
> Over the past few weeks significant work has been undertaken to develop the 
> ability to make use of containerised builds for FreeBSD.

This is great news! Thank you for all the work done on that front.

Re: Major CI changes - FreeBSD and Linux

[Gleb Popov]

On Mon, Jan 22, 2024 at 12:09 PM Ben Cooksley  wrote:
>
> Hi all,
>
> Over the past few weeks significant work has been undertaken to develop the 
> ability to make use of containerised builds for FreeBSD.

This is great news! Thank you for all the work done on that front.

Re: Can we rename gitklient?

[Gleb Popov]

On Thu, Mar 2, 2023 at 9:33 PM Hamed Masafi  wrote:
>
> Considering that gitklient is very close to the first version, I have to say 
> something about its naming.
> When I started the project, no name came to my mind. Of course, it is the 
> same now. That's why, just so that the name of the project is not empty, 
> change its name to gitklient, in kde 3 naming style (I know).
> Considering that the first version of this app has not been published yet, 
> can we change the name?
> If the answer is yes, what name should we choose? To be honest, I still don't 
> have a suitable name in mind!
>
> I sent this email so that we can talk about this (of course, if we can change 
> the name of the program) and find a good name for it.
>
> I had a long chat with ChatGpt! Also, I got help from some name suggestion 
> sites and asked some people for their opinion. Finally, I have a list of 
> suggested names to start a discussion.
>
> - Kodify
> - Kittie
> - Kode
> - Kommit

I really like "Kommit". It starts with "K" and is just to the point.

Re: Gitlab CI Dashboards and retirement of build.kde.org

[Gleb Popov]

On Sat, Sep 3, 2022 at 7:46 AM Ben Cooksley  wrote:
>
> As previously indicated, I have now shutdown build.kde.org along with the 
> domain that supported it's version of the CI tooling.
> The repository containing that tooling has now also been archived, and the 
> former build.kde.org domain has been redirected to metrics.kde.org.
>
> The server which was acting as a builder for build.kde.org will be rebuilt in 
> the coming days and reallocated to support Gitlab CI workloads.
>
> Thanks,
> Ben

What should be used instead of binary-factory? How do I transform this link?

https://binary-factory.kde.org/view/Windows%2064-bit/job/Kate_Release_win64/1762/artifact/kate-22.08.0-1762-windows-msvc2019_64-cl.exe

Re: Gitlab CI Dashboards and retirement of build.kde.org

[Gleb Popov]

On Sat, Sep 3, 2022 at 7:46 AM Ben Cooksley  wrote:
>
> As previously indicated, I have now shutdown build.kde.org along with the 
> domain that supported it's version of the CI tooling.
> The repository containing that tooling has now also been archived, and the 
> former build.kde.org domain has been redirected to metrics.kde.org.
>
> The server which was acting as a builder for build.kde.org will be rebuilt in 
> the coming days and reallocated to support Gitlab CI workloads.
>
> Thanks,
> Ben

What should be used instead of binary-factory? How do I transform this link?

https://binary-factory.kde.org/view/Windows%2064-bit/job/Kate_Release_win64/1762/artifact/kate-22.08.0-1762-windows-msvc2019_64-cl.exe

Re: Gitlab CI Dashboards and retirement of build.kde.org

[Gleb Popov]

On Sat, Sep 3, 2022 at 7:46 AM Ben Cooksley  wrote:
>
> As previously indicated, I have now shutdown build.kde.org along with the 
> domain that supported it's version of the CI tooling.
> The repository containing that tooling has now also been archived, and the 
> former build.kde.org domain has been redirected to metrics.kde.org.
>
> The server which was acting as a builder for build.kde.org will be rebuilt in 
> the coming days and reallocated to support Gitlab CI workloads.
>
> Thanks,
> Ben

What should be used instead of binary-factory? How do I transform this link?

https://binary-factory.kde.org/view/Windows%2064-bit/job/Kate_Release_win64/1762/artifact/kate-22.08.0-1762-windows-msvc2019_64-cl.exe

Re: Rollout of Gitlab CI

[Gleb Popov]

On Sun, Oct 3, 2021 at 10:40 AM Ben Cooksley  wrote:

> As an update to this, work on FreeBSD and Android support is progressing
> well and should be made available within the next week all going well.
>

As a FreeBSD user I'm extremely excited about this. Thanks for all your
hard work!

Cheers,
> Ben
>

Re: Rollout of Gitlab CI

[Gleb Popov]

On Sun, Oct 3, 2021 at 10:40 AM Ben Cooksley  wrote:

> As an update to this, work on FreeBSD and Android support is progressing
> well and should be made available within the next week all going well.
>

As a FreeBSD user I'm extremely excited about this. Thanks for all your
hard work!

Cheers,
> Ben
>

Re: Trying to build knotifications framework, kdesrc-build and qt5

[Gleb Popov]

On Mon, Aug 2, 2021 at 12:26 AM Albert Astals Cid  wrote:

> Your Qt is broken.
>
> /usr/local/lib/libQt5Core.so.5: undefined symbol: u_strToUpper_68
>

This symbol comes from the icu library. There is a setting that adds
version suffixes like "_68" to every symbol. It seems that Qt runs into a
header mismatch or something like that.

D17816: Support for xattrs on kio copy/move

[Gleb Popov]

arrowd abandoned this revision.
arrowd added a comment.


  Mark the revision as closed to reflect reality.

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

D17816: Support for xattrs on kio copy/move

[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

[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

[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

[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

[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

[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

[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

[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

Re: Broken KRFB Build on non-linux platforms

[Gleb Popov]

On Fri, Sep 18, 2020 at 10:16 PM Ben Cooksley  wrote:

> Hi Aleix,
>
> Recently you committed some Linux only changes to KRFB. Unfortunately
> these changes were not guarded accordingly, and have therefore broken the
> build on all other platforms (FreeBSD in particular).
>
> Can you please correct this?
>
> As a side note, Pipewire is showing up far too often as a cause of build
> failures. To those developers doing work with this technology, please take
> extra care to ensure that you are not breaking the build.
>

I made a port of PipeWire for FreeBSD some time ago. Maybe the CI needs to
be adjusted to pull in multimedia/pipewire package?

D17816: Support for xattrs on kio copy/move

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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: Update on Status of Gitlab Migration

[Gleb Popov]

On Sat, Apr 11, 2020 at 1:36 PM Ben Cooksley  wrote:

> Good morning Community,
>
> I'm pleased to report that this week we reached a major milestone,
> with all the necessary technical components now being in place on our
> side for our migration to Gitlab to take place.
>
> This includes the replacement of the tooling that supports the anongit
> network, as well as implementation of a system to sync changes from
> Identity to Gitlab in real time.
>
> As part of this, the anongit network following the migration to Gitlab
> should pick up new repositories, along with changes to default
> branches, project descriptions, repository names and their path, and
> the deletion of repositories within 10 seconds of the change taking
> place on Gitlab (effectively making it real time)
>
> We are currently looking to confirm how repositories will be organised
> on Gitlab, after which we should be able to confirm how and when we
> perform the migration.
>
> Please note that only code hosting and code review will be
> transitioning at this time - CI and tasks will follow later on.
> Existing reviews on Phabricator will not be imported to Gitlab as part
> of this process and will need to be closed out on Phabricator.
>
> To help assist with this, it would be appreciated if all holders of a
> KDE Developer account could please login on our Gitlab instance (at
> https://invent.kde.org/) and ensure they are listed as a 'Developer'
> of the KDE group. You can do this by checking the 'Groups' tab of your
> Profile on Gitlab.
>
> Should you not be listed as a Developer of the group, and you have
> previously logged into Gitlab, please visit KDE Identity and make a
> change to your details there, which will trigger a sync of your
> account to Gitlab.
>

Hi Ben, thanks for working on this.

My account https://invent.kde.org/users/arrowdodger/groups is not in
"Developers" group, and changing details on Identity website doesn't seem
to trigger any updates on Gitlab.
While on it, is it still impossible to turn "arrowdodger" into "arrowd"?


> Should anyone have any questions on the above, please let us know.
>
> Thanks,
> Ben Cooksley
> KDE Sysadmin
>

Re: Does FreeBSD have HAL?

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

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

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

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

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

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

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

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

[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

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

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

[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

[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

[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

[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

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

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

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

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

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

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

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

[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

Re: Gitlab Evaluation & Migration

[Gleb Popov]

On Thu, Feb 28, 2019 at 9:43 PM Ben Cooksley  wrote:

> On Fri, Mar 1, 2019 at 3:13 AM Nate Graham  wrote:
> >
> >   On Wed, 27 Feb 2019 23:02:03 -0700 Ben Cooksley 
> wrote 
> >  > In terms of server load, it would be nice if the setup of forks was
> >  > still something the developer had to initiate rather than being done
> >  > automatically for every repository touched by kdesrc-build (I say this
> >  > mainly as if we had 50 people fork just half of the mainline
> >  > repositories we have, that's ~450GB of space used up - a massive
> >  > scalability issue)
> >
> > This seems like a challenge that needs to be addressed regardless of
> whether or not kdesrc-build does it automatically, because people creating
> tons and tons of forks is guaranteed to happen anyway if we move to Gitlab.
> It seems non-optimal if having more people able to submit merge requests
> results in the potential to blow up our servers.
>
> We have a little over 1,000 mainline repositories, so in the above
> example we'd be talking about 25,000 forks being created - and i'd be
> expecting quite a bit more than 50 people to use kdesrc-build. To use
> another scenario, if the metric of half the repositories being
> involved (or even a quarter) held true with say 300 users, you're now
> looking at 75,000 - 150,000 forked repositories (and probably around
> 1.4TB - 2.7TB of space used) courtesy of an automated tool.
>
> It would take quite a while for us to reach 150,000 forked
> repositories on Gitlab if humans were to be creating these manually,
> however if an automated tool is going to be creating them as part of
> it's workflow, then we will hit it much more quickly (and is a
> phenomenal waste of resources given virtually all of those forks will
> never be utilised)
>

I wonder if advanced filesystem features like ZFS deduplication may help in
this situation.


> I certainly do expect a number of forks to be created yes, but i'd
> rather they be useful forks where someone at least intends on working
> on something, rather than ones created automatically by software "just
> in case" someone decides to work on a project.
>
> >
> > Nate
> >
>
> Cheers,
> Ben
>

Re: Gitlab Evaluation & Migration

[Gleb Popov]

On Sat, Feb 23, 2019 at 1:44 PM Ben Cooksley  wrote:

> Hi all,
>
> Over the past few months a small group of KDE projects have been
> evaluating Gitlab as a replacement for Phabricator, and we’ve now
> reached the point where we’re ready to have a community wide
> discussion regarding whether we’d like to migrate to Gitlab. We'd like
> to start this by thanking those projects that have assisted in
> evaluating Gitlab for KDE, at list of which can be found at
> https://invent.kde.org/kde/
>
> This evaluation process was started in response to feedback Sysadmin
> received in the BoF session in Akademy last year regarding issues
> developers we're having with Phabricator. These included the ability
> to see the submitters details (name and email address), ability to do
> multi-commit reviews and to merge changes conveniently from the web
> amongst other things.
>
> Based on the feedback we’ve received to date regarding the Gitlab
> evaluation, the consensus seems to be that a migration would be
> beneficial and helpful to us in the long term.
>
> Among the benefits identified thus far are:
> 1) Provision of full featured task management and code review
> functionality to scratch (personal) repositories, which will ease the
> transition to playground and first release.
> 2) Ability to handle native Git commits as part of the code review
> process and merge commits from the web interface rather than having to
> take additional steps to integrate a change.
> 3) Easier and more logical grouping of projects, including being able
> to view the tasks and repositories related to a specific project more
> intuitively
>
> Further notes on the evaluation can be found at
> https://notes.kde.org/p/gitlab-evaluation-notes
>
> In regards to Phabricator it should be noted that upstream does not
> currently have plans to work on features which would resolve the
> issues we've encountered, and their responsiveness to inquiries from
> us has decreased since we migrated to it several years ago.


My own experience agrees with this statement. I've got an impression that
Phabricator devs completely not interested in bug reports coming from users
with a pact (a term for paid support used by Phacility).

So, I'm all for moving from Phabricator and that PHP-based CLI.

Gitlab on
> the other hand has a thriving community which openly accepts patches
> and have been very helpful in assisting us with the evaluation process
> (including solving problems we've found).
>
> In addition to sysadmins and some of projects maintainers, KDE e.V.
> board, and onboarding goal team has been involved in Gitlab evaluation
> process as well, and they've made the following comments:
>
> Neofytos Kolokotronis of the Onboarding goal team:
> We believe this switch will be a great step forward for the Onboarding
> goal as well. The workflow, features and general behavior of Gitlab
> should be much more familiar to developers and non-coders interested
> to contribute to KDE, thus lowering the entry barrier.  Further,
> people coming from FOSS projects already on Github or Gitlab should
> find it very straightforward to start contributing to KDE right away.
> One area that is not to be ignored is that Gitlab has some great and
> up to date documentation.
>
> Eike Hein on behalf of the KDE e.V. board of directors:
> After sysadmin gave us a situation report on our code hosting and
> review infrastructure last year, we were initially involved with
> building a relationship with GitLab upstream and setting up a call
> schedule. We then turned the evaluation over to the sysadmin and
> onboarding teams and received continual updates on its progress. We're
> impressed by and thankful for the work done by everyone involved in
> the intervening months, and stand by to help implement a community
> decision based on what was collected.
>
> Based on all of the above we'd like to propose migrating towards
> Gitlab. Comments?
>
> Thanks,
> Ben Cooksley
> KDE Sysadmin
>

D17938: Add support for passing cursor information to KRun.

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

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

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

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

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

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

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

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

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

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

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

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

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

[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

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

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

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

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

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

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

[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

[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

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

[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 129277: Use built-in QByteArrayList type instead of our own.

[Gleb Popov]

> On Nov. 16, 2016, 2:37 a.m., Christoph Feck wrote:
> > Any hope that this could have fixed the cause for bug 345871?

Can't comment on that, since I've been developing on FreeBSD, and our UDisks2 
implementation doesn't handle partitions properly yet.


- Gleb


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


On Oct. 31, 2016, 1:49 p.m., Gleb Popov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129277/
> ---
> 
> (Updated Oct. 31, 2016, 1:49 p.m.)
> 
> 
> Review request for Solid.
> 
> 
> Repository: solid
> 
> 
> Description
> ---
> 
> This patch replaces uses of `typedef QList ByteArrayList;` with 
> built-in `QByteArrayList` type. This type was in Qt since 5.4, while Solid 
> requires 5.5.
> 
> 
> Diffs
> -
> 
>   src/solid/devices/backends/udisks2/udisks2.h 0abf915 
>   src/solid/devices/backends/udisks2/udisksdevice.cpp 5283c2a 
>   src/solid/devices/backends/udisks2/udisksmanager.cpp 1e149c2 
>   src/solid/devices/backends/udisks2/udisksstorageaccess.cpp 1bd6acf 
> 
> Diff: https://git.reviewboard.kde.org/r/129277/diff/
> 
> 
> Testing
> ---
> 
> Compiles, mounting works.
> 
> 
> Thanks,
> 
> Gleb Popov
> 
>

Re: Review Request 128112: New module: ecm_win_resolve_symlinks

[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 129293: Add a CMake option to switch between HAL and UDisks managers on FreeBSD.

[Gleb Popov]

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

(Updated Oct. 31, 2016, 7:38 p.m.)


Status
--

This change has been marked as submitted.


Review request for Solid and Tobias Berner.


Changes
---

Submitted with commit 96be26ee6902c9d2aa8123a540f00ce0e2751871 by Gleb Popov to 
branch master.


Repository: solid


Description
---

While these backends can coexist, this would result in duplicated device 
entries in Solid for a single real device.


Diffs
-

  src/solid/devices/CMakeLists.txt d51ebb3 
  src/solid/devices/config-solid.h.cmake a68af32 
  src/solid/devices/managerbase.cpp eee4de5 

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


Testing
---

Compile.


Thanks,

Gleb Popov

Re: Review Request 129293: Add a CMake option to switch between HAL and UDisks managers on FreeBSD.

[Gleb Popov]

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

(Updated Oct. 31, 2016, 3:58 p.m.)


Review request for Solid and Tobias Berner.


Repository: solid


Description
---

While these backends can coexist, this would result in duplicated device 
entries in Solid for a single real device.


Diffs (updated)
-

  src/solid/devices/CMakeLists.txt d51ebb3 
  src/solid/devices/config-solid.h.cmake a68af32 
  src/solid/devices/managerbase.cpp eee4de5 

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


Testing
---

Compile.


Thanks,

Gleb Popov