Enable portable signing for Mono#26683
Conversation
Desktop signing uses a COM call into the CLR to sign an assembly, which is not available on Mono or CoreCLR. This change always uses portable signing by default on Mono as well as CoreCLR.
|
@akoeplinger Could you or someone on Mono confirm that this is a robust mechanism for detecting if we're running on Mono? |
|
cc @marek-safar |
| { | ||
| bool fallback = | ||
| !CoreClrShim.IsRunningOnCoreClr || | ||
| !(CoreClrShim.IsRunningOnCoreClr || Type.GetType("Mono.Runtime") != null) || |
There was a problem hiding this comment.
this should eventually be refactored into a helper since it's used in a couple places in the codebase: https://github.com/dotnet/roslyn/search?q=%22Mono.Runtime%22&type=Code
There was a problem hiding this comment.
Need to wrap this into a helper for this change I think. The Type.GetType method can throw exceptions in corner cases. Can't let those take down the compiler here.
|
@agocke yes that's recommended way to check for Mono runtime executing the code |
|
approved to merge once all green. |
|
retest windows_debug_vs-integration_prtest please |
|
@jaredpar what's the timeline for 2.8 service release including this fix? |
|
@marek-safar this is currently targeted for 15.8.2. I don't know what the time frame for that to RTM is but typically it's days. The NuGet packages for the fix (2.8.1) will be made available as soon as we have validation the insertion into VS. Possibly later today or Monday at the latest. |
Customer scenario
Signing was previously working on Mono for keyfiles, but does not work after 2.7.1.
Bugs this fixes
#26678
https://devdiv.visualstudio.com/DefaultCollection/DevDiv/_workitems/edit/615168
Workarounds, if any
None, for Mono.
Risk
This check is only used for switching the key providers, and has a focused change just to allow mono to go through the same path as CoreCLR.
Performance impact
Low, one small reflection check per compile.
Is this a regression from a previous update?
Yes.
Root cause analysis
We have no Mono tests. It's not clear that this was supposed to work for Mono, it may have simply been functioning by accident.
How was the bug found?
Mono reported.