csirac2 / snazzer

btrfs snapshotting and backup system offering snapshot measurement, transport and pruning.
BSD 2-Clause "Simplified" License
89 stars 9 forks source link

Added a --progress option for snazzer-receive. #40

Open DL6AKU opened 7 years ago

DL6AKU commented 7 years ago

Fixes #39

florianjacob commented 7 years ago

Good idea!

I'm thinking about whether the explicit command line option is really needed though - maybe we could turn progress metering automatically if snazzer is run interactively, i.e. stderr is a tty with a user in front of it? In fish there's isatty but I don't know how to do that reliably in bash. :wink:

florianjacob commented 7 years ago

Seems like checking that when run in a terminal via ssh is not that trivial in bash, though…

DL6AKU commented 7 years ago

We could show the progress by default if pv is installed. That would actually make for a simpler patch. I did not want to go so far though with my patch. Such a default behaviour would have to be decided by the project maintainer I guess (@csirac2).

csirac2 commented 7 years ago

Great idea! I would love to see receive progress indicator by default, ssh does look fiddly to detect though.

My only concern with the code is that I would like to avoid eval.

DL6AKU commented 7 years ago

Afaik it wouldn't hurt ssh if it was enabled for ssh as well (so we skip the ssh detection), though I did not look into the ssh option just now.

I do not know about bash magic enough to avoid the eval. I tried without, but then you get a double pipe or pv: | not found errors. Or you get more nested ifs.

I can look into expansion of the patch tomorrow.

florianjacob commented 7 years ago

To be more explicit: If possible, the progress meter should show automatically in three cases:

It shouldn't show if snazzer is started from inside a shell script, a cron job or a systemd timer, or when the stderr is redirected to a file or something else that is not a tty.

References: http://stackoverflow.com/questions/911168/how-to-detect-if-my-shell-script-is-running-through-a-pipe Maybe helpful: http://tldp.org/LDP/abs/html/intandnonint.html

florianjacob commented 7 years ago

Regarding the conditional pipeline issue, this looks like a solution that does not need eval or another if level: http://unix.stackexchange.com/questions/38310/conditional-pipeline

csirac2 commented 7 years ago

I'm not sure I'll get it to it this weekend, and I'm in a crunch at work for the next week, but I'll try to take a look next weekend.

DL6AKU commented 7 years ago

Just FYI, I am no no longer using snazzer, so I won't be actively working towards a patch.