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

Add more supports for tar extensions #48

Merged
merged 5 commits into from
Aug 13, 2019
Merged

Add more supports for tar extensions #48

merged 5 commits into from
Aug 13, 2019

Conversation

subosito
Copy link
Contributor

Currently, extractTar only support for .tar.gz, this PR remove flag z on tar command to let it extract other extensions, like tar.xz.

Use case: I need it to create actions for Flutter, which flutter sdk has .tar.xz extension for its Linux distribution, like https://storage.googleapis.com/flutter_infra/releases/stable/linux/flutter_linux_v1.7.8+hotfix.4-stable.tar.xz.

@subosito subosito mentioned this pull request Aug 13, 2019
Copy link
Contributor

@damccorm damccorm left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! I left some comments, but overall I like the idea behind the change

packages/tool-cache/src/tool-cache.ts Outdated Show resolved Hide resolved
packages/tool-cache/__tests__/tool-cache.test.ts Outdated Show resolved Hide resolved
packages/tool-cache/__tests__/tool-cache.test.ts Outdated Show resolved Hide resolved
packages/tool-cache/__tests__/tool-cache.test.ts Outdated Show resolved Hide resolved
packages/tool-cache/__tests__/tool-cache.test.ts Outdated Show resolved Hide resolved
expect(fs.existsSync(`${toolPath}.complete`)).toBeTruthy()
expect(fs.existsSync(path.join(toolPath, 'file.txt'))).toBeTruthy()
expect(
fs.existsSync(path.join(toolPath, 'file-with-ç-character.txt'))
Copy link
Contributor

Choose a reason for hiding this comment

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

This shouldn't be an issue, but could you make the contents of the tar.gz and the tar.xz different? That way if somehow they got unzipped to the same location (which would be a different breakage that we should catch) we'll still see the error. Sorry if I'm being a little nitpicky here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated the tar.xz to be different with tar.gz and added file content checks on it.

packages/tool-cache/__tests__/tool-cache.test.ts Outdated Show resolved Hide resolved
@damccorm
Copy link
Contributor

Looks good to me, thanks for working with me on both PRs!

@damccorm damccorm merged commit 2c3e55b into actions:master Aug 13, 2019
@subosito
Copy link
Contributor Author

Awesome 👍

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

2 participants