AppImageCommunity / AppImageUpdate

AppImageUpdate lets you update AppImages in a decentral way using information embedded in the AppImage itself.
https://appimage.org
MIT License
571 stars 57 forks source link

[rewrite] Do not leave .zs-old files around #14

Open probonopd opened 6 years ago

probonopd commented 6 years ago

When the file was already 100% downloaded and up to date and the user then tries to update it, a .zs-old file is left behind:

me@host:~$ ls
appimaged-x86_64.AppImage     Desktop    Downloads  Pictures
appimaged-x86_64.AppImage.zs-old  Documents  Music  Videos
me@host:~$ rm appimaged-x86_64.AppImage.zs-old 
me@host:~$ ls
appimaged-x86_64.AppImage  Desktop  Documents  Downloads  Music  Pictures  Videos
me@host:~$ Downloads/appimageupdate-x86_64.AppImage appimaged-x86_64.AppImage
Starting update...
Updating from generic server via ZSync
Update URL: https://github.com/AppImage/AppImageKit/releases/download/continuous/appimaged-x86_64.AppImage.zsync
zsync2: appimaged-x86_64.AppImage found, using as seed file
zsync2: Reading seed file: appimaged-x86_64.AppImage
zsync2: Usable data from seed files: 100.000000%
zsync2: Renaming temp file
zsync2: Fetching remaining blocks
zsync2: Verifying downloaded file
zsync2: checksum matches OK
zsync2: used 245760 local, fetched 0
100% done...
Update successful!
me@host:~$ ls
appimaged-x86_64.AppImage     Desktop    Downloads  Pictures
appimaged-x86_64.AppImage.zs-old  Documents  Music  Videos
TheAssassin commented 6 years ago

Rewrite does cleanup its backup files as of https://github.com/AppImage/AppImageUpdate/commit/4b1e3a3e7b50128e47af55cb2bf99d2c6fa061b7.

probonopd commented 6 years ago

Still an issue if you already have the new version downloaded. https://github.com/AppImage/AppImageUpdate/issues/36

TheAssassin commented 6 years ago

That issue originates from your idea to keep the old files and create a new one with the new filename. When there's a file called like the new filename, it will be backed up to avoid deleting it.

A "fix" for this would probably be to make the update check look whether there's such a new file, and whether it is equal to the server file. This avoids unnecessary updates.

probonopd commented 6 years ago

Just having two identical filenames does not mean that the content is necessarily the same. If your proposal takes this into account, then I am all for it.

TheAssassin commented 6 years ago

The idea is to apply the selected update check to this file, too, yes.

TheAssassin commented 6 years ago

This has probably been resolved since AppImageUpdate checks for updates before starting an actual update now. Please confirm.

probonopd commented 6 years ago

Did not encounter it recently, hence closing. Should reopen if the problem appears again. Thanks.

probonopd commented 5 years ago

AppImageUpdate-382-b61bee3-x86_64.AppImage still leaves around .zs-old files.

Let's do some check before exiting to clean up any remaining .zs-old files.

TheAssassin commented 5 years ago

That issue originates from your idea to keep the old files and create a new one with the new filename. When there's a file called like the new filename, it will be backed up to avoid deleting it.

probonopd commented 5 years ago

In the end, please let's make sure that no zs-old files are around anymore in any circumstance. Whatever happens.

TheAssassin commented 5 years ago

i don't intend to delete files with the same filename during updates. Tell the upstream to either do proper versioning and put the version number in the new file's filename.

Quoting you:

Just having two identical filenames does not mean that the content is necessarily the same. If your proposal takes this into account, then I am all for it.

probonopd commented 5 years ago

I have the following use case quite frequently:

Same application version, newer AppImage packaging.

probonopd commented 5 years ago

zs-old screams "error", "bug" at me.

TheAssassin commented 5 years ago

That's nothing we could fix. .zs-old just tells "previous file", not "error".

probonopd commented 5 years ago

Why can't it then just append ".old" to the original filename, or something like that. I really dislike that "zs" thing. Our users don't know (and should not have to know) anything about "zs".

TheAssassin commented 5 years ago

Because that extension is coming from zsync2, which retains compatibility with zsync.

probonopd commented 5 years ago

I know, but AppImageUpdate could should "catch" these files imho.

probonopd commented 5 years ago

By the way:

i don't intend to delete files with the same filename during updates. Tell the upstream to either do proper versioning and put the version number in the new file's filename.

We should think about this.

Just having two identical filenames does not mean that the content is necessarily the same.

That is correct.

But what is the intended behavior when running AppImageUpdate?

In my opinion, if everything goes well, there should be a new file with the new filename as specified by the author of the zsnyc file. If that is a new filename (e.g., with a new version number) then this file should be there in addition to the original file.

But if this is not a new filename (but rather an existing filename), shouldn't we overwrite the existing file with the new file (after we know the update was successful) and not leave an old file around? After all that seems to be what was wanted by the zsync file author.

Not 100% on what's the best thing to do here.

TheAssassin commented 5 years ago

I know, but AppImageUpdate could should "catch" these files imho.

How?

But if this is not a new filename (but rather an existing filename), shouldn't we overwrite the existing file with the new file (after we know the update was successful) and not leave an old file around?

Back when we developed this, you were a fan of keeping the old files around. If the new version doesn't work, you can still use the old AppImage...

After all that seems to be what was wanted by the zsync file author.

Not necessarily. And I don't think they should get that much control.

probonopd commented 5 years ago

Back when we developed this, you were a fan of keeping the old files around. If the new version doesn't work, you can still use the old AppImage...

True. I was thinking about files with a new name though.

Not 100% on what's the best thing to do here.

Still holds ;-)

TheAssassin commented 5 years ago

I don't think this is really an issue. The file contains old, and will be created after an update. As a user, I'd quickly recognize "oh, it's a backup".

probonopd commented 5 years ago

This is what https://github.com/antony-jr/AppImageUpdaterBridge does and I think it's not the worst idea: