312001 2012-12-20 18:35:51 +0000 Amarok may skip files when copying into collection with some tracks already present and not overwriting 2012-12-22 21:03:44 +0000 1 1 2 Applications amarok Collections/Local 2.6.0 Other Linux RESOLVED FIXED NOR normal 2.7 1 dmitry.torokhov amarok-bugs-null matej ralf-engels rdieter http://commits.kde.org/amarok/32c9f7ebf8ca7abf6d5397d9d74cadaa8c676ec2 2.7 0 oldest_to_newest 1325190 0 dmitry.torokhov 2012-12-20 18:35:51 +0000 When selecting a tree of music files and selecting "Copy to Collection" - "Local Collection" from the context menu, and choosing copying tracks without overwriting already existing files, the transfer job will end prematurely if some tracks already exist at destination. Reproducible: Always Steps to Reproduce: 1. Import a directory 2. Remove several files that has just been imported from the local collection 3. Try importing the same directory again Actual Results: Some of the deleted files will still be missing in the local collection. Expected Results: Local collection should contain all tracks present in the original directory. The TransferJob class in src/core-impl/collections/db/sql/SqlCollectionLocation.cpp is inherited from KCompositeJob; transfers are done via KIO::file_copy() and are registered as sub-jobs of TransferJob. The default policy of KCompositeJob is to terminate it early whenever a sub-job fails, so when copy job fails because file is already there we terminate the entire transfer instead of continuing. One possible fix is overriding slotResult() method and handle (ignore) KIO::ERR_FILE_ALREADY_EXIST errors. 1325192 1 75943 dmitry.torokhov 2012-12-20 18:40:51 +0000 Created attachment 75943 Patch to ignore ERR_FILE_ALREADY_EXIST when copying files This patch allows me to copy missing files into local collection skipping ones that are already present and not stopping too early. 1325473 2 myriam 2012-12-21 15:42:44 +0000 (In reply to comment #1) > Created attachment 75943 [details] > Patch to ignore ERR_FILE_ALREADY_EXIST when copying files > > This patch allows me to copy missing files into local collection skipping > ones that are already present and not stopping too early. Thank you for the patch, but please submit it to http://reviewboard.kde.org (you will need an identity on http://identity.kde.org for that). 1325547 3 dmitry.torokhov 2012-12-21 20:50:26 +0000 On Fri, Dec 21, 2012 at 03:42:44PM +0000, Myriam Schweingruber wrote: > https://bugs.kde.org/show_bug.cgi?id=312001 > > --- Comment #2 from Myriam Schweingruber <myriam@kde.org> --- > (In reply to comment #1) > > Created attachment 75943 [details] > > Patch to ignore ERR_FILE_ALREADY_EXIST when copying files > > > > This patch allows me to copy missing files into local collection skipping > > ones that are already present and not stopping too early. > > Thank you for the patch, but please submit it to http://reviewboard.kde.org > (you will need an identity on http://identity.kde.org for that). Really?? A maintainer can't simply evaluate patch and take it from there? I do not really plan to start developing KDE at this time... OK, I am a glutton for punishment, I have created a new identity and tried uploading the patch. But this god awful reviewboard insists that what I have is not really a patch (even though I tried feeding it both unified and context version). Do I really need to have git tree to work with reviewboard? Why can't projects use sane venues to work with patches (AKA mailing lists)? Bleh :( 1325550 4 rdieter 2012-12-21 21:03:43 +0000 I'd be willing to help get this posted to reviewboard over the holidays, it does seem like an obvious improvement. 1325554 5 dmitry.torokhov 2012-12-21 21:23:04 +0000 On Fri, Dec 21, 2012 at 09:03:43PM +0000, Rex Dieter wrote: > https://bugs.kde.org/show_bug.cgi?id=312001 > > Rex Dieter <rdieter@math.unl.edu> changed: > > What |Removed |Added > ---------------------------------------------------------------------------- > Status|UNCONFIRMED |CONFIRMED > Ever confirmed|0 |1 > > --- Comment #4 from Rex Dieter <rdieter@math.unl.edu> --- > I'd be willing to help get this posted to reviewboard over the holidays, it > does seem like an obvious improvement. Thanks Rex, I am just grumpy. https://git.reviewboard.kde.org/r/107841/ 1325620 6 myriam 2012-12-22 10:24:40 +0000 (In reply to comment #3) > On Fri, Dec 21, 2012 at 03:42:44PM +0000, Myriam Schweingruber wrote: > > https://bugs.kde.org/show_bug.cgi?id=312001 > > > > --- Comment #2 from Myriam Schweingruber <myriam@kde.org> --- > > (In reply to comment #1) > > > Created attachment 75943 [details] > > > Patch to ignore ERR_FILE_ALREADY_EXIST when copying files > > > > > > This patch allows me to copy missing files into local collection skipping > > > ones that are already present and not stopping too early. > > > > Thank you for the patch, but please submit it to http://reviewboard.kde.org > > (you will need an identity on http://identity.kde.org for that). > > Really?? A maintainer can't simply evaluate patch and take it from there? > I do not really plan to start developing KDE at this time... > > OK, I am a glutton for punishment, I have created a new identity and > tried uploading the patch. But this god awful reviewboard insists that > what I have is not really a patch (even though I tried feeding it both > unified and context version). Do I really need to have git tree to work > with reviewboard? > > Why can't projects use sane venues to work with patches (AKA mailing > lists)? Bleh :( Well, reviewboard expects you to just upload a diff, if you remove the text at the start of your patch file this should work. Add the comment to the description instead, or mark it as a comment in the patch body. 1325760 7 matej 2012-12-22 21:03:44 +0000 Git commit 32c9f7ebf8ca7abf6d5397d9d74cadaa8c676ec2 by Matěj Laitl, on behalf of Dmitry Torokhov. Committed on 22/12/2012 at 20:15. Pushed by laitl into branch 'master'. SqlCollectionLocation: do not stop transfer when some files are already at destination This happens when you want to import a big tree of tracks at once and some tracks are already present in the destination collection. In this case TransferJob's copy subjob fails with KIO::ERR_FILE_ALEADY_EXIST and causes entire transfer to complete prematurely. BUGFIXES: * Fix existing files preventing complete transfer of tracks to collection REVIEW: 107841 FIXED-IN: 2.7 M +1 -0 ChangeLog M +10 -1 src/core-impl/collections/db/sql/SqlCollectionLocation.cpp M +1 -0 src/core-impl/collections/db/sql/SqlCollectionLocation.h http://commits.kde.org/amarok/32c9f7ebf8ca7abf6d5397d9d74cadaa8c676ec2 75943 2012-12-20 18:40:51 +0000 2012-12-20 18:40:51 +0000 Patch to ignore ERR_FILE_ALREADY_EXIST when copying files amarok-skip-existing-files.patch text/plain 2137 dmitry.torokhov QW1hcm9rOiB0byBub3Qgc3RvcCB0cmFuZWZlcnMgd2hlbiBzb21lIGZpbGVzIGFyZSBhbHJlYWR5 IGF0IGRlc3RpbmF0aW9uCgpUaGlzIGhhcHBlbnMgd2hlbiB5b3Ugd2FudCB0byBpbXBvcnQgYSBi aXQgdHJlZSBvZiB0cmFja3MgYXQgb25jZSBhbmQgc29tZQp0cmFja3MgYXJlIGFscmVhZHkgcHJl c2VudCBpbiB0aGUgZGVzdGluYXRpb24gY29sbGVjdGlvbi4KClNpZ25lZC1vZmYtYnk6IERtaXRy eSBUb3Jva2hvdiA8ZG1pdHJ5LnRvcm9raG92QGdtYWlsLmNvbT4KLS0tCgpkaWZmIC1ydSBhbWFy b2stMi42LjAtb3JpZy9zcmMvY29yZS1pbXBsL2NvbGxlY3Rpb25zL2RiL3NxbC9TcWxDb2xsZWN0 aW9uTG9jYXRpb24uY3BwIGFtYXJvay0yLjYuMC9zcmMvY29yZS1pbXBsL2NvbGxlY3Rpb25zL2Ri L3NxbC9TcWxDb2xsZWN0aW9uTG9jYXRpb24uY3BwCi0tLSBhbWFyb2stMi42LjAtb3JpZy9zcmMv Y29yZS1pbXBsL2NvbGxlY3Rpb25zL2RiL3NxbC9TcWxDb2xsZWN0aW9uTG9jYXRpb24uY3BwCTIw MTItMDgtMTEgMDI6MDk6MzYuMDAwMDAwMDAwIC0wNzAwCisrKyBhbWFyb2stMi42LjAvc3JjL2Nv cmUtaW1wbC9jb2xsZWN0aW9ucy9kYi9zcWwvU3FsQ29sbGVjdGlvbkxvY2F0aW9uLmNwcAkyMDEy LTEyLTIwIDEwOjA3OjU1LjQzOTYxNjU5NyAtMDgwMApAQCAtMzgzLDcgKzM4Myw3IEBACiAgICAg REVCVUdfQkxPQ0sKIAogICAgIE1ldGE6OlRyYWNrUHRyIHRyYWNrID0gbV9qb2JzLnZhbHVlKCBq b2IgKTsKLSAgICBpZiggam9iLT5lcnJvcigpICkKKyAgICBpZiggam9iLT5lcnJvcigpICAmJiBq b2ItPmVycm9yKCkgIT0gS0lPOjpFUlJfRklMRV9BTFJFQURZX0VYSVNUICkKICAgICB7CiAgICAg ICAgIC8vVE9ETzogcHJvcGVyIGVycm9yIGhhbmRsaW5nCiAgICAgICAgIHdhcm5pbmcoKSA8PCAi QW4gZXJyb3Igb2NjdXJyZWQgd2hlbiBjb3B5aW5nIGEgZmlsZTogIiA8PCBqb2ItPmVycm9yU3Ry aW5nKCk7CkBAIC03MDEsNiArNzAxLDE1IEBACiAgICAgZW1pdCBpbmZvTWVzc2FnZSggdGhpcywg bWVzc2FnZSApOwogfQogCit2b2lkIFRyYW5zZmVySm9iOjpzbG90UmVzdWx0KCBLSm9iICpqb2Ig KQoreworICAgIC8vIFdoZW4gY29weWluZyB3aXRob3V0IG92ZXJ3cml0aW5nIHNvbWUgZmlsZXMg bWlnaHQgYWxyZWFkeSBiZQorICAgIC8vIHRoZXJlIGFuZCBpdCBpcyBub3QgYSByZWFzb24gZm9y IHN0b3BwaW5nIGVudGlyZSB0cmFuc2Zlci4KKyAgICBpZiAoIGpvYi0+ZXJyb3IoKSA9PSBLSU86 OkVSUl9GSUxFX0FMUkVBRFlfRVhJU1QgKQorICAgICAgICByZW1vdmVTdWJqb2IoIGpvYiApOwor ICAgIGVsc2UKKyAgICAgICAgS0NvbXBvc2l0ZUpvYjo6c2xvdFJlc3VsdCggam9iICk7Cit9CiAK IHZvaWQgVHJhbnNmZXJKb2I6OnN0YXJ0KCkKIHsKT25seSBpbiBhbWFyb2stMi42LjAvc3JjL2Nv cmUtaW1wbC9jb2xsZWN0aW9ucy9kYi9zcWwvOiBTcWxDb2xsZWN0aW9uTG9jYXRpb24uY3BwLnNr aXAtZXhpc3RpbmcKZGlmZiAtcnUgYW1hcm9rLTIuNi4wLW9yaWcvc3JjL2NvcmUtaW1wbC9jb2xs ZWN0aW9ucy9kYi9zcWwvU3FsQ29sbGVjdGlvbkxvY2F0aW9uLmggYW1hcm9rLTIuNi4wL3NyYy9j b3JlLWltcGwvY29sbGVjdGlvbnMvZGIvc3FsL1NxbENvbGxlY3Rpb25Mb2NhdGlvbi5oCi0tLSBh bWFyb2stMi42LjAtb3JpZy9zcmMvY29yZS1pbXBsL2NvbGxlY3Rpb25zL2RiL3NxbC9TcWxDb2xs ZWN0aW9uTG9jYXRpb24uaAkyMDEyLTA4LTExIDAyOjA5OjM2LjAwMDAwMDAwMCAtMDcwMAorKysg YW1hcm9rLTIuNi4wL3NyYy9jb3JlLWltcGwvY29sbGVjdGlvbnMvZGIvc3FsL1NxbENvbGxlY3Rp b25Mb2NhdGlvbi5oCTIwMTItMTItMTkgMjM6NDM6NDAuNTg5NzIyODEwIC0wODAwCkBAIC01OSw2 ICs1OSw3IEBACiAgICAgICAgICAqLwogICAgICAgICB2b2lkIHNsb3RKb2JGaW5pc2hlZCggS0pv YiAqam9iICk7CiAgICAgcHJvdGVjdGVkIHNsb3RzOgorICAgICAgICB2b2lkIHNsb3RSZXN1bHQo IEtKb2IgKmpvYiApOwogICAgICAgICB2b2lkIGRvV29yaygpOwogICAgICAgICB2b2lkIHByb3Bh Z2F0ZVByb2Nlc3NlZEFtb3VudCggS0pvYiAqam9iLCBLSm9iOjpVbml0IHVuaXQsIHF1bG9uZ2xv bmcgYW1vdW50KTsKICAgICBwcm90ZWN0ZWQ6Cg==