Skip to content

Commit

Permalink
Check that SDK and app archs match before setting DOTNET_ROOT when …
Browse files Browse the repository at this point in the history 
…using `dotnet run` (#19860)

* Check that dotnet arch and app's arch match before setting DOTNET_ROOT

* Check RuntimeIdentifier prop

* Use new DOTNET_ROOT format
Update tests

* Remove spaces in prop
Allow test asset copy

* Update test

* Update how we retrieve enum value name
Keep behavior if there is no host to retrieve target arch from

* Update Program.cs

* Use ToUpperInvariant

* Set ProcessArchitecture as targetArchitecture might be null
  • Loading branch information
@mateoatr
mateoatr authored Aug 24, 2021
1 parent 36cc521 commit 36e9be0
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 17 deletions.
5 changes: 4 additions & 1 deletion src/Assets/TestProjects/TestAppEchoDotnetRoot/Program.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,18 @@
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System;
using System.Runtime.InteropServices;

namespace ConsoleApplication
{
public class Program
{
public static void Main(string[] args)
{
var processArchitecture = $"DOTNET_ROOT_{Enum.GetName(typeof(Architecture), RuntimeInformation.ProcessArchitecture)}";
Console.WriteLine($"DOTNET_ROOT='{Environment.GetEnvironmentVariable("DOTNET_ROOT", EnvironmentVariableTarget.Process)}';" +
$"DOTNET_ROOT(x86)='{Environment.GetEnvironmentVariable("DOTNET_ROOT(x86)", EnvironmentVariableTarget.Process)}'");
$"DOTNET_ROOT(x86)='{Environment.GetEnvironmentVariable("DOTNET_ROOT(x86)", EnvironmentVariableTarget.Process)}';" +
$"{processArchitecture}='{Environment.GetEnvironmentVariable(processArchitecture)}'");
}
}
}
2 changes: 1 addition & 1 deletion src/Assets/TestProjects/TestAppEchoDotnetRoot/TestAppEchoDotnetRoot.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), testAsset.props))\testAsset.props" />

<PropertyGroup>
<TargetFramework>netcoreapp3.1</TargetFramework>
<TargetFramework>net5.0</TargetFramework>
<OutputType>Exe</OutputType>
<UseAppHost>false</UseAppHost>
</PropertyGroup>
Expand Down
40 changes: 37 additions & 3 deletions src/Cli/dotnet/commands/dotnet-run/RunCommand.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Runtime.InteropServices;
using Microsoft.Build.Execution;
using Microsoft.Build.Exceptions;
using Microsoft.DotNet.Cli.Utils;
Expand All @@ -25,6 +26,7 @@ public partial class RunCommand
public bool Interactive { get; private set; }
public IEnumerable<string> RestoreArgs { get; private set; }

private Version Version6_0 = new Version(6, 0);
private bool ShouldBuild => !NoBuild;
private bool HasQuietVerbosity =>
RestoreArgs.All(arg => !arg.StartsWith("-verbosity:", StringComparison.Ordinal) ||
Expand Down Expand Up @@ -254,10 +256,21 @@ private ICommand GetTargetCommand()
var command = CommandFactoryUsingResolver.Create(commandSpec)
.WorkingDirectory(runWorkingDirectory);

var rootVariableName = Environment.Is64BitProcess ? "DOTNET_ROOT" : "DOTNET_ROOT(x86)";
if (Environment.GetEnvironmentVariable(rootVariableName) == null)
if (((TryGetTargetArchitecture(project.GetPropertyValue("RuntimeIdentifier"), out var targetArchitecture) ||
TryGetTargetArchitecture(project.GetPropertyValue("DefaultAppHostRuntimeIdentifier"), out targetArchitecture)) &&
targetArchitecture == RuntimeInformation.ProcessArchitecture) || targetArchitecture == null)
{
command.EnvironmentVariable(rootVariableName, Path.GetDirectoryName(new Muxer().MuxerPath));
var rootVariableName = Environment.Is64BitProcess ? "DOTNET_ROOT" : "DOTNET_ROOT(x86)";
string targetFrameworkVersion = project.GetPropertyValue("TargetFrameworkVersion");
if (!string.IsNullOrEmpty(targetFrameworkVersion) && Version.Parse(targetFrameworkVersion.AsSpan(1)) >= Version6_0)
{
rootVariableName = $"DOTNET_ROOT_{RuntimeInformation.ProcessArchitecture.ToString().ToUpperInvariant()}";
}

if (Environment.GetEnvironmentVariable(rootVariableName) == null)
{
command.EnvironmentVariable(rootVariableName, Path.GetDirectoryName(new Muxer().MuxerPath));
}
}

return command;
Expand Down Expand Up @@ -311,5 +324,26 @@ private static string FindSingleProjectInDirectory(string directory)

return projectFiles[0];
}

private static bool TryGetTargetArchitecture(string runtimeIdentifier, out Architecture? targetArchitecture)
{
targetArchitecture = null;
int separator = runtimeIdentifier.LastIndexOf("-");
if (separator < 0)
{
return false;
}

targetArchitecture = runtimeIdentifier.Substring(separator + 1).ToLowerInvariant() switch
{
"arm" => Architecture.Arm,
"arm64" => Architecture.Arm64,
"x64" => Architecture.X64,
"x86" => Architecture.X86,
_ => null
};

return targetArchitecture != null;
}
}
}
34 changes: 22 additions & 12 deletions src/Tests/dotnet-run.Tests/GivenDotnetRootEnv.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System;
using System.IO;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using FluentAssertions;
using Microsoft.NET.TestFramework;
using Microsoft.NET.TestFramework.Assertions;
Expand All @@ -15,17 +15,21 @@ namespace Microsoft.DotNet.Cli.Run.Tests
{
public class GivenDotnetRootEnv : SdkTest
{
private static Version Version6_0 = new Version(6, 0);

public GivenDotnetRootEnv(ITestOutputHelper log) : base(log)
{
}

[Fact]
public void ItShouldSetDotnetRootToDirectoryOfMuxer()
[Theory]
[InlineData("net5.0")]
[InlineData("net6.0")]
public void ItShouldSetDotnetRootToDirectoryOfMuxer(string targetFramework)
{
string expectDotnetRoot = TestContext.Current.ToolsetUnderTest.DotNetRoot;
string expectOutput = GetExpectOutput(expectDotnetRoot);
string expectOutput = GetExpectOutput(expectDotnetRoot, targetFramework);

var projectRoot = SetupDotnetRootEchoProject();
var projectRoot = SetupDotnetRootEchoProject(null, targetFramework);

var runCommand = new DotnetCommand(Log, "run")
.WithWorkingDirectory(projectRoot);
Expand Down Expand Up @@ -65,31 +69,37 @@ public void WhenDotnetRootIsSetItShouldSetDotnetRootToDirectoryOfMuxer()
.And.HaveStdOutContaining(GetExpectOutput(expectDotnetRoot));
}

private string SetupDotnetRootEchoProject([CallerMemberName] string callingMethod = null)
private string SetupDotnetRootEchoProject([CallerMemberName] string callingMethod = null, string targetFramework = null)
{
var testAsset = _testAssetsManager
.CopyTestAsset("TestAppEchoDotnetRoot", callingMethod)
.CopyTestAsset("TestAppEchoDotnetRoot", callingMethod, allowCopyIfPresent: true)
.WithSource()
.WithTargetFrameworkOrFrameworks(targetFramework ?? null, false)
.Restore(Log);

new BuildCommand(testAsset)
.Execute()
.Execute($"{(!string.IsNullOrEmpty(targetFramework) ? "/p:TargetFramework=" + targetFramework : string.Empty)}")
.Should()
.Pass();

return testAsset.Path;
}

private static string GetExpectOutput(string expectDotnetRoot)
private static string GetExpectOutput(string expectDotnetRoot, string targetFramework = null)
{
string expectOutput;
if (Environment.Is64BitProcess)
string processArchitecture = RuntimeInformation.ProcessArchitecture.ToString().ToUpperInvariant();
if (!string.IsNullOrEmpty(targetFramework) && Version.Parse(targetFramework.AsSpan(3)) >= Version6_0)
{
expectOutput = $"DOTNET_ROOT='';DOTNET_ROOT(x86)='';DOTNET_ROOT_{processArchitecture}='{expectDotnetRoot}'";
}
else if (Environment.Is64BitProcess)
{
expectOutput = @$"DOTNET_ROOT='{expectDotnetRoot}';DOTNET_ROOT(x86)=''";
expectOutput = @$"DOTNET_ROOT='{expectDotnetRoot}';DOTNET_ROOT(x86)='';DOTNET_ROOT_{processArchitecture}=''";
}
else
{
expectOutput = @$"DOTNET_ROOT='';DOTNET_ROOT(x86)='{expectDotnetRoot}'";
expectOutput = @$"DOTNET_ROOT='';DOTNET_ROOT(x86)='{expectDotnetRoot}';DOTNET_ROOT_{processArchitecture}=''";
}

return expectOutput;
Expand Down

0 comments on commit 36e9be0

Please sign in to comment.