sdk icon indicating copy to clipboard operation
sdk copied to clipboard

Use architecture specific DOTNET_ROOT variable name

Open tmat opened this issue 1 year ago • 14 comments

Corrects parsing of runtime identifier added by https://github.com/dotnet/sdk/pull/19860 and reuses the code in dotnet watch to do the same.

Other than parsing the rid the behavior of https://github.com/dotnet/sdk/pull/19860 is preserved. However, it doesn't seem consistent with https://github.com/dotnet/sdk/issues/19743#issuecomment-901404323 in the case the architecture specified in rid is unknown. The implementation in this case sets DOTNET_ROOT variable, while it seems it shouldn't since an unknown architecture doesn't match the current architecture.

Implements https://github.com/dotnet/aspnetcore/issues/35793

tmat avatar Aug 30 '22 18:08 tmat

/azp run

tmat avatar Sep 06 '22 15:09 tmat

Azure Pipelines successfully started running 1 pipeline(s).

azure-pipelines[bot] avatar Sep 06 '22 15:09 azure-pipelines[bot]

/azp run

tmat avatar Sep 09 '22 05:09 tmat

Azure Pipelines successfully started running 1 pipeline(s).

azure-pipelines[bot] avatar Sep 09 '22 05:09 azure-pipelines[bot]

@mateoatr @vitek-karas PTAL

tmat avatar Sep 19 '22 20:09 tmat

However, it doesn't seem consistent with https://github.com/dotnet/sdk/issues/19743#issuecomment-901404323 in the case the architecture specified in rid is unknown. The implementation in this case sets DOTNET_ROOT variable, while it seems it shouldn't since an unknown architecture doesn't match the current architecture.

That comment was mainly about the case where both architectures are known and the SDK architecture is different from the target architecture - in that case SDK should not set anything, since it doesn't know anything about the target's architecture situation on the machine - thus it should fallback to existing state on the machine to figure it out. The case where the target architecture is unknown typically means in the "dotnet run" case that it was not specified (I think that can happen), in which case the desire for "dotnet run" is to run the app with the SDK environment - the app should default to SDK's RID in such case basically (which should be the behavior of "dotnet build" as well). I don't know if this is something which actually happens, since I think the msbuild targets will eventually default the target RID to the SDK RID, and thus the command should see the defaulted RID, but maybe it doesn't happen always.

I don't know what is the behavior for "dotnet watch" in this case. The logic should be:

  • If the target is to run on the same architecture as the SDK -> modify the environment to use the SDK's runtime location
  • If the target is to run on a different architecture -> don't modify anything, as the SDK has no clue what that architecture situation is.

I don't feel strongly about the behavior when the target architecture is unknown (I think and hope that it's rare anyway), but since "dotnet run" already defaults to "assume SDK's architecture", then I guess "dotnet watch" should be consistent.

vitek-karas avatar Sep 19 '22 22:09 vitek-karas

@MarcoRossignoli as an FYI, since he implemented (and maintains) a similar logic in "dotnet test".

vitek-karas avatar Sep 19 '22 22:09 vitek-karas

@dotnet/dotnet-cli PTAL

tmat avatar Sep 20 '22 18:09 tmat

@tmat the linked issue is from a year ago and listed for .net 8. Is this not too risky to take in rc2 at this time?

marcpopMSFT avatar Sep 20 '22 22:09 marcpopMSFT

@marcpopMSFT I'm fine moving to .NET 8. Note though that this also corrects the parsing of RID in dotnet run originally implemented in https://github.com/dotnet/sdk/pull/19860.

tmat avatar Sep 20 '22 22:09 tmat

cc: @Evangelink

MarcoRossignoli avatar Sep 21 '22 15:09 MarcoRossignoli

Moving to main.

tmat avatar Sep 21 '22 19:09 tmat

/azp run

tmat avatar Sep 28 '22 20:09 tmat

Azure Pipelines successfully started running 1 pipeline(s).

azure-pipelines[bot] avatar Sep 28 '22 20:09 azure-pipelines[bot]

@dsplaisted Rebased to 7.0.2xx

tmat avatar Oct 10 '22 20:10 tmat

/azp run

tmat avatar Oct 14 '22 17:10 tmat

Azure Pipelines successfully started running 1 pipeline(s).

azure-pipelines[bot] avatar Oct 14 '22 17:10 azure-pipelines[bot]

/azp run

tmat avatar Oct 27 '22 21:10 tmat

Azure Pipelines successfully started running 1 pipeline(s).

azure-pipelines[bot] avatar Oct 27 '22 21:10 azure-pipelines[bot]