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

Sign ILVerification.dll #63471

Merged
merged 1 commit into from
Jan 7, 2022
Merged

Sign ILVerification.dll #63471

merged 1 commit into from
Jan 7, 2022

Conversation

jcouv
Copy link
Member

@jcouv jcouv commented Jan 6, 2022

TL;DR: Given the ILVerification ships in a nuget package and as far as I understand the guideline is to strongly name assemblies (even on Core), I'm trying to get ILVerification.dll to be strongly-named.

Background:
Over the break, I've revived the roslyn PR to use ILVerify in the compiler tests (side-by-side with PEVerify).
It's working pretty well, but it hit two issues related to ILVerification.dll not having a strong name:

  1. In our correctness build legs, we get an compilation error (CSC : error CS8002: Referenced assembly 'ILVerification, Version=6.0.0.0, Culture=neutral, PublicKeyToken=null' does not have a strong name. [D:\a\_work\1\s\src\Compilers\Test\Core\Microsoft.CodeAnalysis.Test.Utilities.csproj])
  2. The validation fails in tests on desktop environment because the desktop runtime won't load the assembly (System.IO.FileLoadException : Could not load file or assembly 'ILVerification, Version=6.0.0.0, Culture=neutral, PublicKeyToken=null' or one of its dependencies. A strongly-named assembly is required. (Exception from HRESULT: 0x80131044))

Relates to dotnet/roslyn#22872

@ghost ghost assigned jcouv Jan 6, 2022
@jkotas jkotas added the area-Tools-ILVerification Issues related to ilverify tool and IL verification in general label Jan 7, 2022
@ghost
Copy link

ghost commented Jan 7, 2022

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

Issue Details

TL;DR: Given the ILVerification ships in a nuget package and as far as I understand the guideline is to strongly name assemblies (even on Core), I'm trying to get ILVerification.dll to be strongly-named.

Background:
Over the break, I've revived the roslyn PR to use ILVerify in the compiler tests (side-by-side with PEVerify).
It's working pretty well, but it hit two issues related to ILVerification.dll not having a strong name:

  1. In our correctness build legs, we get an compilation error (CSC : error CS8002: Referenced assembly 'ILVerification, Version=6.0.0.0, Culture=neutral, PublicKeyToken=null' does not have a strong name. [D:\a\_work\1\s\src\Compilers\Test\Core\Microsoft.CodeAnalysis.Test.Utilities.csproj])
  2. The validation fails in tests on desktop environment because the desktop runtime won't load the assembly (System.IO.FileLoadException : Could not load file or assembly 'ILVerification, Version=6.0.0.0, Culture=neutral, PublicKeyToken=null' or one of its dependencies. A strongly-named assembly is required. (Exception from HRESULT: 0x80131044))
Author: jcouv
Assignees: jcouv
Labels:

area-crossgen2-coreclr, area-ILVerification

Milestone: -

@jcouv
Copy link
Member Author

jcouv commented Jan 7, 2022

@jkotas I've been looking at the build artifacts, but I'm having trouble locating the ILVerification assembly to check that it got signed. Would you know how to find it?

@jcouv jcouv marked this pull request as ready for review January 7, 2022 07:05
@jkotas
Copy link
Member

jkotas commented Jan 7, 2022

I am not able to find it either. I think the binary is missing in the CI published artifacts.

@jcouv
Copy link
Member Author

jcouv commented Jan 7, 2022

Thanks. Can this be merged with a single sign-off?

@jkotas
Copy link
Member

jkotas commented Jan 7, 2022

Yep

@jkotas jkotas merged commit 3eccec0 into dotnet:main Jan 7, 2022
@ericstj
Copy link
Member

ericstj commented Jan 10, 2022

@jkotas @jcouv shouldn't we use the open source key for this? Right now it's using 31bf3856ad364e35 which seems unnecessary for a new assembly.

@ericstj
Copy link
Member

ericstj commented Jan 10, 2022

I would recommend setting <StrongNameKeyId>Open</StrongNameKeyId> to use the open key, which is a full key. We do this for libraries, but it looks like this project isn't using those targets.

<StrongNameKeyId>Open</StrongNameKeyId>

Perhaps we should consider moving that default up.

@jkotas
Copy link
Member

jkotas commented Jan 10, 2022

Perhaps we should consider moving that default up.

#63580

@ghost ghost locked as resolved and limited conversation to collaborators Feb 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Tools-ILVerification Issues related to ilverify tool and IL verification in general
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants