flowey: migrate nuget package restore from nuget.exe to dotnet restore#2907
flowey: migrate nuget package restore from nuget.exe to dotnet restore#2907benhillis wants to merge 6 commits intomicrosoft:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Migrates Flowey’s NuGet package restore path from nuget.exe-based installs to dotnet restore, adding Flowey nodes to provision the .NET SDK and removing Azure Credential Provider plumbing that is no longer needed.
Changes:
- Reworked
nuget_install_packageto generate a synthetic.csprojand download packages viadotnet restore, with local auth using anaz-derived Azure DevOps session token. - Added Flowey nodes to install/locate
dotnet(install_dotnet_cli) and to emit the ADOUseDotNet@2task (ado_task_use_dotnet). - Removed local CLI flags and nodes related to
nuget.exe/mono and Azure Credential Provider installation; updated pipeline parameter wiring accordingly.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| flowey/flowey_lib_hvlite/src/_jobs/cfg_common.rs | Drops nuget/credential-provider wiring; adds dotnet CLI auto-install request for local runs. |
| flowey/flowey_lib_common/src/nuget_install_package.rs | Switches NuGet downloads to dotnet restore; adds local ADO session-token injection into a per-restore nuget.config. |
| flowey/flowey_lib_common/src/lib.rs | Exposes new dotnet-related modules and removes the credential provider module export. |
| flowey/flowey_lib_common/src/install_nuget_azure_credential_provider.rs | Removes the Azure Artifacts Credential Provider installer node. |
| flowey/flowey_lib_common/src/install_dotnet_cli.rs | New node to resolve/install dotnet across Local/ADO/GitHub backends. |
| flowey/flowey_lib_common/src/download_nuget_exe.rs | Simplifies nuget.exe download logic; removes install-platform and WSL mono forcing. |
| flowey/flowey_lib_common/src/ado_task_use_dotnet.rs | New ADO task wrapper for UseDotNet@2. |
| flowey/flowey_lib_common/src/ado_task_nuget_tool_installer.rs | Removes the ADO NuGetToolInstaller@1 wrapper. |
| flowey/flowey_hvlite/src/pipelines_shared/cfg_common_params.rs | Removes local flags for nuget mono/auth and stops passing them into cfg_common. |
| flowey/flowey_hvlite/src/pipelines/vmm_tests.rs | Stops populating removed cfg_common local nuget fields. |
| flowey/flowey_hvlite/src/pipelines/restore_packages.rs | Stops populating removed cfg_common local nuget fields. |
| flowey/flowey_hvlite/src/pipelines/custom_vmfirmwareigvm_dll.rs | Stops populating removed cfg_common local nuget fields. |
| flowey/flowey_hvlite/src/pipelines/build_igvm.rs | Stops populating removed cfg_common local nuget fields. |
You can also share your feedback on Copilot code review. Take the survey.
benhillis
force-pushed
the
user/benhill/dotnet_restore
branch
from
a999251 to
7674711
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
You can also share your feedback on Copilot code review. Take the survey.
benhillis
force-pushed
the
user/benhill/dotnet_restore
branch
from
7674711 to
61407c4
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
You can also share your feedback on Copilot code review. Take the survey.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
You can also share your feedback on Copilot code review. Take the survey.
benhillis
force-pushed
the
user/benhill/dotnet_restore
branch
2 times, most recently
from
43d636a to
7af08bf
Compare
| @@ -6,23 +6,10 @@ | |||
| use flowey::node::prelude::*; | |||
There was a problem hiding this comment.
This is only used by a few internal pipelines that I will be changing over in a follow-up change.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 15 changed files in this pull request and generated 2 comments.
You can also share your feedback on Copilot code review. Take the survey.
benhillis
force-pushed
the
user/benhill/dotnet_restore
branch
from
7af08bf to
57e506d
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 15 changed files in this pull request and generated 5 comments.
You can also share your feedback on Copilot code review. Take the survey.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 15 changed files in this pull request and generated 2 comments.
You can also share your feedback on Copilot code review. Take the survey.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 15 changed files in this pull request and generated no new comments.
You can also share your feedback on Copilot code review. Take the survey.
benhillis
force-pushed
the
user/benhill/dotnet_restore
branch
from
9d38b33 to
b27cf79
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated 2 comments.
You can also share your feedback on Copilot code review. Take the survey.
Replace nuget.exe with dotnet restore for all NuGet package downloads. Authentication on local builds uses az CLI session tokens injected into nuget.config. CI builds use the UseDotNet ADO task and pipeline-level auth. Also removes the Azure Credential Provider module and external_nuget_auth plumbing, which are no longer needed. Adds install_dotnet_cli and ado_task_use_dotnet flowey nodes. Updates getting started docs with az CLI login instructions and dotnet-sdk-8.0 as a WSL dependency.
benhillis
force-pushed
the
user/benhill/dotnet_restore
branch
from
b27cf79 to
95c7160
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated 2 comments.
You can also share your feedback on Copilot code review. Take the survey.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated 2 comments.
You can also share your feedback on Copilot code review. Take the survey.
| let dotnet_bin_name = rt.platform().binary("dotnet"); | ||
| let dotnet_bin_path = install_dir.join(&dotnet_bin_name); | ||
|
|
||
| if !dotnet_bin_path.exists() { | ||
| log::info!( |
There was a problem hiding this comment.
When auto_install is enabled, this code skips running dotnet-install as soon as the dotnet binary exists in the persistent install dir. That can leave an outdated SDK in place (or ignore a different requested Version channel), and later dotnet restore may fail if the required SDK isn’t actually installed. Consider either always re-running dotnet-install (it’s idempotent) for the requested channel, or validating the installed SDKs (e.g. via dotnet --list-sdks) before deciding to skip installation.
| fn move_dir(src: &Path, dest: &Path) -> anyhow::Result<()> { | ||
| match fs_err::rename(src, dest) { | ||
| Ok(()) => Ok(()), | ||
| Err(e) => { | ||
| // rename(2) fails with EXDEV (errno 18 on Linux, error 17 on | ||
| // Windows) when src and dest are on different filesystems. | ||
| // Fall back to a recursive copy + delete. | ||
| log::debug!( |
There was a problem hiding this comment.
move_dir falls back to recursive copy+delete on any rename error. That can mask real failures (e.g. permissions, dest exists) and potentially do extra work before failing. It’s safer to only fall back when the error is an actual cross-filesystem move (EXDEV) and otherwise return the original error.
Replace nuget.exe with dotnet restore for all NuGet package downloads. Authentication on local builds uses az CLI session tokens passed via VSS_NUGET_EXTERNAL_FEED_ENDPOINTS. CI builds use the UseDotNet ADO task and pipeline-level auth.
Also removes the Azure Credential Provider module and external_nuget_auth plumbing, which are no longer needed.
Adds install_dotnet_cli and ado_task_use_dotnet flowey nodes. Updates getting started docs with az CLI login instructions and dotnet-sdk-8.0 as a WSL dependency.
There will be a follow-up change that removes the nuget install node, which is currently used on some internal pipelines that can be migrated over to use the ADO nuget task.