-
Notifications
You must be signed in to change notification settings - Fork 37
Mrdrwest issue#66 bugfix #68
base: master
Are you sure you want to change the base?
Conversation
The modification processes hidden files, but it does not preserve the compressed files Hidden attribute in the zip archive. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resubmitted due to build failure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this PR. It's off to a good start, but I left some comments about whitespace changes that should be undone, and I tried to answer the question you left in a comment.
Also, for this PR to be accepted, additional Pester tests should be added to verify that it works on Windows/macOS/Linux, archiving hidden files and restoring them as hidden.
@@ -323,7 +323,7 @@ function Expand-Archive | |||
else | |||
{ | |||
$createdItem = New-Item -Path $DestinationPath -ItemType Directory -Confirm:$isConfirm -Verbose:$isVerbose -ErrorAction Stop | |||
if($createdItem -ne $null -and $createdItem.PSProvider.Name -ne "FileSystem") | |||
if($null -ne $createdItem -and $createdItem.PSProvider.Name -ne "FileSystem") | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this PR, and for some of the best practice changes. It is very much appreciated.
@@ -744,10 +744,10 @@ function ZipArchiveHelper | |||
# At this point we are sure that the archive file has write access. | |||
$archiveFileStreamArgs = @($destinationPath, $fileMode) | |||
$archiveFileStream = New-Object -TypeName System.IO.FileStream -ArgumentList $archiveFileStreamArgs | |||
|
|||
$zipArchiveArgs = @($archiveFileStream, [System.IO.Compression.ZipArchiveMode]::Update, $false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like you have additional whitespace changes in the PR that shouldn't be included. Can you please remove them?
$zipArchiveArgs = @($archiveFileStream, [System.IO.Compression.ZipArchiveMode]::Update, $false) | ||
$zipArchive = New-Object -TypeName System.IO.Compression.ZipArchive -ArgumentList $zipArchiveArgs | ||
|
||
$currentEntryCount = 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto on whitespace changes.
@@ -765,7 +765,6 @@ function ZipArchiveHelper | |||
{ | |||
$relativeFilePath = [System.IO.Path]::GetFileName($currentFilePath) | |||
} | |||
|
|||
# Update mode is selected. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto on whitespace changes.
@@ -823,13 +822,13 @@ function ZipArchiveHelper | |||
if($null -ne $currentFileStream) | |||
{ | |||
$srcStream = New-Object System.IO.BinaryReader $currentFileStream | |||
|
|||
$entryPath = DirectorySeparatorNormalizeHelper $relativeFilePath |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto on whitespace changes.
@@ -842,7 +841,7 @@ function ZipArchiveHelper | |||
|
|||
while($numberOfBytesRead = $srcStream.Read($buffer, 0, $bufferSize)) | |||
{ | |||
$destStream.Write($buffer, 0, $numberOfBytesRead) | |||
$destStream.Write($buffer, 0, $numberOfBytesRead) # can file attributes be specified here | |||
$destStream.Flush() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To address your comment, looking at the docs it seems that each ZipArchiveEntry
has an ExternalAttributes
property that is OS and application dependent. I would start testing with that property, see what is captured in it when an archive is created, and what can be restored when an archive is extracted.
/azp run PowerShell.Microsoft.PowerShell.Archive |
Azure Pipelines successfully started running 1 pipeline(s). |
Modified script to process hidden files. Added -Force switch to the (only) Get-ChildItem cmdlet and the Get-Item cmdlet with the LastWriteTime member property.