Download folders in .tar.gz format

[ghost]

This is my WIP for solving #2 (only .tar.gz at the moment).

The code isn't quite clean, the error handling needs some more work, and I have a bug on Mac OS X because user folders are wierd symlinks, and it causes the current code to fail. I'm investigating this issue.

Otherwise, it works pretty well :)

[ghost]

So, I had to disable symlinks following strategy in the TarBuilder object because of these issues:

• append_dir_all appears to be working, but is returning an error alexcrichton/tar-rs#147
• Crashes when directory contains symlink to a directory. (When append_dir_all.) alexcrichton/tar-rs#174

I filed a PR for this issue, now let's see what happens :)

[svenstaro]

I just had a super cursory view and it kind of appears like you're rebuilding pretty-env-logger. I think we should use standard Rust logging.

[ghost]

2723bab: the TAR content is now being built in a buffer, so I removed the overhead of creating a temporary TAR file, then re-read its content.

17ef61a: I switched to log. I set the level to Error if not in verbose mode because I think it's still important for a user to know what happened when something fails. I removed useless colors, as it started to look like a rainbow.

Also, I changed the "Info: miniserve has been invoked [...]" to an orange notice (kind of a warning, but not on the log stream), because I think it can be a security issue in some cases, so better really warn the user. I even thought about putting the whole message in orange, I don't know.

Also, my PR (alexcrichton/tar-rs#187) is going good, I have to add some comments and we should be fine, then we can wait for the new release of tar-rs, and eventually we'll be able to set the follow_symlink option to true (false if -P) in archive.rs 👍

Finally, what do you think about limiting the scope of this PR to tar.gz ? zip will follow, eventually, but that's already a very big chunk of changes... :)

[svenstaro]

Yes, it's pretty big already! Let's keep it .tar.gz.

[svenstaro]

It's amazing that you're fixing this issue and the two over at tar-rs in one go! Keep it up.

…

On Thu, Mar 14, 2019, 18:56 Boastful Squirrel ***@***.***> wrote: Great! I'll keep you posted when it comes to the fix of tar-rs. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#48 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAGgApLnYZmOvKUwYyKtBkp24-ZFkhKks5vWjjkgaJpZM4bmKAA> .

[svenstaro]

What I generally noticed is that the transfer apparently doesn't start before the whole folder is tarred. Can we start it while it's working? The current implementation makes usage on really large folders quite impossible.

[svenstaro]

The web server should start sending bytes as soon as we give it bytes but I don't think we give it bytes early enough. I'm pretty sure it should work. I'm unsure that what you're doing right now is even supposed to work: Setting the Content-Length and chunked at the same time. See https://gist.github.com/CMCDragonkai/6bfade6431e9ffb7fe88.

Basically, while we're archiving, we have no idea how long the entire file is going to be but this behavior is already supported by all browsers, thankfully.

[svenstaro]

I never said it was gonna be easy but in the end I think the payoff is going to be worth it since it will be a lot more useful for sharing large directories! :)

[ghost]

I love challenges ;)

[svenstaro]

Keep at it, you've done a marvelous job so far!

[ghost]

I fear the tar-rs crate won't let us stream its result... All methods seem blocking, I'll dig deeper in the doc

Ah, alexcrichton/tar-rs#149

[svenstaro]

Ok that really is a bummer. Tell you what, I'll merge this as is without async but I really kinda want async in tbe long run because it does enable sending really large directories which currently is impossible.

[ghost]

I agree with you. Maybe we should add a little mention in the README, so people don't start trying to share huge folders.

Anyway, if we're gonna implement another compression algorithm so the user can chose, let's chose a crate that supports streaming :p

[svenstaro]

That seems like band-aid for what should really be an implementation detail of tar-rs. As I said, we'll need to fix this properly in tar-rs if anything. It shouldn't be that hard.

[svenstaro]

Please take a look at the remaining open comments.

[svenstaro]

Can you just #48 (comment) a bit of text and that's that?

[svenstaro]

Ok, back from vacation and merging! Stellar work. Hopefully we can make this async at some point.