arcade icon indicating copy to clipboard operation
arcade copied to clipboard

Add option to install dotnet at .dotnet/$rid

Open am11 opened this issue 1 year ago • 8 comments

It is enabled by setting DOTNET_USE_ARCH_IN_INSTALL_PATH=1.

Fix https://github.com/dotnet/arcade/issues/14751

cc @akoeplinger

am11 avatar Jul 02 '24 09:07 am11

Honestly I'm not in favor of this change. While the current layout might not be correct in all cases, it existed for years and works. I fear that many repositories (150+ repos depend on Arcade) expect the dotnet installation to exist in the current location.

Another example is https://github.com/dotnet/arcade/blob/48f3eafb2fa9dddb1ca655b9ea96ebf4b5a2ba0a/src/Microsoft.DotNet.Arcade.Sdk/tools/RepoLayout.props#L24 which would now be wrong.

ViktorHofer avatar Jul 02 '24 11:07 ViktorHofer

This is enabled with repo sets DOTNET_USE_ARCH_IN_INSTALL_PATH, as discussed in the issue.

We also have DOTNET_INSTALL_DIR support, so it's not the only different path in these scripts.

am11 avatar Jul 02 '24 11:07 am11

Right, but I question the benefit of adding yet another path. I'm not sold that this is needed. Which repositories would set this?

ViktorHofer avatar Jul 02 '24 12:07 ViktorHofer

This enabler is mainly for runtime where we do a lot of platform-specific builds, I'm testing it https://github.com/dotnet/runtime/pull/104295 to see how CI reacts.

BTW, we don't have to set it repo-wide either. Just having this facility enables the consumer (either the repo itself, or someone building a repo) to use the derived RID in installation path that works with dotnet.sh/cmd.

am11 avatar Jul 02 '24 13:07 am11

After thinking through this some more I'm also not sure anymore this is the best approach since it's only really useful if we enable it by default and we probably don't have the resources to fix all the cases where we assume the current path.

I had another simpler idea though: we should be able to detect whether we have the right dotnet by running e.g. .dotnet/dotnet --version in InitializeDotNetCli and if it doesn't work we could delete and redownload it. That'd leave the path unchanged. @ViktorHofer what do you think?

akoeplinger avatar Jul 03 '24 11:07 akoeplinger

Re-downloading the right RID can be nice - when jumping locally between Windows and WSL, you need to keep nuking the .dotnet dir (dotnet--version does not even run though).

Alternatively, why not just adding an optional argument with the destination dir and override it in runtime's scenarios?

premun avatar Jul 18 '24 13:07 premun

when jumping locally between Windows and WSL, you need to keep nuking

You can work around this by installing the right .NET SDK in a global location on both systems. These scripts are able to auto-detect that you have the right version available and skip downloading the repo-local copy.

jkotas avatar Jul 24 '24 05:07 jkotas

Another example is

https://github.com/dotnet/arcade/blob/48f3eafb2fa9dddb1ca655b9ea96ebf4b5a2ba0a/src/Microsoft.DotNet.Arcade.Sdk/tools/RepoLayout.props#L24

It will not fail with the current state of this PR because of line 22. DOTNET_INSTALL_DIR is set by the script and we don't hit this path when tools.ps1/sh are involved. If we have another example where DOTNET_INSTALL_DIR is not honored, it is likely a bug which that needs fixing. I covered the ones in runtime https://github.com/dotnet/runtime/pull/104295.

IMO we should take this change and fix like those few places taking wrong assumptions and there aren't that many. I think this change is even less riskier than any other change we make in arcade which can break downstream, in that we have it behind an enviornment variable, which we can remove if we want after updating all consumer repos to use the new scheme.

am11 avatar Jul 24 '24 06:07 am11

@ViktorHofer, any further thoughts on this? Yesterday, I was building dotnet/winforms on win-arm64 by invoking build.cmd on powershell arm64. Before the package restore begins, it installed:

  • win-arm64 dotnet version from globa.json at .dotnet
  • latest daily version of win-x64 dotnet at .dotnet
  • latest daily version of win-86 dotnet at .dotnet/x86 <- this is similar to what the PR is achieving (except it'd be /win-86 instead of /x86 and would apply for all three archs not just x86..)

as I was fixing the compilation issue with latest roslyn, all three versions were the same (global.json had latest version). Consequently, .dotnet\host\fxr\10.0.0-alpha.1.24560.1\hostfxr.dll was x86_64 because it installs second and overwrites arm64 one, which failed to load in the arm64 process. Workaround was to manually unzip arm64 package one more time before re-invoking build.cmd.

Separate directories is not new in dotnet install. There are multiple scenarios discussed where it'd help (docker, wsl, winform). If we are considering it we should take this change at the beginning of 10.0 cycle and update the downstream repos gradually with trivial changes.

am11 avatar Nov 14 '24 10:11 am11