/azp run dotnet-sdk-public-ci

/backport to release/9.0.1xx-rc2

Started backporting to release/9.0.1xx-rc2: https://github.com/dotnet/sdk/actions/runs/10910656220

@dsplaisted Are you ok to merge this without a test and do you agree this is a good candidate for rc2?

/backport to release/8.0.4xx

@DL444 We would be happy to merge this and we are going to merge this fix in 8.0.400 and 9.0.100 rc2 now but it wont be in any of the future releases until we merge it into 9.0.1xx. Would you mind retargetting this to the release/9.0.1xx branch and adding a test? If you can do that, we'd be happy to merge it.

/backport to release/8.0.4xx

Started backporting to release/8.0.4xx: https://github.com/dotnet/sdk/actions/runs/10910761384

@nagilson backporting to release/8.0.4xx failed, the patch most likely resulted in conflicts:

$ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch

Applying: Fix null reference exception for full global tools update.
Applying: Fix empty ID in message when global package is up to date.
Using index info to reconstruct a base tree...
M	src/Cli/dotnet/commands/dotnet-tool/install/ToolInstallGlobalOrToolPathCommand.cs
Falling back to patching base and 3-way merge...
Auto-merging src/Cli/dotnet/commands/dotnet-tool/install/ToolInstallGlobalOrToolPathCommand.cs
CONFLICT (content): Merge conflict in src/Cli/dotnet/commands/dotnet-tool/install/ToolInstallGlobalOrToolPathCommand.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Patch failed at 0002 Fix empty ID in message when global package is up to date.
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

@nagilson an error occurred while backporting to release/8.0.4xx, please check the run log for details!

Error: git am failed, most likely due to a merge conflict.

@DL444 We would be happy to merge this and we are going to merge this fix in 8.0.400 and 9.0.100 rc2 now but it wont be in any of the future releases until we merge it into 9.0.1xx. Would you mind retargetting this to the release/9.0.1xx branch and adding a test? If you can do that, we'd be happy to merge it.

Bumping this to the top so it's more visible.

For 8.0.4xx, the code is significantly different so some time will have to be spent to backport it. We would also appreciate it if you could backport this fix so it can go into .NET 8. I will try to take a look at this if it's not done when I have finished some other work items.

Would you mind retargetting this to the release/9.0.1xx branch

Sure, I'll see what it takes soon, and I assume it should be relatively straightforward. Backporting to .NET 8 probably can't be done automatically, but it shouldn't be too complicated since I originally developed this fix for .NET 8 and then ported to main.

By the way, just to be sure: by "retargeting this to release/9.0.1xx" you mean creating a separate PR targeting that branch and keeping this one as-is, right? Or should I just change the target branch of this PR and then merging into main will be handled for me automatically?

and adding a test

Tests are currently not added because of testability problems I see, and I may need some guidance to proceed. To reproduce the problem, tools have to be actually installed into the runner's global store. Any mocking using an alternative store location will mask the problematic code path, which is why the issue wasn't caught by existing tests. I'm not sure testing this way is acceptable, or if there are better ways to approach this.

Thank you for your help. You can keep this PR as is and change the merge target. Any changes in 9.0.1xx will automatically get merged into the main branch as well. We appreciate your contribution in improving .NET. 😊 Sorry it took a bit for me to get to the review.

I will go through the process to get it approved in RC 2 as well as 8.0.400 once the PR is created.

To add tests, I see what you mean now and how the original test did not cover this case. I will have to look and see if there is any possible way for us to test this well, so dont worry about that. Off the top of my head, you're right in that the existing test code don't really support testing this scenario.

Related backport PR:
#43516 .NET 9 RC 2
#43550 .NET 8

/azp run

The failures are an unrelated repo issue. Will check in on this again later to rerun

/azp run dotnet-sdk-public-ci

The 8.0.4xx backport merged and was servicing approved - @nagilson was this one approved at the same time?

The changes were previously approved but merging was blocked by irrelevant CI problems.

@DL444 they are PR code approved, but we also have a .NET process requirement to get changes approved by our management because of how close we are to release.

Oh, I misunderstood that. Approval status for this PR wasn't communicated before so I'm unable to tell.

This was approved, thank you for checking.