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 public Architecture enum value for s390x #52909

Closed
uweigand opened this issue May 18, 2021 · 7 comments
Closed

Add public Architecture enum value for s390x #52909

uweigand opened this issue May 18, 2021 · 7 comments
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-System.Runtime.InteropServices
Milestone

Comments

@uweigand
Copy link
Contributor

uweigand commented May 18, 2021

Background and Motivation

We are in the processes of adding support for Linux on the IBM Z architecture (s390x-ibm-linux) to .NET. As part of this effort, the System.Runtime.InteropServices.Architecture enum needs to be extended by a new value to refer to this architecture.

Proposed API

namespace System.Runtime.InteropServices
{
    public enum Architecture
    {
        X86,
        X64,
        Arm,
        Arm64,
        Wasm,
+       S390x,
    }
}

Full implementation is provided here: #52906

Usage Examples

Code can use the existing properties System.Runtime.InteropServices.OSArchitecture and System.Runtime.InteropServices.ProcessArchitecture, which will now return the new value when running on the IBM Z architecture.

Alternative Designs

n/a

Risks

There may be existing code that uses System.Runtime.InteropServices.OSArchitecture and System.Runtime.InteropServices.ProcessArchitecture but does not expect any new return value and will fail if it is returned. I've seen one example of such code in vstest. Such code would have to be updated.

@uweigand uweigand added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label May 18, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.Runtime.InteropServices untriaged New issue has not been triaged by the area owner labels May 18, 2021
@akoeplinger akoeplinger added this to the 6.0.0 milestone May 18, 2021
@akoeplinger akoeplinger self-assigned this May 18, 2021
@akoeplinger akoeplinger added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed untriaged New issue has not been triaged by the area owner api-suggestion Early API idea and discussion, it is NOT ready for implementation labels May 18, 2021
@akoeplinger
Copy link
Member

@terrajobst this should be a quick one for API review.

@AaronRobinsonMSFT AaronRobinsonMSFT added the blocking Marks issues that we want to fast track in order to unblock other important work label May 18, 2021
@iSazonov
Copy link
Contributor

Also there is System.Reflection.ProcessorArchitecture.

@bartonjs
Copy link
Member

bartonjs commented May 19, 2021

Video

Looks good as proposed

namespace System.Runtime.InteropServices
{
    public enum Architecture
    {
        X86,
        X64,
        Arm,
        Arm64,
        Wasm,
+       S390x,
    }
}

@bartonjs bartonjs added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation blocking Marks issues that we want to fast track in order to unblock other important work labels May 19, 2021
@uweigand
Copy link
Contributor Author

Also there is System.Reflection.ProcessorArchitecture.

This is related to the assembly file format, right? We currently don't have a processor-specific file format since we don't (yet) support AOT. Once we add support for that, I guess we'd have to allocate a new System.Reflection.ProcessorArchitecture (as well as System.Reflection.ImageFileMachine I think). I'll create a new API issue for those when we get there.

@jkotas
Copy link
Member

jkotas commented May 19, 2021

System.Reflection.ProcessorArchitecture.

It is for architecture specific assemblies that are really only a thing on Windows. AOT should not require it.

@jkotas
Copy link
Member

jkotas commented May 19, 2021

Fixed by #52906

@jkotas jkotas closed this as completed May 19, 2021
@uweigand
Copy link
Contributor Author

System.Reflection.ProcessorArchitecture.

It is for architecture specific assemblies that are really only a thing on Windows. AOT should not require it.

Thanks for the clarification!

@ghost ghost locked as resolved and limited conversation to collaborators Jun 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Runtime.InteropServices
Projects
None yet
Development

No branches or pull requests

6 participants