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

Obsolete thumbtacked AssemblyName properties #59061

Closed
Tracked by #57207
jkotas opened this issue Sep 13, 2021 · 6 comments · Fixed by #59522
Closed
Tracked by #57207

Obsolete thumbtacked AssemblyName properties #59061

jkotas opened this issue Sep 13, 2021 · 6 comments · Fixed by #59522
Labels
api-approved API was approved in API review, it can be implemented area-System.Reflection
Milestone

Comments

@jkotas
Copy link
Member

jkotas commented Sep 13, 2021

Background and motivation

AssemblyName has number of properties that are not proper part of assembly name. They do not not roundtrip through AssemblyName string representation and they are ignored by the assembly loader in .NET Core. They should be obsoleted.

API Proposal

    public sealed partial class AssemblyName
    {
        public System.Configuration.Assemblies.AssemblyHashAlgorithm HashAlgorithm
        {
+           [Obsolete("AssemblyName.HashAlgorithm has been deprecated and is not supported.")]
            get;
+           [Obsolete("AssemblyName.HashAlgorithm has been deprecated and is not supported.")]
            set;
        }
        public System.Reflection.ProcessorArchitecture ProcessorArchitecture
        {
+           [Obsolete("AssemblyName.ProcessorArchitecture has been deprecated and is not supported.")]
            get;
+           [Obsolete("AssemblyName.ProcessorArchitecture has been deprecated and is not supported.")]
            set;
        }
        public System.Configuration.Assemblies.AssemblyVersionCompatibility VersionCompatibility
        {
+           [Obsolete("AssemblyName.VersionCompatibility has been deprecated and is not supported.")]
            get;
+           [Obsolete("AssemblyName.VersionCompatibility has been deprecated and is not supported.")]
            set;
        }
    }

API Usage

var an = new AssemblyName("System.Diagnostics.Process");
an.ProcessorArchitecture = ProcessorArchitecture.IA64; // IA64 is Itanium
Console.WriteLine(an.FullName); // IA64 is not included in the full name
Console.WriteLine(Assembly.Load(an)); // Works fine, requested ProcessorArchitecture is ignored
@jkotas jkotas added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Sep 13, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Sep 13, 2021
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@jkotas jkotas changed the title [API Proposal]: Obsolete thumbtacked AssemblyName properties Obsolete thumbtacked AssemblyName properties Sep 13, 2021
@terrajobst terrajobst added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation untriaged New issue has not been triaged by the area owner labels Sep 13, 2021
@jeffhandley jeffhandley added this to the 7.0.0 milestone Sep 15, 2021
@vitek-karas
Copy link
Member

Would there be some replacement for functionality like:

Assembly asm = Assembly.GetAssemblyName(path);
Console.WriteLine(asm.ProcessorArchitecture);

Specifically the architecture can be rather useful to get like this. The GetAssemblyName effectively exposes the PE/managed header parser from the runtime to the managed world. The code above does not load the assembly, and it's relatively efficient.

Just trying to add context for cases where the obsoleted properties can be useful.

@jkotas
Copy link
Member Author

jkotas commented Sep 19, 2021

Assembly asm = Assembly.GetAssemblyName(path);
Console.WriteLine(asm.ProcessorArchitecture);

Use System.Reflection.Metadata. System.Reflection.Metadata is full featured PE parser. We have been recommending to use System.Reflection.Metadata for all situations where Assembly.GetAssemblyName is missing features or where it is not fast enough, e.g. #21785 (comment)

@bartonjs
Copy link
Member

bartonjs commented Sep 21, 2021

Video

Looks good as proposed (though we moved the obsoletions to the properties instead of the accessors

    public sealed partial class AssemblyName
    {
+       [Obsolete("AssemblyName.HashAlgorithm has been deprecated and is not supported.", DiagnosticId = next available)]
        public System.Configuration.Assemblies.AssemblyHashAlgorithm HashAlgorithm { get; set; }
+       [Obsolete("AssemblyName.ProcessorArchitecture has been deprecated and is not supported.", DiagnosticId = same as above)]
        public System.Reflection.ProcessorArchitecture ProcessorArchitecture { get; set; }
+       [Obsolete("AssemblyName.VersionCompatibility has been deprecated and is not supported.", DiagnosticId = same as above)]
        public System.Configuration.Assemblies.AssemblyVersionCompatibility VersionCompatibility { get; set;}
    }

@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 labels Sep 21, 2021
i3arnon added a commit to i3arnon/runtime that referenced this issue Sep 23, 2021
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Sep 23, 2021
jeffhandley added a commit that referenced this issue Nov 16, 2021
* Obsolete thumbtacked AssemblyName properties

Fix #59061

* Ignore obsoletion

* Fix pragma

* Merge the AssemblyName member obsoletions into a single diagnostic id

* Fix pragma to use updated diagnostic id

* Suppress SYSLIB0037 in reflection tests

* Suppress SYSLIB0037 warnings

Co-authored-by: Jeff Handley <[email protected]>
Co-authored-by: Jeff Handley <[email protected]>
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Nov 16, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Dec 16, 2021
@Forgind
Copy link
Member

Forgind commented Aug 16, 2022

@jkotas, I had some trouble figuring out how to get architecture out of System.Reflection.Metadata and had trouble finding a relevant example on GitHub. Could you link to somewhere someone did it? Thanks!

@jkotas
Copy link
Member Author

jkotas commented Aug 16, 2022

@Forgind We do not respond to comments on old closed PRs and issues. If it is still a problem, open a new discussion or issue

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.Reflection
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

6 participants