search

AppImageCommunity / zsync2

Rewrite of https://github.com/AppImage/zsync-curl, using modern C++, providing both a library and standalone tools.
Other
132 stars 25 forks source link

Parity with original zsync: "-i" flag prints an error and stops if the input file doesn't exist locally; Original zsync keeps going in this case #36

Open DeeDeeG opened 5 years ago

DeeDeeG commented 5 years ago

Steps to reproduce:

Expected:

zsync2 can ignore the fact that the input file is missing and try to download anyway.

Actual:

This error message shows: Failed to open file [path-to-file/filename.extension] and zsync2 exits without downloading the file.

Note: original zsync handles this by basically ignoring the missing input, and tries to download the file anyway. So by changing zsync2's behavior, it could more-closely match the original zsync's behavior.

More details (click to expand!) Command I ran: ```sh zsync2 $URL -k ~/.zsyncs/$ZSYNCFILENAME -i ~/Downloads/$OUTPUTFILENAME -o ~/Downloads/$OUTPUTFILENAME ``` effectively: ```sh zsync2 http://cdimage.ubuntu.com/ubuntu-mate/daily-live/pending/disco-desktop-amd64.iso.zsync -k ~/.zsyncs/disco-desktop-amd64.iso.zsync -i ~/Downloads/disco-desktop-amd64.iso -o ~/Downloads/disco-desktop-amd64.iso.desktop ``` It works the same if the URL for the remote `.zsync` file is at the end of the command... ```sh zsync2 -k ~/.zsyncs/disco-desktop-amd64.iso.zsync -i ~/Downloads/disco-desktop-amd64.iso -o ~/Downloads/disco-desktop-amd64.iso.desktop http://cdimage.ubuntu.com/ubuntu-mate/daily-live/pending/disco-desktop-amd64.iso.zsync ``` Here was the full command output: ```sh zsync2 version 2.0.0-alpha-1 (commit 7857ff1), build built on 2018-12-30 22:18:20 UTC Checking for changes... Cannot find file /home/[me]/Downloads/disco-desktop-amd64.iso, triggering full download Storing copy of .zsync file in /home/[me]/.zsyncs/disco-desktop-amd64.iso.zsync, as requested Target file: /home/[me]/Downloads/disco-desktop-amd64.iso Reading seed file: /home/[me]/Downloads/disco-desktop-amd64.iso Failed to open file /home/[me]/Downloads/disco-desktop-amd64.iso ```
TheAssassin commented 5 years ago

I don't like to introduce such IMO implicit behavior. I as a user would want the tool to notify me with an error if a file I think is there and would like to use as an input file does not exist. Otherwise, bandwidth usage will explode, which can't be afforded.

Do you really need this again? @probonopd what do you think? I'm not so much a fan of this idea... but I'm not strictly against it, but would show a big fat warning at least.

DeeDeeG commented 5 years ago

One of the reasons I found this project is that I had some problems with how the original zsync is set up. So I don't necessarily take issue with with deviating from that.

The workaround for this situation is very simple: Only specify a -o flag, no -i flag.

That said, this is one of the few areas where a script using original zsync can't switch to zsync2 as a drop-in replacement. Such a script (like mine) would need to be updated (only slightly) to fix this failure-to-download scenario. I like the notion of backward-compatibility, and it seems like a noble goal, but there are of course times where you might not want to imitate the original. I would leave it up to the maintainers here to decide.

TheAssassin commented 5 years ago

You are right. Thinking about this issue, we actually state that we want to be compatible, but that was over a year ago, and since I haven't really worked on this. I think we don't have a large user base anyway, mostly people using this tool through AppImageUpdate.

I think we should then show a warning instead of failing.