Skip to content

Fix .NET installation #1320

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Jul 1, 2020
Merged

Fix .NET installation #1320

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6b4077e
Fix and parallelize .NET installation
rjmholt Jul 1, 2020
7a91c68
Serialize downloads
rjmholt Jul 1, 2020
20efc53
Move back to write-host
rjmholt Jul 1, 2020
98e6b27
Remove defunct parallel comment
rjmholt Jul 1, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 35 additions & 13 deletions PowerShellEditorServices.build.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,28 +55,52 @@ function Invoke-WithCreateDefaultHook {

function Install-Dotnet {
param (
$Channel
[string[]]$Channel
)

Write-Host "`n### Installing .NET CLI $Version...`n" -ForegroundColor Green
$env:DOTNET_INSTALL_DIR = "$PSScriptRoot/.dotnet"

Write-Host "Installing .NET channels $Channel" -ForegroundColor Green

# The install script is platform-specific
$installScriptExt = if ($script:IsUnix) { "sh" } else { "ps1" }
$installScript = "dotnet-install.$installScriptExt"
Copy link
Member

Choose a reason for hiding this comment

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

do you need this extra variable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gets used twice below


# Download the official installation script and run it
$installScriptPath = "$([System.IO.Path]::GetTempPath())dotnet-install.$installScriptExt"
Invoke-WebRequest "https://dot.net/v1/dotnet-install.$installScriptExt" -OutFile $installScriptPath
$env:DOTNET_INSTALL_DIR = "$PSScriptRoot/.dotnet"
$installScriptPath = Join-Path ([System.IO.Path]::GetTempPath()) $installScript
Invoke-WebRequest "https://dot.net/v1/$installScript" -OutFile $installScriptPath

# Download and install the different .NET channels
foreach ($dotnetChannel in $Channel)
{
Write-Host "`n### Installing .NET CLI $Version...`n"

if ($script:IsUnix) {
chmod +x $installScriptPath
}

$params = if ($script:IsUnix)
{
@('-Channel', $dotnetChannel, '-InstallDir', $env:DOTNET_INSTALL_DIR, '-NoPath', '-Verbose')
}
else
{
@{
Channel = $dotnetChannel
InstallDir = $env:DOTNET_INSTALL_DIR
NoPath = $true
Verbose = $true
}
}
Comment on lines +82 to +94
Copy link
Member

Choose a reason for hiding this comment

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

why did you do this instead of what I had before with Invoke-Expression?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Broke with my path which has spaces in it. Needing to fix it and wanting to avoid iex I went with this

Copy link
Member

Choose a reason for hiding this comment

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

Fine I guess... annoying having a platform check for the same thing.

Copy link
Member

Choose a reason for hiding this comment

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

cc @daxian-dbw

Copy link
Contributor Author

Choose a reason for hiding this comment

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

> invoke-build build
Build build C:\Users\Robert Holt\Documents\Dev\Microsoft\PowerShellEditorServices\PowerShellEditorServices.build.ps1
Task /build/BinClean
Done /build/BinClean 00:00:00.0032695
Task /build/SetupDotNet

### Installing .NET CLI ...

ERROR: The term 'C:\Users\Robert' is not recognized as the name of a cmdlet, function, script file, or operable program.
Check the spelling of the name, or if a path was included, verify that the path is correct and try again.
At C:\Users\Robert Holt\Documents\Dev\Microsoft\PowerShellEditorServices\PowerShellEditorServices.build.ps1:76 char:5
+     Invoke-Expression "$installScriptPath $paramArr"
+     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
At C:\Users\Robert Holt\Documents\Dev\Microsoft\PowerShellEditorServices\PowerShellEditorServices.build.ps1:82 char:1
+ task SetupDotNet -Before Clean, Build, TestHost, TestServerWinPS, Tes …
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
At C:\Users\Robert Holt\Documents\Dev\Microsoft\PowerShellEditorServices\PowerShellEditorServices.build.ps1:211 char:1
+ task Build BinClean,{
+ ~~~~~~~~~~~~~~~~~~~~~
Build FAILED. 3 tasks, 1 errors, 0 warnings 00:00:01.7445922
Invoke-Expression: C:\Users\Robert Holt\Documents\Dev\Microsoft\PowerShellEditorServices\PowerShellEditorServices.build.ps1:76
Line |
  76 |      Invoke-Expression "$installScriptPath $paramArr"
     |      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     | The term 'C:\Users\Robert' is not recognized as the name of a cmdlet, function, script file, or
     | operable program. Check the spelling of the name, or if a path was included, verify that the path is
     | correct and try again.

Copy link
Member

Choose a reason for hiding this comment

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

Oh I totally believe you... I just had exactly this code once and wanted to not have to do this, which is why I used iex in the first place. I would guess that if we wrapped InstallDir in quotes in might work... but seems hacky.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, fixing it I didnt' want to get into a quote escaping tarpit. Ideally install-dotnet.ps1 would work cross-platform. Frankly that script could use some improvement anyway


& $installScriptPath @params

if ($script:IsUnix) {
chmod +x $installScriptPath
Write-Host "`n### Installation complete for version $Version."
}

$paramArr = @('-Channel', $Channel, '-InstallDir', "'$env:DOTNET_INSTALL_DIR'", '-NoPath')
Invoke-Expression "$installScriptPath $paramArr"
$env:PATH = $env:DOTNET_INSTALL_DIR + [System.IO.Path]::PathSeparator + $env:PATH

Write-Host "`n### Installation complete." -ForegroundColor Green
Write-Host '.NET installation complete' -ForegroundColor Green
}

task SetupDotNet -Before Clean, Build, TestHost, TestServerWinPS, TestServerPS7, TestServerPS71, TestE2E {
Expand All @@ -85,9 +109,7 @@ task SetupDotNet -Before Clean, Build, TestHost, TestServerWinPS, TestServerPS7,
$dotnetExePath = if ($script:IsUnix) { "$dotnetPath/dotnet" } else { "$dotnetPath/dotnet.exe" }

if (!(Test-Path $dotnetExePath)) {
Install-Dotnet -Channel '2.1'
Install-Dotnet -Channel '3.1'
Install-Dotnet -Channel 'release/5.0.1xx-preview6'
Install-Dotnet -Channel '2.1','3.1','release/5.0.1xx-preview6'
}

# This variable is used internally by 'dotnet' to know where it's installed
Expand Down