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

fix: Use '/' path separator on Windows for tar archives #24

Merged
merged 3 commits into from Aug 2, 2023

Conversation

Infiltrator
Copy link
Contributor

@Infiltrator Infiltrator commented Mar 20, 2019

Creating a tar archive on Windows with a directory results instead in the file having '\' in its name, and on extraction on POSIX, does not create the appropriate directory.

To fix this, use path.posix to force the '/' path separator even if creating the archive on Windows.

closes #22

Creating a tar archive on Windows with a directory results instead in the file having '\\' in its name, and on extraction on POSIX, does not create the appropriate directory.

To fix this, use `path.posix` to force the '/' path separator even if creating the archive on Windows.
@Infiltrator
Copy link
Contributor Author

Fixes #22 .

@Infiltrator
Copy link
Contributor Author

Hmmm. How do I squash commits in the browser?

@zevero
Copy link

zevero commented Feb 5, 2021

Will this be merged?
Currently tgz created on windows are not readable on linux.

@fengmk2
Copy link
Member

fengmk2 commented Jul 11, 2022

@Infiltrator Can you add a test case for this bug fix?

@fengmk2 fengmk2 added the bug label Jul 11, 2022
@DesignByOnyx
Copy link

I know this issue is old, but I confirmed that this fixes issues on Windows 10. I am generating a tgz using:

compressing.tgz.compressDir(stagingDir, outputDest);

Tools like 7Zip are unable to correctly extract the folder structure. When I apply the changes in this PR, everything works as expected.

@fengmk2 fengmk2 changed the title Use '/' path separator on Windows for tar archives fix: Use '/' path separator on Windows for tar archives Aug 2, 2023
@codecov
Copy link

codecov bot commented Aug 2, 2023

Codecov Report

Merging #24 (72651bb) into master (290b7b3) will not change coverage.
Report is 1 commits behind head on master.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master      #24   +/-   ##
=======================================
  Coverage   96.64%   96.64%           
=======================================
  Files          19       19           
  Lines         597      597           
  Branches      112      112           
=======================================
  Hits          577      577           
  Misses         20       20           
Files Changed Coverage Δ
lib/tar/stream.js 96.51% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@fengmk2 fengmk2 merged commit 3aa065b into node-modules:master Aug 2, 2023
15 of 18 checks passed
fengmk2 pushed a commit that referenced this pull request Aug 2, 2023
[skip ci]

## [1.9.1](v1.9.0...v1.9.1) (2023-08-02)

### Bug Fixes

* Use '/' path separator on Windows for tar archives ([#24](#24)) ([3aa065b](3aa065b))
@github-actions
Copy link

github-actions bot commented Aug 2, 2023

🎉 This PR is included in version 1.9.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@DesignByOnyx
Copy link

Thanks @fengmk2 for pushing this along. I'm trying to create a test for this, but there are a couple issues which fall into the "not worth it" category for me:

  • The tests will need to run on Windows to make sure it truly passes.
  • The bug doesn't happen using this library alone - meaning, this tool can uncompress files that it created. In order to actually test this, we will need to run tar commands (if it exists) or 3rd party tools to uncompress the files.

None of this seems worth it for what appears to be a reasonable and simple fix. Thanks for getting this in!

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

Successfully merging this pull request may close these issues.

windows tgz and linux tar zxvf, path error
4 participants