Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for ppc64le processor architecture #4028

Merged
merged 4 commits into from
Sep 30, 2022

Conversation

Sapana-Khemkar
Copy link
Contributor

Description

We have added support for the ppc64le processor architecture to the dotnet runtime. For that we have added a new value for the System.Runtime.InteropServices.Architecture enum: dotnet/runtime#67428
Code in Microsoft.VisualStudio.TestPlatform.PlatformAbstractions does not recognize this new value. This patch adds a new value to Microsoft.VisualStudio.TestPlatform.PlatformAbstractions.PlatformArchitecture.

Related issue

NA

@Sapana-Khemkar
Copy link
Contributor Author

Sapana-Khemkar commented Sep 27, 2022 via email

@Sapana-Khemkar Sapana-Khemkar marked this pull request as ready for review September 27, 2022 09:29
@@ -13,4 +13,5 @@ public enum PlatformArchitecture
ARM,
ARM64,
S390x,
PPC64le,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe name this Ppc64le to keep it consistent with dotnet/runtime?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree. Will change

@@ -28,6 +28,7 @@ public PlatformArchitecture Architecture
// preview 6 or later, so use the numerical value for now.
// case System.Runtime.InteropServices.Architecture.S390x:
(Architecture)5 => PlatformArchitecture.S390x,
(Architecture)6 => PlatformArchitecture.PPC64le,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this value correct? Shouldn't this be 8? I am looking at https://github.com/dotnet/runtime/blob/main/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Architecture.cs, and there's LoongArch64 and Armv6 between S390x and PPC64le.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes you are right. Thanks for catching that. I matched it with enum in VSTest code so counted it 6. Will correct.

…stractions.PlatformArchitecture.Ppc64le

and Microsoft.VisualStudio.TestPlatform.ObjectModel.Architecture.Ppc64le in Microsoft.TestPlatform.ObjectModel
and /Microsoft.TestPlatform.PlatformAbstractions PublicAPI.Shipped.txt file.
@leecow
Copy link
Member

leecow commented Sep 28, 2022

@nohwnd - could you have a look so that we can queue this up for .NET 7 GA review?

cc @crummel

@nohwnd nohwnd merged commit 255e975 into microsoft:main Sep 30, 2022
nohwnd pushed a commit to nohwnd/vstest that referenced this pull request Sep 30, 2022
@Sapana-Khemkar
Copy link
Contributor Author

@nohwnd Thanks for merging. Where can I find daily build artifacts for VSTest repo? Azure Link provided in readme is not reachable. Can you please help?

@nohwnd
Copy link
Member

nohwnd commented Sep 30, 2022

@Sapana-Khemkar there is some issue with our token for publishing into our public feed, we are trying to fix it so you can see the preview package that would go into net8 and possibly try it.

@Sapana-Khemkar
Copy link
Contributor Author

@Sapana-Khemkar there is some issue with our token for publishing into our public feed, we are trying to fix it so you can see the preview package that would go into net8 and possibly try it.

Noted. Thanks.

@Sapana-Khemkar
Copy link
Contributor Author

@nohwnd It seems Azure link is yet not up. Is there anyway to download daily build binary for testing?

@janani66
Copy link

janani66 commented Oct 5, 2022

@nohwnd - Where can we download the preview package that would go into net8? Just trying to access a binary build with the fix for #4028 for testing purposes.

@nohwnd
Copy link
Member

nohwnd commented Oct 7, 2022

@Sapana-Khemkar @janani66 Just using the latest dotnet installer for net8-alpha should be the easiest way to acquire it. The latest successful merge from vstest to dotnet/sdk happened 3 days ago, and from sdk to installer 2 days ago. So it should be safely there. But please double check the version printed in the console when you use dotnet test, the number is a date, you want October 1st or newer.

As for the nuget package we insert into dotnet/sdk that one is here: https://dev.azure.com/dnceng/public/_artifacts/feed/test-tools/NuGet/Microsoft.TestPlatform.CLI/overview/17.5.0-preview-20221007-01

If you want redistributable .NET Framework package you should choose Microsoft.TestPlatform (without the .CLI) instead. That is what CI servers use to run tests, when not using dotnet test.

This is the feed we push it into: https://dev.azure.com/dnceng/public/_artifacts/feed/test-tools

@nohwnd
Copy link
Member

nohwnd commented Oct 7, 2022

@janani66 Or maybe you want Microsoft.NET.Test.SDK which is what you install in your .NET project, that package is here https://dev.azure.com/dnceng/public/_artifacts/feed/test-tools/NuGet/Microsoft.NET.Test.Sdk/overview/17.5.0-preview-20221007-01 on the same feed.

@janani66
Copy link

janani66 commented Oct 11, 2022

@nohwnd The test with the 17.5.0-preview-20221007-01 failed to recognize ppc64le as a supportedArchitecture. Has something changed In .NET7 w.r.t. VSTest integration with .NET .


ubuntu@dotnet1:~/janani$ dotnet --info
.NET SDK:
Version: 7.0.100-preview.7.22377.5
Commit: ba310d9309

Runtime Environment:
OS Name: ubuntu
OS Version: 20.04
OS Platform: Linux
RID: linux-ppc64le
Base Path: /home/ubuntu/sapana/preview7/output/.dotnet/sdk/7.0.100-preview.7.22377.5/

Host:
Version: 7.0.0-preview.7.22375.6
Architecture: ppc64le
Commit: N/A

.NET SDKs installed:
7.0.100-preview.7.22377.5 [/home/ubuntu/sapana/preview7/output/.dotnet/sdk]

.NET runtimes installed:
Microsoft.AspNetCore.App 7.0.0-preview.7.22376.6 [/home/ubuntu/sapana/preview7/output/.dotnet/shared/Microsoft.AspNetCore.App]
Microsoft.NETCore.App 7.0.0-preview.7.22375.6 [/home/ubuntu/sapana/preview7/output/.dotnet/shared/Microsoft.NETCore.App]

Other architectures found:
None

Environment variables:
DOTNET_ROOT [/home/ubuntu/sapana/preview7/output/.dotnet]

global.json file:
Not found

To Reproduce the Problem

  1. Create a sample xunit program
    dotnet new xunit

  2. Modify the csproj to override the Microsoft.NET.Test.Sdk page

<Project Sdk="Microsoft.NET.Sdk">
  
  <PropertyGroup>
    <TargetFramework>net7.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>

    <IsPackable>false</IsPackable>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.5.0-preview-20221007-01" />
    <PackageReference Include="xunit" Version="2.4.1" />
    <PackageReference Include="xunit.runner.visualstudio" Version="2.4.3">
      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
      <PrivateAssets>all</PrivateAssets>
    </PackageReference>
    <PackageReference Include="coverlet.collector" Version="3.1.2">
      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
      <PrivateAssets>all</PrivateAssets>
    </PackageReference>
  </ItemGroup>

</Project>
  1. Run the program
    dotnet test

This gives error:
$ dotnet test
Determining projects to restore...
All projects are up-to-date for restore.
/home/ubuntu/sapana/preview7/output/.dotnet/sdk/7.0.100-preview.7.22377.5/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.RuntimeIdentifierInference.targets(219,5): message NETSDK1057: You are using a preview version of .NET. See: https://aka.ms/dotnet-support-policy [/home/ubuntu/janani/vstest/janani.csproj]
janani -> /home/ubuntu/janani/vstest/bin/Debug/net7.0/janani.dll
Test run for /home/ubuntu/janani/vstest/bin/Debug/net7.0/janani.dll (.NETCoreApp,Version=v7.0)

Unhandled Exception:
System.NotSupportedException: Specified method is not supported.
at Microsoft.VisualStudio.TestPlatform.PlatformAbstractions.ProcessHelper.GetCurrentProcessArchitecture()
at Microsoft.VisualStudio.TestPlatform.CommandLine.Executor.PrintSplashScreen(Boolean isDiag)
at Microsoft.VisualStudio.TestPlatform.CommandLine.Executor.Execute(String[] args)
at Microsoft.VisualStudio.TestPlatform.CommandLine.Program.Run(String[] args, UiLanguageOverride uiLanguageOverride)
at Microsoft.VisualStudio.TestPlatform.CommandLine.Program.Main(String[] args)
[ERROR] FATAL UNHANDLED EXCEPTION: System.NotSupportedException: Specified method is not supported.
at Microsoft.VisualStudio.TestPlatform.PlatformAbstractions.ProcessHelper.GetCurrentProcessArchitecture()
at Microsoft.VisualStudio.TestPlatform.CommandLine.Executor.PrintSplashScreen(Boolean isDiag)
at Microsoft.VisualStudio.TestPlatform.CommandLine.Executor.Execute(String[] args)
at Microsoft.VisualStudio.TestPlatform.CommandLine.Program.Run(String[] args, UiLanguageOverride uiLanguageOverride)
at Microsoft.VisualStudio.TestPlatform.CommandLine.Program.Main(String[] args)

@janani66
Copy link

janani66 commented Oct 11, 2022

Running the code on ppc64le I get:

using System.Runtime.InteropServices;

foreach (var name in Enum.GetNames())
{
    Architecture value = Enum.Parse(name);
    Console.WriteLine($"{name} is {value.ToString("D")}");
}

Console.WriteLine($"OS Arch: {RuntimeInformation.OSArchitecture}");
Console.WriteLine($"Processor Arch: {RuntimeInformation.ProcessArchitecture}");

gives me:

$ dotnet run
X86 is 0
X64 is 1
Arm is 2
Arm64 is 3
Wasm is 4
S390x is 5
LoongArch64 is 6
Armv6 is 7
Ppc64le is 8
OS Arch: Ppc64le
Processor Arch: Ppc64le

@MarcoRossignoli
Copy link
Contributor

MarcoRossignoli commented Oct 12, 2022

@janani66 the new enum won't be shipped into 7.0(we're in stabilization feature phase) you should try with the preview SDK 8.0 that you download from here https://github.com/dotnet/installer#table

@janani66
Copy link

Looks like we also need the fix : #4066

@janani66
Copy link

janani66 commented Oct 24, 2022

I was able to test with the 17.5.0-preview-20221024-01 build published on https://pkgs.dev.azure.com/dnceng/public/_packaging/test-tools/nuget/v3/index.json. and the dotnet xunit test passed.

A couple of comments/questions:

it is not able to find "Newtonsoft.Json": "13.0.1".

Once I remove the "clear" from the nuget.config, it found NewtonSoft.Json from nuget.org. Is there a reason why the example nuget.config on the dnceng feed shows the clear in the example nuget.config?

i.e.

<?xml version="1.0" encoding="utf-8"?>
<configuration>
  <packageSources>
    <clear />
    <add key="test-tools" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/test-tools/nuget/v3/index.json" />
  </packageSources>
</configuration>

@MarcoRossignoli
Copy link
Contributor

Is there a reason why the example nuget.config on the dnceng feed shows the clear in the example nuget.config?

Because our test-tools feed doesn't have any upstream and so there you can find only packages released by us and not for instance Newtonsoft.Json": "13.0.1

Just wanted to point out that on this page https://dev.azure.com/dnceng/public/_artifacts/feed/test-tools/connect/dotnet, the {0}/{1}/{2} variables are not substituted for nuget.config {0}/.csproj{1}/.sln{{2}

this is not clear to me...can you elaborate? I don't see any {0}/{1}/{2} in that page.

@janani66
Copy link

janani66 commented Nov 2, 2022

Just wanted to point out that on this page https://dev.azure.com/dnceng/public/_artifacts/feed/test-tools/connect/dotnet, the {0}/{1}/{2} variables are not substituted for nuget.config {0}/.csproj{1}/.sln{{2}

this is not clear to me...can you elaborate? I don't see any {0}/{1}/{2} in that page.

On the https://dev.azure.com/dnceng/public/_artifacts/feed/test-tools/connect/dotnet page, I see under "Project setup"

Add a {0} file to your project, in the same folder as your {1} or {2} file

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants