Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pack root path fix #37

Closed
wants to merge 1 commit into from
Closed

Pack root path fix #37

wants to merge 1 commit into from

Conversation

26medias
Copy link

The archive doesn't include the current directory, only its content.

When I archive the directory '/test', I only want its content in the archive, not a new directory '/test'.

The archive doesn't include the current directory, only its content.
@othiym23
Copy link
Contributor

What is the intent of this patch? And could you include a test to go with it? I'm having a little trouble figuring out what you want to happen from the description and patch.

@26medias
Copy link
Author

If you want to archive the content of a directory, then you only want to content in the archive, not the directory itself.

If I archive that directory with the current version:
fstream.Reader({ path: '/test', type: "Directory" }).pipe(tar.Pack({ noProprietary: true })).pipe(fs.createWriteStream('test.tar.gz'));

I get a tar.gz file that contains /test itself hen it should only archive the content of that directory.

I don't know if it's on purpose, but none of the other libs work in that way, and there doesn't seem to be any way to archive the content of a directory without including the directory itself in the code.

@26medias
Copy link
Author

I'm not going to write a test, there's a single line change. It's ok if you don't merge, I forked and using that modified version for my own use.

@isaacs isaacs closed this Jun 30, 2014
@isaacs
Copy link
Owner

isaacs commented Jun 30, 2014

I'm not opposed to the feature, but it must be done in a way that doesn't break existing semantics (presumably by keeping defaults as they are, and adding new API for the change), and must have a test.

@26medias
Copy link
Author

I understand, you can cancel the request.

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.

None yet

3 participants