I really don't like it: I had to cheat both in put and copy.
I had problems with dataReq() and readData() - the put method stalls waiting for an empty(?) buffer.
Details
Details
- Reviewers
gengisdave - Group Reviewers
Krusader - Maniphest Tasks
- T3479: Krusader freezes when renaming a file that is inside an archive
- Commits
- R167:fb60eda72097: Rename files inside archives
Atm, the original file is not yet deleted. Temporary files are not deleted too. More code checks needed. Everything must be tested. I'm open to any other solution.
Diff Detail
Diff Detail
- Repository
- R167 Krusader
- Lint
Lint Skipped - Unit
Unit Tests Skipped
Comment Actions
Davide, thanks a lot for taking over this issue. Thanks to this I've finally started to understand things a bit better. I used your code to elaborate more (see attached diff
). Here is what I changed in your diff:- commented codec check in copy function (I still don't understand what is that for; everything seems to work so far)
- made krarcCopy and krarcPut functions to return boolean success value, than used it in rename
- krarcPut can take additional putFileUrl parameter to handle it, I think it is better understandable then setting realPut=true/false field, but that is for You to decide :-)
- delete temporary file in the end of rename
Known Issues:
- probably more checks missing (handling error state of del(...))
- renaming folder result in empty renamed folder
- definitely needs more testing
Notes:
I've also had problems with dataReq() and readData(). I think it needs to be handled by another "file" ioslave in this case to provide data of the temporary local file. I really don't know how to do it. But since we are creating the temp file directly, I think it is quite fine approach to "cheat" in put() like that. But I'm no expert of course.
Comment Actions
I forgot non-empty folders. My solution can't be useful anymore. So I turned again on zipnote and looking to a possibile method with other types of archive, I've found that newer versions of 7za can rename files inside archives (both .zip and .7z).
I will have a look to the diffusion of p7zip in distros, but I think we can use it.
In that case, the actual can be discarded, and the solutions is to implement a renCmd in kio_krarcProtocol::initArcParameters (plus a ton of sanity checks).
Comment Actions
Understood. I'll start to work on that, but feel free to take over anytime, because I'm not sure how much time I'll have this week.
Comment Actions
This is the first attempt to use 7za to rename a file inside an archive. It works, but we have to do more checks.
Comment Actions
Works perfectly! Thanks. I wanted to update your diff with one line but I was unable to do that since this revision is not created by me. Anyway, we need to add a line:
renCmd << cmd << "rn";
...in initArcParameters() method somewhere under this conditional:
} else if (arcType == "7z") {...to support 7z archives, too.
And so far I wasn't able to make the rename process stop when renaming is not supported (e.g. in non zip/7z archives). When renaming such archive, the slave will try to rename, then copy & del and then put where it hangs... Do You know of any way we can notify the slave to give up for good?
Comment Actions
I tried only on zip files, but I have to check all kind of archives supported by 7za. To stop the kio, I use error(ERR_CANNOT_RENAME). This doesn't permit KIO to switch to copy+del or get+put.
Comment Actions
Thanks Davide, that (ERR_CANNOT_RENAME) works perfectly. I'll try to upload new diff with more checks.
Comment Actions
I just added:
- checks if rename command is not empty and if exit code is a success
- support for 7z archives
I've also checked that 7z renaming does not work for .tar.* archives. Rest of archive types are still to be checked.
Comment Actions
Tested another archives like deb, rpm and rar but none of them is write-supported by 7-zip. In fact 7-zip websites claim only unpack support for the rest of proprietary archive types I didn't test.
So renaming only works on zip-like and 7zip archives. Attempt to rename inside unsupported package (or if 7za binary is missing) leads to an error message. I feel the code is ready to be merged and if problems arise, I'll fix them:).