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

Only write cross platform compatible directory separators in Compress-Archive #62

Merged
merged 5 commits into from Feb 19, 2019
Merged

Conversation

mryanmurphy
Copy link
Contributor

@mryanmurphy mryanmurphy commented Aug 22, 2018

This resolves #11 and #48

The approach taken is leveraging the fact that .net on Windows all the way back to Framework 1.1 specifies \ as DirectoryPathSeparatorChar and / as AltDirectoryPathSeparatorChar, while other platforms in .net Core use / for DirectoryPathSeparatorChar and AltDirectoryPathSeparatorChar.

When using a *nix platform, the replacements will be no-ops, while Windows will convert all \ to / for the purposes of the ZipEntry FullName.

@msftclas
Copy link

msftclas commented Aug 22, 2018

CLA assistant check
All CLA requirements met.

@mryanmurphy
Copy link
Contributor Author

Not sure who the maintainers of the repo are, so this has been waiting for quite some time. As top contributor @anmenaga, is there anything I can do to get some visibility?

@SteveL-MSFT
Copy link
Member

@mryanmurphy we appreciate your contribution. Sorry about the delay. @anmenaga can you review this?

Copy link
Collaborator

@anmenaga anmenaga left a comment

Choose a reason for hiding this comment

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

Overall looks good; can merge after some minor updates:

  1. Please bump ModuleVersion from 1.2.2.0 to 1.2.3.0 in Microsoft.PowerShell.Archive/Microsoft.PowerShell.Archive.psd1
  2. Please add to the end of the description that archive expansion on Windows is doing the opposite conversion from forward slashes to backslashes in Expand-Archive/ExpandArchiveHelper/Join-Path.
  3. Entire PR description should be added as a comment to DirectorySeparatorNormalizeHelper. This will greatly help with reading the code.
  4. It would be good to rebase this PR based on the latest code in the repo.

Thank you.

@mryanmurphy
Copy link
Contributor Author

Thanks for the feedback all, I believe I addressed each point.

@mryanmurphy
Copy link
Contributor Author

@anmenaga @SteveL-MSFT Let me know if there's anything else I can do to get this merged. Thanks!

@anmenaga anmenaga merged commit b8b6131 into PowerShell:master Feb 19, 2019
@mikebridge
Copy link

Is there a workaround for the separator issue until version 1.2.3 is generally available?

@mikebridge
Copy link

mikebridge commented Feb 27, 2019

This should fix files that have been extracted with backslashes in the meantime:

for file in *\\*; do target="${file//\\//}"; mkdir -p "${target%/*}"; mv -v "$file" "$target"; done

https://stackoverflow.com/questions/44490413/how-to-rename-file-name-contains-backslash-in-bash#answer-44490964

@mryanmurphy
Copy link
Contributor Author

I'm not sure how often they push changes to PSGallery. Since opening the PR I've had the fixed module on my machines with a #Requires -Modules @{ ModuleName="Microsoft.PowerShell.Archive"; ModuleVersion="1.2.3.0" } statement on scripts involving archives. Downloading the source and moving the Microsoft.PowerShell.Archive folder to ~/Documents/WindowsPowerShell/Modules and ~/Documents/PowerShell/Modules has worked for me.

Besides that I think the only workaround would be to not use this module for cross platform archiving.

@mikebridge
Copy link

I since discovered that tar on windows is a thing:

https://blogs.technet.microsoft.com/virtualization/2017/12/19/tar-and-curl-come-to-windows/

That works better for what I'm trying to do, which is extract archives on linux machines.

@mikemaccana
Copy link

Compress-Archive still does not write cross platform directory separators on Powershell 6.2

Is compress-archive part of Powershell itself, i.e should 6.2 should include this fix?

If so there seems to be more issues with directory separators:

#71

@mikemaccana
Copy link

Hrm it seems 6.2 still ships with the broken compress-archive:

get-Module Microsoft.PowerShell.Archive

ModuleType Version    Name                                ExportedCommands
---------- -------    ----                                ----------------
Manifest   1.2.2.0    Microsoft.PowerShell.Archive        {Compress-Archive, Expand-Archive}

And I can't update it:

Update-Module Microsoft.PowerShell.Archive
Update-Module : Module 'Microsoft.PowerShell.Archive' was not installed by using Install-Module, so it cannot be updated.

How can we get our hands on the fix?

@mryanmurphy
Copy link
Contributor Author

@SteveL-MSFT is there a process to get version 1.2.3.0 deployed to PS Gallery?

@SteveL-MSFT
Copy link
Member

@anmenaga is working on publishing new version of Archive module to PSGallery. Unfortunately, we missed this and didn't get it into 6.2. Perhaps we can pick it up for a future 6.2.x servicing release.

@mryanmurphy
Copy link
Contributor Author

Looks like it got into 6.2.1 today from PowerShell/PowerShell#9594, and updated in PSGallery last week https://www.powershellgallery.com/packages/Microsoft.PowerShell.Archive/1.2.3.0

Thanks.

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.

Compress-Archive on Windows PowerShell generates an archive incompatible with OS X Archiver
6 participants