Skip to content

Makes tar-fs able to pack files that are actively being written to. - #88

Merged
mafintosh merged 3 commits into
mafintosh:masterfrom
shardus:master
May 11, 2020
Merged

Makes tar-fs able to pack files that are actively being written to.#88
mafintosh merged 3 commits into
mafintosh:masterfrom
shardus:master

Conversation

@asyed94

@asyed94 asyed94 commented Aug 22, 2019

Copy link
Copy Markdown
Contributor

Pretty simple change that allows tar-fs to be robust in more conditions, while maintaining its expected behavior.

@mafintosh

Copy link
Copy Markdown
Owner

Can you explain a bit more what this fixes?

Comment thread index.js
if (!entry) return

var rs = mapStream(xfs.createReadStream(path.join(cwd, filename)), header)
var rs = mapStream(xfs.createReadStream(path.join(cwd, filename), { start: 0, end: header.size > 0 ? header.size - 1 : header.size }), header)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I remember what this does now (it's been a while).

tar.pack('some_dir') works by getting a file from some_dir, creating a ReadStream for that file, then piping the ReadStream to a tar entry, which triggers this whole process for the next file in some_dir upon completion of the pipe, until all files from some_dir are packed.

However, if a file in some_dir is continuously being written to (e.g. a log file), that file has no EOF, and piping it's ReadStream to an entry will never complete, holding up the whole packing process and causing the tar file for some_dir to never be made.

This change adds start and end parameters to the ReadStream of a file based on the files size at that time. Adding these parameters ensures that piping the ReadStream to an entry will reach completion even if the file has no EOF, allowing the tar packing process to continue.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense

@mafintosh

Copy link
Copy Markdown
Owner

Thanks for explaining this. If you can remove the lock file then this is good to go! :)

@asyed94

asyed94 commented May 11, 2020

Copy link
Copy Markdown
Contributor Author

Went ahead and removed the package-lock.json. I think that got in by accident 😅

@mafintosh
mafintosh merged commit 462dd91 into mafintosh:master May 11, 2020
@mafintosh

Copy link
Copy Markdown
Owner

2.1.0

@mafintosh

Copy link
Copy Markdown
Owner

Thanks for the contribution! Appreciate it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants