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

Fix type reference name in attribute argument getting mangled when #722

Merged
merged 1 commit into from
Feb 11, 2021

Conversation

mrvoorhe
Copy link
Contributor

@mrvoorhe mrvoorhe commented Feb 9, 2021

such attribute is put on a method in a projected type.

This is a fix that Unity's cecil has had for a couple years.

@@ -88,7 +88,7 @@ public void ByRefTypeReference ()
public void FullyQualifiedTypeReference ()
{
var module = GetCurrentModule ();
var cecil = module.AssemblyReferences.Where (reference => reference.Name == "Mono.Cecil").First ();
var cecil = module.AssemblyReferences.Where (reference => reference.Name == "Unity.Cecil").First ();
Copy link
Owner

Choose a reason for hiding this comment

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

Nope :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@mrvoorhe
Copy link
Contributor Author

I can't tell if a test failed or if there was some CI instability. All I see in the log is

  Determining projects to restore...
D:\a\cecil\cecil\symbols\pdb\Mono.Cecil.Pdb.csproj : error NU1101: Unable to find package Microsoft.SourceLink.GitHub. No packages exist with this id in source(s): Microsoft Visual Studio Offline Packages [D:\a\cecil\cecil\Mono.Cecil.sln]
D:\a\cecil\cecil\symbols\pdb\Mono.Cecil.Pdb.csproj : error NU1101: Unable to find package Microsoft.NETFramework.ReferenceAssemblies.net40. No packages exist with this id in source(s): Microsoft Visual Studio Offline Packages [D:\a\cecil\cecil\Mono.Cecil.sln]
  Failed to restore D:\a\cecil\cecil\symbols\pdb\Mono.Cecil.Pdb.csproj (in 1.1 sec).
  Restored D:\a\cecil\cecil\symbols\pdb\Test\Mono.Cecil.Pdb.Tests.csproj (in 49.83 sec).
  Restored D:\a\cecil\cecil\symbols\mdb\Test\Mono.Cecil.Mdb.Tests.csproj (in 48.71 sec).
  Restored D:\a\cecil\cecil\symbols\mdb\Mono.Cecil.Mdb.csproj (in 7 ms).
  Restored D:\a\cecil\cecil\Test\Mono.Cecil.Tests.csproj (in 125 ms).
  Restored D:\a\cecil\cecil\rocks\Mono.Cecil.Rocks.csproj (in 5 ms).
  Restored D:\a\cecil\cecil\rocks\Test\Mono.Cecil.Rocks.Tests.csproj (in 127 ms).
  Restored D:\a\cecil\cecil\Mono.Cecil.csproj (in 5 ms).

Build FAILED.

@mrvoorhe
Copy link
Contributor Author

Looks like it was a CI instability. I force pushed to trigger a new CI run and now the tests are green.

@jbevain
Copy link
Owner

jbevain commented Feb 11, 2021

Yeah that's NuGet having hiccups.

private void WriteCustomAttributeTypeValue (TypeReference value)
{
var typeDefinition = value as TypeDefinition;
TypeDefinition outermostDeclaringType = typeDefinition;
Copy link
Owner

Choose a reason for hiding this comment

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

Could you move this into the if branch?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

…ch attribute is put on a method in a projected type.
@jbevain jbevain merged commit 43336ee into jbevain:master Feb 11, 2021
@jbevain
Copy link
Owner

jbevain commented Feb 11, 2021

Thank you!

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.

3 participants