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.
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.
(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).
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 :(
I'd be willing to help get this posted to reviewboard over the holidays, it does seem like an obvious improvement.
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/
(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.
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