Change Get-FileHash to close file handles before writing output #12474
Conversation
Fix the use case `Get-FileHash | Rename-Item`
by changing to `using () {}` and closing the hashed file before writing the output result.
|
|
There was a problem hiding this comment.
Looks good to me! Just a couple questions on the test. 😊
test/powershell/Modules/Microsoft.PowerShell.Utility/Get-FileHash.Tests.ps1
Outdated
Show resolved
Hide resolved
Copy the test document first, then remove the copy after, so it won't affect other tests. Assumes the test drive is writable.
There was a problem hiding this comment.
Looks good, nice work! 😊
|
@PoshChan please retry macos |
|
@vexx32, successfully started retry of |
|
NB. The failing |
src/Microsoft.PowerShell.Commands.Utility/commands/utility/GetHash.cs
Outdated
Show resolved
Hide resolved
Add more documentation comments. Remove Algorithm argument from ComputeFileHash method.
Add param descriptions.
src/Microsoft.PowerShell.Commands.Utility/commands/utility/GetHash.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.PowerShell.Commands.Utility/commands/utility/GetHash.cs
Outdated
Show resolved
Hide resolved
Move bytehash var into it. Update how it returns success/failure state.
src/Microsoft.PowerShell.Commands.Utility/commands/utility/GetHash.cs
Outdated
Show resolved
Hide resolved
|
@HumanEquivalentUnit Please rebase to get a fix for CI-static. |
Remove hashCompletedSuccessfully variable and replace with a return value calculated from hash variable.
Remove blank line before closing brace.
test/powershell/Modules/Microsoft.PowerShell.Utility/Get-FileHash.Tests.ps1
Outdated
Show resolved
Hide resolved
Change the test logic to try the rename, and then Test-Path the expected new name. Make the context and description clearer.
OK, I followed https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/syncing-a-fork and think it has merged the updated markdown files. Fingers crossed.
|
|
@PoshChan please retry macos |
test/powershell/Modules/Microsoft.PowerShell.Utility/Get-FileHash.Tests.ps1
Outdated
Show resolved
Hide resolved
Co-Authored-By: Ilya <darpa@yandex.ru>
|
@PoshChan please retry macos |
|
@vexx32, successfully started retry of |
|
Thank you for reviewing and improving it, @iSazonov ! |
|
🎉 Handy links: |
|
🎉 Handy links: |
…rShell#12474) # Conflicts: # src/Microsoft.PowerShell.Commands.Utility/commands/utility/GetHash.cs
PR Summary
Fix #12473 where Get-FileHash holds the hashed file handles open unnecessarily long.
Change is to hash the file contents, then dispose the file stream, then write the results, in that order.
PR Context
Get-ChildItem | Get-FileHash | Rename-Item -NewName {$_.Hash}throws because the file is still open when trying to be renamed, this PR tries to fix that by closing the file handle before Get-FileHash writes output.PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerWIP:or[ WIP ]to the beginning of the title (theWIPbot will keep its status check atPendingwhile the prefix is present) and remove the prefix when the PR is ready.