search

multiSnow / mcomix3

End of Fork
Other
97 stars 39 forks source link

Bookmarks getting deleted #98

Closed kini closed 4 years ago

kini commented 4 years ago

Debian switched from upstream mcomix to this fork last month, and when I upgraded and opened the application, it deleted all my bookmarks. It seems the bookmarks are stored in ~/.local/share/mcomix/bookmarks.pickle. I restored this file from backup and tried to start the application from the terminal to see if there were any error messages. I got this:

$ mcomix
12:57:00 [MainThread] ERROR: ! Could not parse bookmarks file /home/kkini/.local/share/mcomix/bookmarks.pickle

When I closed the application again, the bookmarks.pickle file had been reset to a 30-byte presumably empty file.

From what I know of the pickle library in Python, it is not intended to be used as a portable data file format, so it's not surprising that Python 3 would be unable to load a pickle file created by a Python 2 app (the upstream mcomix).

How can I migrate my bookmarks from the old mcomix?

emfox commented 4 years ago

hello, this bug affect user data so mark as RC(release critical) in Debian, so will make mcomix removed from debian if I don't fix it, but I'd like to make the fix consistant with upstream, so any idea of the pull request?

multiSnow commented 4 years ago

This issue is marked as wontfix. The old bookmarks.pickle will be never loaded nor removed. I decided to use json instead of pickle, which should be compatible in any version of python in the future.

kini commented 4 years ago

After the change in #100, the old bookmarks.pickle will never be loaded or removed, but from the user's perspective it still looks like their bookmarks are gone, and the program offers no functionality for them to retrieve their old bookmarks from the pickle. Unless they are programmers, it means their old bookmarks became unusable. So this seems almost as bad as deleting the user's bookmarks, to me. Basically it means that mcomix3 is backwards-incompatible with the Python 2 upstream version.

Can you please add a code path in bookmark_backend.py that will read bookmarks.pickle if it exists and migrate it to the new json format?

multiSnow commented 4 years ago

After the change in #100, the old bookmarks.pickle will never be loaded or removed, but from the user's perspective it still looks like their bookmarks are gone, and the program offers no functionality for them to retrieve their old bookmarks from the pickle. Unless they are programmers, it means their old bookmarks became unusable. So this seems almost as bad as deleting the user's bookmarks, to me. Basically it means that mcomix3 is backwards-incompatible with the Python 2 upstream version.

Can you please add a code path in bookmark_backend.py that will read bookmarks.pickle if it exists and migrate it to the new json format?

Would #103 satisfy you?

kini commented 4 years ago

Thanks, this solves the problem for Debian Stable users who are still using upstream mcomix version 1.2.1.

But users of Debian Testing or Debian Unstable have already been using mcomix3 for some time. Their bookmarks are already deleted, but they may restore them from backups (like I did). So how can they use those backup bookmarks now? With #103, mcomix3 will not upgrade their bookmarks because the preferences file already has 'external commands' in it. Is there any way to help these users?

My thought was to always check for the existence of the .pickle files on startup and upgrade them and move to .pickle.bak if they are found. What do you think?

multiSnow commented 4 years ago

103 has been updated. the two .pickles is always be checked.

kini commented 4 years ago

Great, thanks.

I just realized that one more possibility is that the user already has new json bookmarks (because they were using mcomix3 for some time) and then they try to restore their old pickle bookmarks. In such a case, I think the current code in #103 will overwrite the json bookmarks, right? Instead, could you combine the old and new bookmarks?

multiSnow commented 4 years ago

The json commit is not yet merged into debian. It's only one week after that commit and user should take the risk of using developing code into account. .json is a text format, which is far more easier to edited. I will make these .jsons human-readable, and no more coding for old version compatible.

kini commented 4 years ago

OK, sounds fine to me. Thank you!