Skip to content

[Build System: build-script] Remove the tar module from swift_build_support. #29468

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

Merged
merged 1 commit into from
Jan 27, 2020

Conversation

Rostepher
Copy link
Contributor

The single function it provided to archive a source diretory has been moved to build-script which was the only caller.

…upport.

The single function it provided to archive a source diretory has been moved to build-script which was the only caller.
@Rostepher Rostepher requested review from compnerd and edymtt January 27, 2020 07:36
@Rostepher
Copy link
Contributor Author

@swift-ci please smoke test

Copy link
Member

@compnerd compnerd left a comment

Choose a reason for hiding this comment

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

I'm fine with the shuffling, not the original code. I think that we should get this merged as-is and then follow it up with a replacement with tarfile.

@Rostepher
Copy link
Contributor Author

@compnerd I just wanted to remove this unnecessary module while I'm transitioning the important functionality from swift_build_support to build_swift. I'm definitely in favor of using a Python standard than rolling our own wrapper around the tar CLI.

@Rostepher Rostepher merged commit fecaca5 into swiftlang:master Jan 27, 2020
@Rostepher Rostepher deleted the remove-tar-module branch January 27, 2020 17:41
'destination' path.
"""
# We do not use `tarfile` here because:
# - We wish to support LZMA2 compression while also supporting Python 2.7.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pretty sure we've never used this for LZMA2 compression, thus we should just use the builtin tarfile library. I'll do that in a follow-up PR.

Copy link
Member

Choose a reason for hiding this comment

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

The builtin tarfile module supports lzma as well I think

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only for Python 3.2 (or 3.3) and beyond. Python 2.7 supports gzip however, which appears to be all we use for the time being.

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