-
-
Notifications
You must be signed in to change notification settings - Fork 390
Fix create archive to a continuous writing source file failed #388
Conversation
mholt#387 Signed-off-by: Yan Zhu <[email protected]>
c55f399
to
85ad2b9
Compare
Thanks! Why does this fix it, though? Wouldn't Copy() just copy the 2 GB and finish at EOF? Why do we need to limit the number of bytes? |
I suppose the problem is caused by writing a header with the original file size and the reader reading the new append content. https://github.com/mholt/archiver/blob/v4.0.0-alpha.8/tar.go#L74-L100
|
ping @mholt |
Thanks, sorry for the delay. Just had a baby. I just need to think about this one more time but I suppose it is likely to be merged soon 🙂 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I mean, I think I have no problems with this, but I also don't really know what the implications are for not your use case (which I don't fully understand still from code screenshots).
Could we add a comment maybe explaining why we're doing it this way?
Even better, tests to ensure correct behavior in your use case and the 'standard' use cases. I haven't really gotten around to doing a lot of testing in this package yet but if you want to make me even more confident in this change, then that'd be good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the improvement, we'll try it out
Fix #387