actions / runner-images

GitHub Actions runner images
MIT License
9.17k stars 2.84k forks source link

Add install of dotnet tools #4820

Closed josesimoes closed 2 years ago

josesimoes commented 2 years ago

Description

Related issue:

Check list

miketimofeev commented 2 years ago

/azp run windows2016, windows2019, windows2022, ubuntu1804, ubuntu2004

azure-pipelines[bot] commented 2 years ago
Azure Pipelines successfully started running 5 pipeline(s).
josesimoes commented 2 years ago

@miketimofeev can you please fire another build to test the latest changes?

miketimofeev commented 2 years ago

/azp run windows2016, windows2019, windows2022, ubuntu1804, ubuntu2004

azure-pipelines[bot] commented 2 years ago
Azure Pipelines successfully started running 5 pipeline(s).
josesimoes commented 2 years ago

I can see in the log that the tool it's failing to install on windows and I can't find any mention to it in linux. For Windows there is no detail that I can find about the failure reason. For Linux I suspect that's my code... 😊 I'm not used to linux and I'm not familiar with the jq library, so it's highly likely that's the reason.

I believe I've now fixed the tests, the framework wasn't happy about the string generation for the context and condition messages.

Really appreciate if you guys could review and fix any syntax errors that are there.

josesimoes commented 2 years ago

Hey @miketimofeev another spin please 🙂

miketimofeev commented 2 years ago

/azp run windows2016, windows2019, windows2022, ubuntu1804, ubuntu2004

azure-pipelines[bot] commented 2 years ago
Azure Pipelines successfully started running 5 pipeline(s).
josesimoes commented 2 years ago

@miketimofeev and again please.

(I can't figure out from the log what's wrong with the dotnet tool install... can you run it with debug output?)

josesimoes commented 2 years ago

hey @miketimofeev sorry for nagging you again with this, but can you run the build again? If possible with debug output to understand why the dotnet tool install it's failing on Windows.

Thanks! 😄

shilovmaksim commented 2 years ago

Hi @josesimoes . I queued builds for your PR.

josesimoes commented 2 years ago

Hopefully the install part is fixed for Windows and the Pester tests too. Please fire a new build.

josesimoes commented 2 years ago

@miketimofeev done and rebased with main! Please fire another build to check all these.

miketimofeev commented 2 years ago

/azp run windows2016, windows2019, windows2022, ubuntu1804, ubuntu2004

azure-pipelines[bot] commented 2 years ago
Azure Pipelines successfully started running 5 pipeline(s).
josesimoes commented 2 years ago

@miketimofeev please fire it again as the running one was canceled with the latest changes...

miketimofeev commented 2 years ago

@josesimoes could you please fix this comment as well? https://github.com/actions/virtual-environments/pull/4820/files#r781244560

josesimoes commented 2 years ago

@josesimoes could you please fix this comment as well? https://github.com/actions/virtual-environments/pull/4820/files#r781244560

Done!

miketimofeev commented 2 years ago

/azp run windows2016, windows2019, windows2022, ubuntu1804, ubuntu2004

azure-pipelines[bot] commented 2 years ago
Azure Pipelines successfully started running 5 pipeline(s).
josesimoes commented 2 years ago

@al-cheb the Pester test keeps failing for the tool, even with the latest changes you've suggested. I'm not familiar that framework so appreciate if you could look into that and letting me know what should be the correct syntax to use. Thanks!

miketimofeev commented 2 years ago

@al-cheb the Pester test keeps failing for the tool, even with the latest changes you've suggested. I'm not familiar that framework so appreciate if you could look into that and letting me know what should be the correct syntax to use. Thanks!

@josesimoes you need to pass the testcases param and variable, something like this https://github.com/actions/virtual-environments/blob/2e47a52b7e29c54c85559fc8f925dc7363c9f274/images/linux/scripts/tests/Android.Tests.ps1#L45-L60

josesimoes commented 2 years ago

@josesimoes you need to pass the testcases param and variable, something like this

@miketimofeev please check if my last commit is what you would like to see there

miketimofeev commented 2 years ago

/azp run ubuntu1804, ubuntu2004

azure-pipelines[bot] commented 2 years ago
Azure Pipelines successfully started running 2 pipeline(s).
josesimoes commented 2 years ago

The test seems to be running now with the proper command, but it fails when trying to call it... Something missing in the .NET SDK configuration to be able to run dotnet tools?

al-cheb commented 2 years ago

The test seems to be running now with the proper command, but it fails when trying to call it... Something missing in the .NET SDK configuration to be able to run dotnet tools?

The main issue is we install ngbv tool at user wide ~/.dotnet/tools folder which is not presented in PATH. Another issue we have to install this tool at system wide folder due to remove a user profile after an image generation has been finished. Any thoughts about system wide path?

josesimoes commented 2 years ago

The main issue is we install ngbv tool at user wide ~/.dotnet/tools folder which is not presented in PATH. Another issue we have to install this tool at system wide folder due to remove a user profile after an image generation has been finished. Any thoughts about system wide path?

Agreed. Looking at the nbgv github action, they're adjusting the path to have access to it, see here. So I would think that this is "known issue". On Windows it works out-of-the-box. I suggest we add this to the installer.

Maybe @AArnott can confirm this...

AArnott commented 2 years ago

I'm not sure why ~/.dotnet/tools isn't on the path by default on GitHub Action agents. But the workaround my own GitHub Action uses seems easy enough since it's always run in a script.

If the intent here is to make nbgv Just Work on agents without having to install the tool, that's fantastic. For this to be a good user experience, I agree we'd need that path added to PATH by default. That would help all other dotnet tools to just work as they do on local dev boxes as well.

josesimoes commented 2 years ago

I'm not sure why ~/.dotnet/tools isn't on the path by default on GitHub Action agents. But the workaround my own GitHub Action uses seems easy enough since it's always run in a script.

That seems to be the case. On Windows machines it's all good. On Linux, not so god... But the workaround seems to be clear-

If the intent here is to make nbgv Just Work on agents without having to install the tool, that's fantastic. For this to be a good user experience, I agree we'd need that path added to PATH by default. That would help all other dotnet tools to just work as they do on local dev boxes as well.

@AArnott that's exactly the point of all this! 😉

josesimoes commented 2 years ago

@al-cheb are we good with adding the tools path on the .NET SDK installer script?

al-cheb commented 2 years ago

In that case, I only see to install the tool to the /etc/skel/.dotnet as we do for rust - https://github.com/actions/virtual-environments/blob/af645a7b5e433a9fcc3ea1d6388896e0e3a8cad8/images/linux/scripts/installers/rust.sh

josesimoes commented 2 years ago

In that case, I only see to install the tool to the /etc/skel/.dotnet as we do for rust - https://github.com/actions/virtual-environments/blob/af645a7b5e433a9fcc3ea1d6388896e0e3a8cad8/images/linux/scripts/installers/rust.sh

@al-cheb if you install a dotnet tool in a specific path it will become a local tool instead of a global tool, which defeats the purpose of all this. Me thinks...

In other words: what's wrong with adding the dotnet tools path? As more dotnet tools are added to the images, it will work without any further changes or tweaks.

al-cheb commented 2 years ago

In that case, I only see to install the tool to the /etc/skel/.dotnet as we do for rust - https://github.com/actions/virtual-environments/blob/af645a7b5e433a9fcc3ea1d6388896e0e3a8cad8/images/linux/scripts/installers/rust.sh

@al-cheb if you install a dotnet tool in a specific path it will become a local tool instead of a global tool, which defeats the purpose of all this. Me thinks...

It's a small hack - The /etc/skel directory contains files and directories that are automatically copied over to a new user's home directory when such user is created by the useradd program. -> ~/.dotnet/tools

josesimoes commented 2 years ago

@al-cheb done, but not entirely sure if that's correct or not ðŸĪŠ

AArnott commented 2 years ago

if you install a dotnet tool in a specific path it will become a local tool instead of a global tool, which defeats the purpose of all this. Me thinks...

FWIW using --tool-path instead of -g does exactly the same thing, but installs to a different location. This isn't the same thing as installing a tool locally in some specific project manifest. You could literally do dotnet tool install --tool-path $HOME/.nuget/tools and have that be equivalent to using the -g switch, I'm pretty sure.

josesimoes commented 2 years ago

@al-cheb is the new install path OK? or should I point it to the existing prependEtcEnvironmentPath ?

al-cheb commented 2 years ago

@al-cheb is the new install path OK? or should I point it to the existing prependEtcEnvironmentPath ?

The path should be /etc/skel/.donet/tools and this path should be temporary add to PATH during the test like we do for rust - https://github.com/actions/virtual-environments/blob/af645a7b5e433a9fcc3ea1d6388896e0e3a8cad8/images/linux/scripts/tests/Tools.Tests.ps1 and software update - https://github.com/actions/virtual-environments/blob/af645a7b5e433a9fcc3ea1d6388896e0e3a8cad8/images/linux/scripts/SoftwareReport/SoftwareReport.Rust.psm1

$env:PATH = "/etc/skel/.dotnet/tools:$($env:PATH)"

josesimoes commented 2 years ago

@al-cheb done. Please check

josesimoes commented 2 years ago

I've just rebased the PR branch, should be good to go for a new build? @miketimofeev please

miketimofeev commented 2 years ago

/azp run windows2016, windows2019, windows2022, ubuntu1804, ubuntu2004

azure-pipelines[bot] commented 2 years ago
Azure Pipelines successfully started running 5 pipeline(s).
josesimoes commented 2 years ago

@al-cheb done! Please review and fire build.

miketimofeev commented 2 years ago

/azp run ubuntu1804, ubuntu2004

azure-pipelines[bot] commented 2 years ago
Azure Pipelines successfully started running 2 pipeline(s).
josesimoes commented 2 years ago

@al-cheb ðŸĨģ Linux VMs build ðŸŸĒ with test and version output as expected!! Can you please run the Windows pipelines to check those?

Or maybe @miketimofeev ...

miketimofeev commented 2 years ago

/azp run windows2016, windows2019, windows2022

azure-pipelines[bot] commented 2 years ago
Azure Pipelines successfully started running 3 pipeline(s).
miketimofeev commented 2 years ago

/azp run windows2016, windows2019, windows2022