robweber / xbmcbackup

Backup Addon for Kodi
MIT License
112 stars 48 forks source link

Kodi 19 Settings Bug - Backups to keep #186

Closed supafyn closed 3 years ago

supafyn commented 3 years ago

Before working on a new issue I'm probably going to ask you all this stuff anyway, probably just easier to provide it now.....

Describe the problem The issue I am seeing is that on Kodi 19, when setting the value of Backups to keep under Settings --> General, the value is being ignored and instead the default behavior of 0 to keep all backups is being used.

Could you take a look into this and see why regardless of what value is being placed in this setting the only thing the code is doing is keeping all backups?

Platform and Kodi version On android for nvidia shield using Kodi 19

robweber commented 3 years ago

Are you using the "browse path" or type path option in the settings. I did just notice a but where if you don't include the final directory slash when typing a path the listBackups function doesn't work correctly. I'll tag the fix to this issue in case it applies.

supafyn commented 3 years ago

Yes, I am using the browse path option and obviously in navigating the path you can't put a final slash in.

Looking at the settings file though I do see that the tag "remote_path" does have the final slash in it for the value, it just does not display when you are looking at it within the Settings dialog of the app.

Hope this helps answer your question and hopefully the fix queued up resolves this issue. Thanks for looking into this!

robweber commented 3 years ago

Thanks for the update. I'll do some more testing and see if the rotate functionality itself is having any issues I can recreate. If you notice it again please get a debug log. Sometimes there are messages from Kodi such as a "failure to remove directory XXX" that might indicate a file permissions issue.

supafyn commented 3 years ago

So I am attempting to get you a log file when doing a Backup with debugging turned on, however, every time I go to grab the log file from Kodi after performing the task, the log file is always blank.

I even have gone so far as completely uninstalling Kodi and re-installing it to see if I could grab any data in the log file and still the file is blank at least in Kodi 19

Since going to Kodi 19, are you able to grab a log file with content? If you have any suggestions on how I can grab a log file for you, please let me know.

Just a note, but I am using an NFS browse path when placing the files out to a shared folder and retrieving them. While for the most part the functionality is working, I do get some sort of error after the restore process is completed when using the Simple option and enabling all the features.

supafyn commented 3 years ago

Never mind about the log file issue as I had forgotten on this android device I had set the logger buffer size to zero and that is why no actual content was available in the log file. Making the appropriate adjustment to the buffer size and I now have a log file here I can provide you.

Here is the link: http://paste.kodi.tv/unemusedec

Also, did the Backup action twice in the log and quickly parsing it, I see the following Error being noted:

Remove - Error( rmdir call failed with "NFS: RMDIR of /Kodi/backups/202103071808 failed with NFS3ERR_NOTEMPTY(-39)" )

If there is anything additional I can do to help further troubleshoot this issue, please let me know. Thanks!

supafyn commented 3 years ago

Also, here is the error I was referring to in regards to restoring from a backup:

ERROR : EXCEPTION Thrown (PythonToCppException) : -->Python callback/script returned the following error<--

Here is the full log file link: http://paste.kodi.tv/amujehesup

robweber commented 3 years ago

Thanks for the log file - that helps. I see you're getting NFS3ERR_NOTEMPTY(-39). It appears the underlying issue is that when using NFS the rmdir() function fails as NFS won't recursively delete, but expects the directory to be empty. This give us only 2 real options:

  1. escalate this as a Kodi issue since the function call really is xbmcvfs.rmdir(). I kind of feel it is their responsibility to make sure the underlying file system calls work. That library should recursively removing the files.
  2. do another walk of the file system within the addon, deleting files up the tree.

I like 1 as it's cleaner but then we're stuck waiting for a Kodi release to get it fixed - if the issue is ever even fixed at that level. I'm going to look at the Kodi C++ code a bit and see if it was just an oversight.

Regarding your other issue. Are you restoring from a backup that was made with a different version of the addon? There was a change to the guisettings restore process so backups made with older versions may throw an error like that. Let me know if the archive it was made with something like 1.6.3 or newer.

supafyn commented 3 years ago

So for my restore issue that comes from a backup made with the latest version and in fact was made just 40 minutes or so prior to actually attempting that restore.

Went ahead and confirmed that everything is done using version 1.6.5

robweber commented 3 years ago

I've done some more experiments with this. I really think it's an issue with how Kodi is handling the rmdir() call in all file systems. Even on Windows I'm getting errors trying to remove a directory. In previous versions it would remove recursively but it appears that if the directory has files in it you get an error.

To test I wrote a loop that deletes the folder recursively within the addon (option 2 above). This worked. Backups are rotated properly. I'm going to check on the Kodi forum if this is intended behavior before pushing as a fix but it appears that is the issue.

robweber commented 3 years ago

Link to forum post just for record keeping: https://forum.kodi.tv/showthread.php?tid=361395

robweber commented 3 years ago

Based on a comment in the forums I added a force option to the rmdir() call. On my system this fixed things up with the backup rotation. If possible, can you test this on your end? If you can either just replace the vfs.py file or add the missing arg to the call based on the commit and test it. I'll push this to the repo but another confirmation it's the right fix would be great.

supafyn commented 3 years ago

I went ahead and replaced the vfs.py file on my nvidia shield and can confirm now that the backup process is working as expected with creating the new backup folder and once done displaying a message that it is removing the previous backup. Confirmed that on my NAS the entire previous backup folder and files were deleted as well.

Appreciate you taking the time to dig through this issue and getting this issue fixed. Let me know if there is any additional testing I should do.