Runtime: Consider adding GetAssemblyName() : AssemblyName to AssemblyDefinition and AssemblyReference

Created on 3 Nov 2016  路  11Comments  路  Source: dotnet/runtime

Constructing System.Reflection.AssemblyName from metadata is a bit tricky. Providing an easy to use helper would avoid common pitfalls and reduce code duplication. See e.g. http://source.roslyn.io/#Microsoft.CodeAnalysis/InternalUtilities/AssemblyIdentityUtils.cs

api-approved area-System.Reflection.Metadata up-for-grabs

Most helpful comment

I believe what @tmat meant was not to add an AssemblyName getter, per comment https://github.com/dotnet/corefx/issues/13295#issuecomment-260037855 he meant to add a public method that returns a System.Reflection.AssemblyName object with the required metadata such as PublicKey, Flags, Culture, Version, etc. Because from the AssemblyDefinition itself to build that object requires a little bit of work like in the link he posted: http://source.roslyn.io/#Microsoft.CodeAnalysis/InternalUtilities/AssemblyIdentityUtils.cs

With this public API we would avoid a lot of code duplication trying to extract the metadata from multiple places from the handles. As you can see the AssemblyDefinition.Name is a StringHandle that needs to be passed to the MetadataReader to get the actual string, so we want to avoid code duplication to convert those handles to actual values with this new APIs.

All 11 comments

Approved (cc: @terrajobst @weshaggard should be non-controversial).
We need someone to implement it - it will be medium complex, the code is more or less in Roslyn.

I've no objection to the concept but what's the actual API that's being approved here? We should have at least an outline.

It's in the title :)

https://source.dot.net/#System.Reflection.Metadata/System/Reflection/Metadata/TypeSystem/AssemblyDefinition.cs:

``` C#
struct AssemblyDefinition
{
...
public AssemblyName GetAssemblyName() {}
...
}

https://source.dot.net/#System.Reflection.Metadata/System/Reflection/Metadata/TypeSystem/AssemblyReference.cs:

``` C#
struct AssemblyReference
{  
   ...
   public AssemblyName GetAssemblyName() {}
   ...
}

It's in the title :)

馃槃 I really didn't see this. :-)

APIs look good as proposed.

BTW: Me neither until @tmat and @nguerrera told me -- @tmat @nguerrera if you can in future add the mini-specs, it will help streamline things :)

Assigning to @safern who is going to give this to our new hire shortly.

@tmat what should be the expected output for GetAssemblyName() ?

where do I find the underlying logic for GetAssemblyName()?

@safern @joperezr I already see a Name getter in AssemblyDefinition. What is the purpose of having a AssemblyName getter and how is it different from assemblyDefinition.Name?

I believe what @tmat meant was not to add an AssemblyName getter, per comment https://github.com/dotnet/corefx/issues/13295#issuecomment-260037855 he meant to add a public method that returns a System.Reflection.AssemblyName object with the required metadata such as PublicKey, Flags, Culture, Version, etc. Because from the AssemblyDefinition itself to build that object requires a little bit of work like in the link he posted: http://source.roslyn.io/#Microsoft.CodeAnalysis/InternalUtilities/AssemblyIdentityUtils.cs

With this public API we would avoid a lot of code duplication trying to extract the metadata from multiple places from the handles. As you can see the AssemblyDefinition.Name is a StringHandle that needs to be passed to the MetadataReader to get the actual string, so we want to avoid code duplication to convert those handles to actual values with this new APIs.

@safern is right. Also note the exact proposed API shape here: https://github.com/dotnet/corefx/issues/13295#issuecomment-260037855

Was this page helpful?
0 / 5 - 0 ratings

Related issues

yahorsi picture yahorsi  路  3Comments

nalywa picture nalywa  路  3Comments

omariom picture omariom  路  3Comments

btecu picture btecu  路  3Comments

bencz picture bencz  路  3Comments