-
Notifications
You must be signed in to change notification settings - Fork 10.6k
[windows] fix sccache installation on arm64 #84292
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
[windows] fix sccache installation on arm64 #84292
Conversation
|
@swift-ci please smoke test |
utils/build.ps1
Outdated
| Write-Output "Extracting '$TarBallFileName' ..." | ||
| New-Item -ItemType Directory -ErrorAction Ignore -Path $destination | Out-Null | ||
| try { | ||
| tar -xvf $source -C $destination | Out-Null |
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.
Will this actually ever throw?
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.
This should not. I thought that if tar returned 1 the function would throw but that's not the case. Fixed, thanks!
utils/build.ps1
Outdated
| } | ||
| catch { |
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.
Please coddle the braces
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.
Removed the try/catch altogether.
utils/build.ps1
Outdated
| } | ||
| } | ||
|
|
||
| $destination = if ($CreateExtractPath) { $destination } else { $BinaryCache } |
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.
Why is this needed?
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.
I copied the Expand-ZipFile function to have the same functionalities. That's not needed in this patch, I removed it.
utils/build.ps1
Outdated
| [string]$TarBallFileName, | ||
| [string]$BinaryCache, | ||
| [string]$ExtractPath, | ||
| [bool]$CreateExtractPath = $true |
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.
These map to names, so please rename these:
TarBallFileName -> Source
ExtractPath -> Destination
CreateExtractPath -> CreateDestination
Do we ever invoke this with CreateDestinattion being $false? If not, perhaps we should just drop the parameter.
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.
Removed CreateDestination as we only call the function in one place.
The initial name of the parameters was copied from the Expand-ZipFile function. I have renamed them.
0b8ffb3 to
6c109eb
Compare
6c109eb to
071c789
Compare
|
@swift-ci please test |
utils/build.ps1
Outdated
| @@ -1174,6 +1174,31 @@ function Get-Dependencies { | |||
| Expand-Archive -Path $source -DestinationPath $destination -Force | |||
| } | |||
|
|
|||
| function Expand-TarBall { | |||
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.
Can we rename this to Expand-TapeArchive or Expand-TarFile? "tarball" is a very unix-y colloquialism.
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.
Switched to using Expand-TapeArchive.
utils/build.ps1
Outdated
| $source = Join-Path -Path $BinaryCache -ChildPath $SourceName | ||
| $destination = Join-Path -Path $BinaryCache -ChildPath $DestinationName |
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.
Please use Pascal Case
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.
Fixed, thanks!
071c789 to
5cc226a
Compare
|
@swift-ci please smoke test |
utils/build.ps1
Outdated
| @@ -1227,8 +1252,13 @@ function Get-Dependencies { | |||
|
|
|||
| if ($EnableCaching) { | |||
| $SCCache = Get-SCCache | |||
| DownloadAndVerify $SCCache.URL "$BinaryCache\sccache-$SCCacheVersion.zip" $SCCache.SHA256 | |||
| Expand-ZipFile sccache-$SCCacheVersion.zip $BinaryCache sccache-$SCCacheVersion | |||
| $FileExtension = if ($SCCache.URL.EndsWith('.tar.gz')) { "tar.gz" } else { "zip" } | |||
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.
[System.IO.Path]::GetExtension() is better to get the extension; that along with the inversion below should work.
if ($Extension -eq "zip") {
Expand-ZipFile ...
} else {
Expand-TapeArchive ...
}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! Testing only on Windows for now because I'm not sure if this API works for URLs as well.
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.
Seems to be working!
5cc226a to
1312cb7
Compare
|
@swift-ci please smoke test windows |
|
@swift-ci please smoke test linux |
|
@swift-ci please smoke test macos |
|
@swift-ci please test windows |
1 similar comment
|
@swift-ci please test windows |
Add an
Extract-TarBallfunction to build.ps1 and use it to extractSCCachearchives instead ofExtract-ZipFileon ARM64 Windows.SCCacheis released as a.tar.gzfile on ARM64 Windows, but as a.zipfile on x64 Windows. Because of this, with the-EnableCachineflag,build.ps1fails to installSCCacheon ARM64 Windows. This patch introduces anExtract-TarBallfunction which is then used to extracttar.gzfiles.