win_zip Module #177

Open
wants to merge 6 commits into
from

Projects

None yet

10 participants

@schwartzmx
Contributor

win_zip module:

  • Uses PowerShell Community Extensions' (PSCX) Write-Zip cmdlet to zip the specified file or directory.
    • The reason for this, is implementing zipping of files/directories in PowerShell natively is ugly. Files are sometimes skipped and other weird anomalies have been noted of happening.
    • PSCX Write-Zip uses 7zip as the core library, and is much more reliable.
  • If PSCX is not detected on the machine, it will be downloaded and installed, then imported for the script.
  • Also takes a rm argument, for specifying whether to remove the src file after compression is completed.
  • The dest argument's path must exist, but the basename does not need to exist.
    • The script detects whether the .zip extension for dest was supplied, and adds it if need be.
  • I've tried to make exception handling and the resultant error messages as specific and helpful as possible.

Updates 1/11:

  • Added tar, gzip, bzip functionality.
    • Added param type to take care of this.
      • Ex: type: gzip
  • Fixes to PSCX import problems after installation
  • Updated docs
  • Also added attempt to install PSCX with chocolatey if present on the machine, if not continue with the .msi installation.
  • Updates to win_unzip module PR: #174 have been made to coincide with the changes here.

Questions, comments, or suggestions for additions please let me know.

Phil

@schwartzmx schwartzmx referenced this pull request Jan 11, 2015
Merged

win_unzip Module #174

@TimothyVandenbrande
Contributor

Is there a specific reason you auto-download/install and install pscx?
I believe the strength of ansible relies on the fact that you don't need software on your clients to run it, only configuration.

I would rather see it happen like this:
http://blogs.msdn.com/b/daiken/archive/2007/02/12/compress-files-with-windows-powershell-then-package-a-windows-vista-sidebar-gadget.aspx

Especially since I don't like things to be automatically installed on my systems.

(I have the same argument for: #174)

@schwartzmx
Contributor

What you linked will only work for .zip files, and it is very fragile depending on the size of the files hence the code:

foreach($file in $input) 
    { 
            $zipPackage.CopyHere($file.FullName)
            **Start-sleep -milliseconds 500**
    }

That doesn't look reliable to me or ensure consistency.

PSCX is very powerful and allows much more added functionality than compressing and decompressing files/folders. I understand that you don't want things to be automatically installed.

I can change it to fail if PSCX isn't installed, but the reason I did it this way is because I was constantly bringing up new machines, and I didn't want it to fail when doing application configuration so it was autoinstalled. I can change it though and just add more steps in my playbook, using win_get_url and use win_msi to install PSCX.

Pertaining to these modules, I can change it to do a check for PSCX, and use a similar method to that article for just .zip files and if the extension is otherwise, then I could check if it's installed to be used for .msu .bz2 .tar and .gz files.

@schwartzmx schwartzmx updates docs, added tar,bzip,gzip functionality
- Updates docs to incorporate new functionality
- Added parameter type
- Fixed issue with error of importing PSCX (if it needed to be installed)
  - Also added an attempt to install with chocolatey (if present on the system)
- Other fixes and improvements
8a7c3c6
@wenottingham wenottingham referenced this pull request Jun 1, 2015
Closed

Windows unzip #553

@gregdek
Contributor
gregdek commented Jun 17, 2015

See discussion in #553 about which module we choose.

@schwartzmx
Contributor

@gregdek, @trondhindenes replicated the discussed (from #174) changes and proposals to this module as well.

@schwartzmx schwartzmx adds changes param, removes force install of PSCX, uses built-in meth…
…od for '.zip' files if PSCX does not exist on the machine.
697595d
@gregdek
Contributor
gregdek commented Sep 25, 2015

Adding new process. We will be evaluating all new module PRs according to this process, effective immediately.

Thanks for submitting this new module to Ansible Extras! This module is now in community review, a process that is open to all Ansible users. In order for this module to be approved, it must gain the following votes:

“works_for_me”: If you have tested the module thoroughly, including testing of all of the module’s options, and if the module works for you, please add “works_for_me” in the comments.

“passes_guidelines”: If you have gone through the module guidelines and the module meets all of the requirements, please add “passes_guidelines” in the comments. Guidelines are available here: http://docs.ansible.com/developing_modules.html#module-checklist

“needs_revision”: If the module fails to work for you, or if it doesn’t meet guidelines, please add “needs_revision” in the comments with details about what needs to be fixed.

When a module has both “works_for_me” and “passes_guidelines” tags, we will promote the module for inclusion in Ansible Extras. At this point, you will be expected to maintain the module by fixing bugs and evaluating pull requests in a timely manner.

Thanks again for submitting your Ansible module!

@trondhindenes
Contributor

@schwartzmx I wouldn't assume that the pscx module is in any specified location. From how I read the code you first test that it exists, and if so you should be able to just import it (or at least use the psmodulepath attribute from the module object to get to the actual module dir instead of assuming its in C:\program files (x86).

I would Update the documentation to specify that pscx needs to be install and in a path where Powershell can auto-load it ($env:PSModulePath), and then import the module using auto-loading.

@trondhindenes trondhindenes and 1 other commented on an outdated diff Sep 25, 2015
windows/win_zip.ps1
+# Check if PSCX is installed
+$list = Get-Module -ListAvailable
+If (-Not ($list -match "PSCX")) {
+ Set-Attr $result.win_zip "pscx_status" "absent"
+ $pscxPresent = $false
+}
+Else {
+ $pscxPresent = $true
+ Set-Attr $result.win_zip "pscx_status" "present"
+}
+
+# Import
+Try {
+ If ($pscxPresent) {
+ Try {
+ Import-Module 'C:\Program Files (x86)\Powershell Community Extensions\pscx3\pscx\pscx.psd1'
@trondhindenes
trondhindenes Sep 25, 2015 Contributor

This is the part I don't get

@schwartzmx
schwartzmx Oct 1, 2015 Contributor

Yep good point @trondhindenes, the reason that was like this is because when I originally wrote this I was auto-installing PSCX and the module couldn't import it as it was running in the same PS session, so I had to import it from the install location. Just a bad lingering snippet. Will fix.

@trondhindenes
Contributor

There are now two modules in PR that do pretty much the same: #999 and #177. Maybe figure out which one to go forward with so we don't waste anyone's coding cycles? ping @schwartzmx @davidobrien1985

@davidobrien1985

Hey @trondhindenes mine only uses native PowerShell and doesn't need any external prereqs. Then again, probably not my call?!

@schwartzmx
Contributor

@trondhindenes @davidobrien1985 We can use whomever's. They both have the same idea in mind but have different functionalities.

This one does bzip (pscx), gzip (pscx), tar (pscx) and zip (natively if pscx isn't available). The other only does zip it appears.

Both have dependencies, here PSCX (if doing non-zip format, i.e. gzip).
The other, .NET 4.5.

Whichever way we go is fine with me. I don't mind if we close this, but since #174 got merged and is consistent with this one, if we close this one, we should revert #174 as well.

@jhawkesworth
Contributor

Not really contributed to either, but how about we have both? @schwartzmx 's modules support other formats so they are have utility. I'd suggest adding _pscx onto the end of @schwartzmx's module names the so they are differentiated, or perhaps call them win_archive/ win_unarchive. I guess it would fairest if this was named something like win_zip_net45 but seems a little overkill when you get .net 4.5 or later installed with most recent windows. Ultimately as long as its clear which version to chose from the module docs I see no harm in having both modules.

@bcoca
Member
bcoca commented Oct 2, 2015

actually having a single module that can do it 'both ways' might be more interesting, not sure what advantages/disadvantages each way has. It seems it would be easy enough to detect and use one and fail to the other or add an option to let the user decide which one to use.

@robynbergeron robynbergeron removed the P3 label Dec 22, 2015
@ansibot
ansibot commented Dec 6, 2016

This repository has been locked. All new issues and pullrequests should be filed in https://github.com/ansible/ansible

Please read through the repomerge page in the dev guide. The guide contains links to tools which automatically move your issue or pullrequest to the ansible/ansible repo.

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