pino added a comment.
Nice progresses, thanks for the fixes. I added few more notes, just mentioning the first occurrence of each. One more thing is to print errno (and possibly its string representation using `strerror`/`strerror_r`) on failure, so that the debugging is easier. INLINE COMMENTS > jobtest.cpp:454 > const int perms = 0666; > + > // copy the file with file_copy extra empty line > jobtest.cpp:534 > + * Set xattrs on source file using default linux commandline > + * TODO: port to multiplatfom > + * TODO: write tests for copyAs you can use `QStandardPaths::findExecutable()` to check whether `setfattr` is available, and QSKIP the test if not > jobtest.cpp:540 > + QString command = QString("setfattr -n user.baloo.rating -v 1 > %1").arg(src); > + xattrwriter.start(command); > + xattrwriter.waitForFinished(-1); please use the `QProcess::start(QString,QStringList)` variant, splitting the arguments; this way there is no need to manual quoting the paths, and thus avoid problems with spaces > jobtest.cpp:540 > + QString command = QString("setfattr -n user.baloo.rating -v 1 > %1").arg(src); > + xattrwriter.start(command); > + xattrwriter.waitForFinished(-1); check using `QVERIFY`/`QCOMPARE` that the process was started correctly > jobtest.cpp:541 > + xattrwriter.start(command); > + xattrwriter.waitForFinished(-1); > + command = QString("setfattr -n user.fnoValue %1").arg(src); check using `QVERIFY`/`QCOMPARE` that the process ended correctly > CMakeLists.txt:5-6 > > +check_include_files(sys/xattr.h HAVE_SYS_XATTR_H) > +check_include_files(sys/extattr.h HAVE_SYS_EXTATTR_H) > + good checks, although they fit better in `ConfigureChecks.cmake` (which is there for a reasons ;) ) > copyjob.cpp:2192 > + if (!QFileInfo::exists(source.toLocalFile()) && > !QFileInfo::exists(dest.toLocalFile())) { > + qCWarning(KIO_COPYJOB_DEBUG) << "failed to open source and destiny"; > + return; destiny is another thing ;) "destination" in this case > copyjob.cpp:2205-2209 > + if (listlen < 0) { > + qCWarning(KIO_COPYJOB_DEBUG) << "libc failed to extract xattr from " > << xattrsrc; > + } else if (listlen == 0) { > + qCDebug(KIO_COPYJOB_DEBUG) << "File " << xattrsrc << " don't have > any xattr."; > + } else { this is more of a personal taste rather than an issue: IMHO you can avoid the "if .. else if ... else" that adds a nesting level more, especially when all the checks lead to early return; so something like: if (listlen < 0) { if (errno != ENOTSUP) { qCWarning(KIO_COPYJOB_DEBUG) << "failed to extract xattr from" << xattrsrc; } return; } if (listlen == 0) { qCDebug(KIO_COPYJOB_DEBUG) << "File" << xattrsrc << "doesn't have any xattr."; return; } [etc..] > copyjob.cpp:2256 > + if (errno == ENOTSUP) { > + qCWarning(KIO_COPYJOB_DEBUG) << "destination filesystem > don't support xattrs."; > + } else { IMHO this is not a warning worth situation REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D17816 To: cochise, dfaure Cc: abika, pino, davidedmundson, ngraham, atha.kane, spoorun, nicolasfella, kde-frameworks-devel, michaelh, bruns