Skip to content

[mono] Support public key token in DefineDynamicAssembly - #58016

Merged
lambdageek merged 1 commit into
dotnet:mainfrom
uweigand:mono-publickeytoken
Aug 27, 2021
Merged

[mono] Support public key token in DefineDynamicAssembly#58016
lambdageek merged 1 commit into
dotnet:mainfrom
uweigand:mono-publickeytoken

Conversation

@uweigand

Copy link
Copy Markdown
Contributor

@ghostghost added area-VM-meta-mono community-contribution Indicates that the PR has been added by a community member labels Aug 24, 2021
@ghost

Copy link
Copy Markdown

Tagging subscribers to this area:
See info in area-owners.md if you want to be subscribed.

Issue Details
Author:uweigand
Assignees:-
Labels:

area-VM-meta-mono

Milestone:-

@jkotas

Copy link
Copy Markdown
Member

Do we need to add test?

Comment threadsrc/mono/mono/metadata/sre.c Outdated
@uweigand

Copy link
Copy Markdown
ContributorAuthor

Do we need to add test?

I noticed there are existing tests for DefineDynamicAssembly, but

I've just pushed a version that fixes the endless recursion, enables the test cases, and adds a public key test. The test passes on CoreCLR, fails on Mono before this patch, and succeeds on Mono after this patch.

Does this look OK or would you prefer if I split off the regression fix into a separate PR?

@lambdageek

Copy link
Copy Markdown
Member

Does this look OK or would you prefer if I split off the regression fix into a separate PR?

I'd prefer the fix in a separate PR - just because it'll be easier to reason about the backports to 6.0

@uweigand

Copy link
Copy Markdown
ContributorAuthor

Does this look OK or would you prefer if I split off the regression fix into a separate PR?

I'd prefer the fix in a separate PR - just because it'll be easier to reason about the backports to 6.0

OK, this is now #58201

Comment threadsrc/libraries/System.Reflection.Emit/tests/AssemblyBuilderTests.cs Outdated
Comment threadsrc/libraries/System.Reflection.Emit/tests/AssemblyBuilderTests.cs Outdated
@uweigand

Copy link
Copy Markdown
ContributorAuthor

Rebased after #58201 was merged.

@lambdageek

Copy link
Copy Markdown
Member

/backport to release/6.0

@github-actions

Copy link
Copy Markdown
Contributor

Started backporting to release/6.0: https://github.com/dotnet/runtime/actions/runs/1175273337

@lambdageek
lambdageek merged commit b547356 into dotnet:mainAug 27, 2021
@ghostghost locked as resolved and limited conversation to collaborators Sep 26, 2021
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-VM-meta-monocommunity-contributionIndicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[mono] DefineDynamicAssembly ignores PublicKeyToken

4 participants

@uweigand@jkotas@lambdageek@vargaz