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.

Sign up for GitHub

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

Jump to bottom

Update win_copy for #32677 #32682

Merged
merged 2 commits into from
Nov 9, 2017
Merged

Update win_copy for #32677 #32682

merged 2 commits into from
Nov 9, 2017

Conversation

u625030
Copy link
Contributor

@u625030 u625030 commented Nov 8, 2017

SUMMARY

Fixes missing large file support in win_copy

Fixes: #32677

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

lib/ansible/plugins/action/win_copy.py

ANSIBLE VERSION

Stable-2.4

@u625030
Update win_copy for ansible#32677
5468a2c 
enable large zip file support in win_copy
@ansibot ansibot added affects_2.5 This issue/PR affects Ansible v2.5 bugfix_pull_request needs_triage Needs a first human triage before being processed. plugins/action support:core This issue/PR relates to code supported by the Ansible Engineering Team. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Nov 8, 2017
@jborean93 jborean93 added windows Windows community and removed needs_triage Needs a first human triage before being processed. labels Nov 8, 2017
@jborean93
Copy link
Contributor

jborean93 commented Nov 9, 2017 •
edited

Does this actually work for you, by default the compression and allowZip64 are set to ZIP_STORED and True. The only difference that this makes is that instead of just creating a zip file with no compression (ZIP_STORED), it compresses the archive (ZIP_DEFLATED).

I never set ZIP_DEFLATED as it requires zlib and I wanted to reduce the number of dependencies as much as possible. I can't find any documentation to say otherwise but I would have thought zipfile would be able to use the Zip64 extension regardless of the compression chosen.

I'll do some testing the background and see if t3hat is the case.

Edit 1: Looks like since Python 3.4 allowZip64 is set to true but on 2.6 and 2.7 it is set to false potentially causing this issue.

jborean93
jborean93 requested changes Nov 9, 2017
View reviewed changes
Copy link
Contributor

@jborean93 jborean93 left a comment •
edited

Choose a reason for hiding this comment

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

Have tested this with Python 2.6, 2.7 and 3.6 and it works properly. Once you've removed the compression setting I'm happy to merge it in.

lib/ansible/plugins/action/win_copy.py Outdated
@@ -233,7 +233,7 @@ def _create_content_tempfile(self, content):
def _create_zip_tempfile(self, files, directories):
tmpdir = tempfile.mkdtemp()
zip_file_path = os.path.join(tmpdir, "win_copy.zip")
zip_file = zipfile.ZipFile(zip_file_path, "w")
zip_file = zipfile.ZipFile(zip_file_path, "w", zipfile.ZIP_DEFLATED, True)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you change this to just set allowZip64=True and not set the compression.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me run a test here too; once that passes with your suggested setting, I will do a new commit. Stay tuned.

@u625030
Copy link
Contributor Author

u625030 commented Nov 9, 2017

I ran some tests here with the ZIP_STORE setting rather than ZIP_DEFLATE and that works ok. Commited the ZIP_STORE setting.

@jborean93
Copy link
Contributor

jborean93 commented Nov 9, 2017 •
edited

Errors are unrelated to the code changes, thanks for the fix @u625030.

@jborean93 jborean93 merged commit 6d597ac into ansible:devel Nov 9, 2017
jborean93 pushed a commit that referenced this pull request Nov 9, 2017
@u625030 @jborean93
Update win_copy for #32677 (#32682)
612d9e0 
* Update win_copy for #32677

enable large zip file support in win_copy

* Update win_copy.py

(cherry picked from commit 6d597ac)
@jborean93
Copy link
Contributor

Fix cherry-picked to stable-2.4 612d9e0, will be in the 2.4.2rc1 release.

@ansibot ansibot added bug This issue/PR relates to a bug. and removed bugfix_pull_request labels Mar 6, 2018
@ansible ansible locked and limited conversation to collaborators Apr 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jborean93 jborean93 jborean93 approved these changes

Assignees
No one assigned
Labels
affects_2.5 This issue/PR affects Ansible v2.5 bug This issue/PR relates to a bug. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. plugins/action support:core This issue/PR relates to code supported by the Ansible Engineering Team. windows Windows community
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

win_copy does not support large files
3 participants
@u625030 @jborean93 @ansibot