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

(GH-616) Capture 7z's stdout in Get-ChocolateyUnzip #617

Closed

Conversation

Erlkoenig90
Copy link
Contributor

Sometimes, Install-ChocolateyZipPackage and Get-ChocolateyUnzip capture
files that don't belong to the package, record them in the
Install.zip.txt file, and cause them to be removed upon
uninstallation by UnInstall-ChocolateyZipPackage. This fix uses 7z's
standard output instead to determine which files belong to the package.
This also avoids scanning possibly large directory trees.
This commit should fix the issue GH-616.

Sometimes, Install-ChocolateyZipPackage and Get-ChocolateyUnzip capture
files that don't belong to the package, record them in the
<packagename>Install.zip.txt file, and cause them to be removed upon
uninstallation by UnInstall-ChocolateyZipPackage. This fix uses 7z's
standard output instead to determine which files belong to the package.
This also avoids scanning possibly large directory trees.
Add-Content $zipExtractLogFullPath $file
}
# Print the line, such that it looks as if stdout was not redirected
Write-Debug $line
Copy link
Member

Choose a reason for hiding this comment

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

Would you move this to Write-Verbose? This will help with #476

@ferventcoder
Copy link
Member

Overall 👍

Only print 7z's stdout in verbose mode, buffer the file list while
writing to <packagename>Install.zip.txt, and don't create that file at
all if the archive was empty.
@Erlkoenig90
Copy link
Contributor Author

Thanks for the quick feedback! I incorporated your suggestions in commit 4a2b56d. The file is written using a StringBuffer and a FileStream, and removed again if nothing was written at all.

@ferventcoder
Copy link
Member

FileStream may have issues.

@Erlkoenig90
Copy link
Contributor Author

Okay, what kind of issues? Should is change it to Add-Content?

@ferventcoder
Copy link
Member

It may be fine. No worries

while(($process.StandardOutput -ne $null) -and (($line = $process.StandardOutput.ReadLine()) -ne $null)) {
if($line.StartsWith("Extracting")) {
# This is a line indicating an extracted file
$file = $destination + "\" + $line.Substring(12)
Copy link
Member

Choose a reason for hiding this comment

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

What is this 12?

Copy link
Member

Choose a reason for hiding this comment

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

Nevermind, the two spaces after "Extracting".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In 7z's output in the form "Extracting path\of\file\in\zip", the file path begins at character 12.

@ferventcoder
Copy link
Member

I changed this a bit to better match Start-ProcessAsAdmin (it was pretty close already). I think you'll find the perf quite sufficient.

@ferventcoder
Copy link
Member

Commits squashed, rebased and updated at bde0941

Merged into stable at 95c000d

@ferventcoder
Copy link
Member

Thanks for your contribution!

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