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

Skip dotnet-install #40551

Closed
wants to merge 1 commit into from
Closed

Skip dotnet-install #40551

wants to merge 1 commit into from

Conversation

sebastienros
Copy link
Member

Whenever I build locally it tries to install all the required runtimes even if they are present. It seems like dotnet-install is not very optimal in verifying it's already done. This change will skip dotnet-install if we know the runtime is already there. On my machine it saves 10 seconds on each build.

We can't merge this PR but this is to get some feedback.

Before

Detected JDK in C:\code\aspnetcore60\eng\..\.tools\jdk\win-x64\ (via local repo convention)

  Determining projects to restore...
  Tool 'dotnet-serve' (version '1.8.15') was restored. Available commands: dotnet-serve
  Tool 'microsoft.playwright.cli' (version '1.2.2') was restored. Available commands: playwright

  Restore was successful.
  All projects are up-to-date for restore.
  Determining projects to restore...
  All projects are up-to-date for restore.
  GenerateFiles -> C:\code\aspnetcore60\artifacts\bin\GenerateFiles\Directory.Build.props
  GenerateFiles -> C:\code\aspnetcore60\artifacts\bin\GenerateFiles\Directory.Build.targets
Attempting to install dotnet from public location.
dotnet-install: Note that the intended use of this script is for Continuous Integration (CI) scenarios, where:
dotnet-install: - The SDK needs to be installed without user interaction and without admin rights.
dotnet-install: - The SDK installation doesn't need to persist across multiple CI runs.
dotnet-install: To set up a development environment or to run apps, use installers rather than this script. Visit https://dotnet.microsoft.com/download to get the installer.

dotnet-install: .NET Core Runtime with version '2.1.30' is already installed.
dotnet-install: Adding to current process PATH: "C:\code\aspnetcore60\.dotnet\". Note: This change will not be visible if PowerShell was run as a child process.
Attempting to install dotnet from public location.
dotnet-install: Note that the intended use of this script is for Continuous Integration (CI) scenarios, where:
dotnet-install: - The SDK needs to be installed without user interaction and without admin rights.
dotnet-install: - The SDK installation doesn't need to persist across multiple CI runs.
dotnet-install: To set up a development environment or to run apps, use installers rather than this script. Visit https://dotnet.microsoft.com/download to get the installer.

dotnet-install: .NET Core Runtime with version '7.0.0-preview.3.22127.1' is already installed.
dotnet-install: Adding to current process PATH: "C:\code\aspnetcore60\.dotnet\". Note: This change will not be visible if PowerShell was run as a child process.
Attempting to install dotnet from public location.
dotnet-install: Note that the intended use of this script is for Continuous Integration (CI) scenarios, where:
dotnet-install: - The SDK needs to be installed without user interaction and without admin rights.
dotnet-install: - The SDK installation doesn't need to persist across multiple CI runs.
dotnet-install: To set up a development environment or to run apps, use installers rather than this script. Visit https://dotnet.microsoft.com/download to get the installer.

dotnet-install: .NET Core Runtime with version '7.0.0-preview.3.22127.1' is already installed.
dotnet-install: Adding to current process PATH: "C:\code\aspnetcore60\.dotnet\x86\". Note: This change will not be visible if PowerShell was run as a child process.
Attempting to install dotnet from public location.
dotnet-install: Note that the intended use of this script is for Continuous Integration (CI) scenarios, where:
dotnet-install: - The SDK needs to be installed without user interaction and without admin rights.
dotnet-install: - The SDK installation doesn't need to persist across multiple CI runs.
dotnet-install: To set up a development environment or to run apps, use installers rather than this script. Visit https://dotnet.microsoft.com/download to get the installer.

dotnet-install: ASP.NET Core Runtime with version '3.1.21' is already installed.
dotnet-install: Adding to current process PATH: "C:\code\aspnetcore60\.dotnet\". Note: This change will not be visible if PowerShell was run as a child process.
  Determining projects to restore...
  All projects are up-to-date for restore.
  RepoTasks -> C:\code\aspnetcore60\artifacts\bin\RepoTasks\Release\net7.0\RepoTasks.dll
  RepoTasks -> C:\code\aspnetcore60\artifacts\bin\RepoTasks\Release\net472\RepoTasks.dll

After

Detected JDK in C:\code\aspnetcore60\eng\..\.tools\jdk\win-x64\ (via local repo convention)

  Determining projects to restore...
  Tool 'dotnet-serve' (version '1.8.15') was restored. Available commands: dotnet-serve
  Tool 'microsoft.playwright.cli' (version '1.2.2') was restored. Available commands: playwright

  Restore was successful.
  All projects are up-to-date for restore.
  Determining projects to restore...
  All projects are up-to-date for restore.
  GenerateFiles -> C:\code\aspnetcore60\artifacts\bin\GenerateFiles\Directory.Build.props
  GenerateFiles -> C:\code\aspnetcore60\artifacts\bin\GenerateFiles\Directory.Build.targets

@ghost ghost added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Mar 5, 2022
@sebastienros
Copy link
Member Author

/cc @adityamandaleeka @dotnet/aspdoi

$runtimePath = $runtimePath + "\" + $version

if (Test-Path $runtimePath) {
$installSuccess = $true
Copy link
Member Author

Choose a reason for hiding this comment

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

I think this can be removed

Copy link
Member

@dougbu dougbu left a comment

Choose a reason for hiding this comment

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

We do not own these files. They're automatically updated whenever we get a new version of Arcade. If you want to skip dotnet installations, create a PR in dotnet/Arcade.
Besides that, this approach will usually save only the download of the dotnet-install.* script (if it's not cached in .dotnet/) and a check of the *productVersion.txt file for whatever is being installed.
What problem are you trying to solve❔

@dougbu
Copy link
Member

dougbu commented Mar 5, 2022

From the PR description, it seems everything is already installed in your case. The scripts avoided any time-consuming downloads.

@sebastienros
Copy link
Member Author

We do not own these files. They're automatically updated whenever we get a new version of Arcade.

I know, I mentioned we can't merge that PR for this reason. I want a discussion only.

Besides that, this approach will usually save only the download of the dotnet-install.* script

These files are cached already with my tests. It's 10 secs on script execution.

What problem are you trying to solve?

It's taking 10s of my life every time I start a build

@sebastienros
Copy link
Member Author

Looks like it's the invocation of the script that is very slow, the dotnet-install script run by itself is very fast when the runtime is already installed.

@RussKie
Copy link
Member

RussKie commented Mar 22, 2023

@sebastienros is this something still relevant?

@sebastienros
Copy link
Member Author

I still believe this is something we should look to improve build times.

@adityamandaleeka
Copy link
Member

@sebastienros Are you still seeing this behavior? I just tried it and didn't see any time difference on my machine with/without the change (I did consecutive unchanged builds of src\Servers\Kestrel).

If this is still happening for you, I recommend we start a discussion with the Arcade folks about it instead of just keeping this PR open.

@sebastienros
Copy link
Member Author

I checked, investigated, found more details and I confirm it's still a problem and this PR is helping.
I filed an issue in Arcade with more potential gains suggestion.

dotnet/arcade#13128

@sebastienros
Copy link
Member Author

For those watching this issue, it was fixed by @RussKie in Arcade dotnet/arcade#13359

@sebastienros sebastienros deleted the sebros/dotnetinstall branch May 5, 2023 16:36
@adityamandaleeka
Copy link
Member

Nice!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants