AppImageUpdate icon indicating copy to clipboard operation
AppImageUpdate copied to clipboard

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

Open probonopd opened this issue 8 years ago • 22 comments

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

probonopd avatar Oct 25 '17 15:10 probonopd

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

TheAssassin avatar Oct 28 '17 03:10 TheAssassin

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

probonopd avatar Nov 12 '17 06:11 probonopd

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.

TheAssassin avatar Nov 14 '17 23:11 TheAssassin

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 avatar Nov 15 '17 00:11 probonopd

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

TheAssassin avatar Nov 15 '17 00:11 TheAssassin

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

TheAssassin avatar Feb 22 '18 22:02 TheAssassin

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

probonopd avatar Feb 23 '18 18:02 probonopd

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.

probonopd avatar Nov 11 '18 10:11 probonopd

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.

TheAssassin avatar Nov 11 '18 11:11 TheAssassin

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

probonopd avatar Nov 11 '18 11:11 probonopd

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.

TheAssassin avatar Nov 11 '18 11:11 TheAssassin

I have the following use case quite frequently:

Same application version, newer AppImage packaging.

probonopd avatar Nov 11 '18 11:11 probonopd

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

probonopd avatar Nov 11 '18 11:11 probonopd

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

TheAssassin avatar Nov 11 '18 11:11 TheAssassin

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".

probonopd avatar Nov 11 '18 11:11 probonopd

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

TheAssassin avatar Nov 11 '18 12:11 TheAssassin

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

probonopd avatar Nov 11 '18 12:11 probonopd

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.

probonopd avatar Nov 11 '18 12:11 probonopd

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.

TheAssassin avatar Nov 11 '18 12:11 TheAssassin

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 ;-)

probonopd avatar Nov 11 '18 12:11 probonopd

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".

TheAssassin avatar Nov 11 '18 12:11 TheAssassin

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

probonopd avatar Feb 26 '19 20:02 probonopd