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

Attempt to make tar work on Windows #11

Closed
wants to merge 3 commits into from

Conversation

Fydon
Copy link
Contributor

@Fydon Fydon commented Feb 13, 2017

The tar commandline requires changes to the path for it to work on Windows. As a result I moved CompressSpecificFilesInDir and DecompressFileToDir into separate Windows and non Windows build files. I also split the tests into separate Windows and non Windows build files, so that they will be able to run build specific tests. All the tests in fileutil are passing on Windows.

I'm still new to Go, so please let me know how I can improve this.

As tar isn't available on Windows by default, so #9 is still advised, but these changes should enable the version of tar that comes with Git to work.

@cfdreddbot
Copy link

Hey Fydon!

Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you and the commit authors have already signed the CLA.

@charlievieth
Copy link
Contributor

charlievieth commented Feb 16, 2017

@Fydon Thanks for the PR. This isn't documented (and probably should be), but we added Windows support to bosh-utils in order to support the bosh-agent, which includes a BSD tar compiled for Windows (note: you will also need zlib.dll). I believe the source comes from libarchive - we have a story to start building tar in our CI.

We do not use the tar bundled with Git as it does not work with Windows file paths and the Unix paths it accepts are not valid Windows paths. It breaks interoperability with the Windows API and Go's standard library. For example:

Path Windows API BSD Tar Git Tar
/Windows/Temp/foo valid valid invalid
/c/Windows/Temp/foo invalid invalid valid
C:\Windows\Temp\foo valid valid invalid

We cannot support one without breaking the other and supporting both would require deducing the version/flavor of tar at runtime, which is not viable.

I think the best solution is for us to improve the documentation around tar on Windows, build tar in our CI and make that tar publicly available (and pointed to in the bosh-utils and bosh-agent docs).

Edit: This also break cygwin tar.

@charlievieth charlievieth mentioned this pull request Feb 16, 2017
@charlievieth
Copy link
Contributor

charlievieth commented Feb 16, 2017

Closing as this PR makes bosh-utils dependent on the tar installed with git-bash and breaks compatibility with cygwin-tar and libarchive-tar (which we use and correctly handles Windows paths).

We created a story to document tar on Windows: 139998715.

@cppforlife
Copy link
Contributor

cppforlife commented Feb 16, 2017 via email

@charlievieth
Copy link
Contributor

Didn't realize this was related to the bosh-cli (it's mentioned in #9, but not here).

I would rather not re-open this PR as it would make the bosh-agent and bosh-cli dependent on the tar bundled with git-bash (GPL-2.0). This would also likely break the bosh-cli for those using the popular cygwin terminal. Maybe #9 or the bosh-cli is a better place for this issue.

Using Go's archive/tar lib would be great, but the lib is spartan. Maybe we could explore bundling the bsdtar provided by libarchive with the bosh-cli or bosh-utils.

@cppforlife cppforlife reopened this Feb 17, 2017
@cfdreddbot
Copy link

Hey Fydon!

Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you and the commit authors have already signed the CLA.

@Fydon
Copy link
Contributor Author

Fydon commented Feb 23, 2017

How about using archiver?

@jaresty
Copy link
Contributor

jaresty commented Nov 17, 2017

Merge story #152974592

@bosh-admin-bot
Copy link

This pull request was marked as Stale because it has been open for 21 days without any activity. If no activity takes place in the coming 7 days it will automatically be close. To prevent this from happening remove the Stale label or comment below.

@bosh-admin-bot
Copy link

This pull request was closed because it has been labeled Stale for 7 days without subsequent activity. Feel free to re-open this pull request at any time by commenting below.

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

Successfully merging this pull request may close these issues.

6 participants