Merge pull request #11469 from khyperia/fix9288

Improve error message for failing to sign on CoreCLR mac/linux

src/Compilers/CSharp/Test/Syntax/Diagnostics/DiagnosticTest.cs

@@ -8,6 +8,7 @@
 using Microsoft.CodeAnalysis.Text;
 using Roslyn.Test.Utilities;
 using Xunit;
+using Microsoft.CodeAnalysis.Emit;
 
 namespace Microsoft.CodeAnalysis.CSharp.UnitTests
 {
@@ -2233,5 +2234,44 @@ public override string GetErrorDisplayString(ISymbol symbol)
         }
 
         #endregion
+
+        #region CoreCLR Signing Tests
+        // These aren't actually syntax tests, but this is in one of only two assemblies tested on linux
+        [ConditionalFact(typeof(UnixLikeOnly), typeof(NotMonoOnly)), WorkItem(9288, "https://github.com/dotnet/roslyn/issues/9288")]
+        public void Bug9288_keyfile()
+        {
+            var snk = Temp.CreateFile().WriteAllBytes(TestResources.General.snKey);
+            var snkPath = snk.Path;
+
+            const string source = "";
+
+            var ca = CreateCompilationWithMscorlib(source, options: TestOptions.ReleaseDll.WithStrongNameProvider(new DesktopStrongNameProvider()).WithCryptoKeyFile(snkPath));
+
+            ca.VerifyEmitDiagnostics(EmitOptions.Default.WithDebugInformationFormat(DebugInformationFormat.PortablePdb),
+                // error CS7027: Error signing output with public key from file '{temp path}' -- Assembly signing not supported.
+                Diagnostic(ErrorCode.ERR_PublicKeyFileFailure).WithArguments(snkPath, "Assembly signing not supported.").WithLocation(1, 1)
+            );
+        }
+
+        [ConditionalFact(typeof(UnixLikeOnly), typeof(NotMonoOnly)), WorkItem(9288, "https://github.com/dotnet/roslyn/issues/9288")]
+        public void Bug9288_keycontainer()
+        {
+            const string source = "";
+
+            var ca = CreateCompilationWithMscorlib(source, options: TestOptions.ReleaseDll.WithStrongNameProvider(new DesktopStrongNameProvider()).WithCryptoKeyContainer("bogus"));
+
+            ca.VerifyEmitDiagnostics(EmitOptions.Default.WithDebugInformationFormat(DebugInformationFormat.PortablePdb),
+                // error CS7028: Error signing output with public key from container 'bogus' -- Assembly signing not supported.
+                Diagnostic(ErrorCode.ERR_PublicKeyContainerFailure).WithArguments("bogus", "Assembly signing not supported.").WithLocation(1, 1)
+            );
+        }
+
+        // There are three places where we catch a ClrStrongNameMissingException,
+        // but the third cannot happen - only if a key is successfully retrieved
+        // from a keycontainer, and then we fail to get IClrStrongName afterwards
+        // for the actual signing. However, we error on the key read, and can never
+        // get to the third case (but there's still error handling if that changes)
+
+        #endregion
     }
 }

src/Compilers/Core/Portable/Compilation/Compilation.cs

@@ -1971,6 +1971,12 @@ private static EmitResult ToEmitResultAndFree(DiagnosticBag diagnostics, bool su
                     {
                         Options.StrongNameProvider.SignAssembly(StrongNameKeys, signingInputStream, peStream);
                     }
+                    catch (DesktopStrongNameProvider.ClrStrongNameMissingException)
+                    {
+                        diagnostics.Add(StrongNameKeys.GetError(StrongNameKeys.KeyFilePath, StrongNameKeys.KeyContainer,
+                            new CodeAnalysisResourcesLocalizableErrorArgument(nameof(CodeAnalysisResources.AssemblySigningNotSupported)), MessageProvider));
+                        return false;
+                    }
                     catch (IOException ex)
                     {
                         diagnostics.Add(StrongNameKeys.GetError(StrongNameKeys.KeyFilePath, StrongNameKeys.KeyContainer, ex.Message, MessageProvider));

src/Compilers/Core/Portable/StrongName/DesktopStrongNameProvider.cs

@@ -104,6 +104,18 @@ public override void Write(byte[] buffer, int offset, int count)
             }
         }
 
+        // This exception is only used to detect when the acquisition of IClrStrongName fails
+        // and the likely reason is that we're running on CoreCLR on a non-Windows platform.
+        // The place where the acquisition fails does not have access to localization,
+        // so we can't throw some generic exception with a localized message.
+        // So this is sort of a token for the eventual message to be generated.
+
+        // The path from where this is thrown to where it is caught is all internal,
+        // so there's no chance of an API consumer seeing it.
+        internal sealed class ClrStrongNameMissingException : Exception
+        {
+        }
+
         private readonly ImmutableArray<string> _keyFileSearchPaths;
 
         // for testing/mocking
@@ -203,6 +215,8 @@ internal override StrongNameKeys CreateKeys(string keyFilePath, string keyContai
                 {
                     return new StrongNameKeys(StrongNameKeys.GetKeyFileError(messageProvider, keyFilePath, ex.Message));
                 }
+                // it turns out that we don't need IClrStrongName to retrieve a key file,
+                // so there's no need for a catch of ClrStrongNameMissingException in this case
             }
             else if (!string.IsNullOrEmpty(keyContainerName))
             {
@@ -211,6 +225,11 @@ internal override StrongNameKeys CreateKeys(string keyFilePath, string keyContai
                     ReadKeysFromContainer(keyContainerName, out publicKey);
                     container = keyContainerName;
                 }
+                catch (ClrStrongNameMissingException)
+                {
+                    return new StrongNameKeys(StrongNameKeys.GetContainerError(messageProvider, keyContainerName,
+                        new CodeAnalysisResourcesLocalizableErrorArgument(nameof(CodeAnalysisResources.AssemblySigningNotSupported))));
+                }
                 catch (IOException ex)
                 {
                     return new StrongNameKeys(StrongNameKeys.GetContainerError(messageProvider, keyContainerName, ex.Message));
@@ -226,6 +245,11 @@ private void ReadKeysFromContainer(string keyContainer, out ImmutableArray<byte>
             {
                 publicKey = GetPublicKey(keyContainer);
             }
+            catch (ClrStrongNameMissingException)
+            {
+                // pipe it through so it's catchable directly by type
+                throw;
+            }
             catch (Exception ex)
             {
                 throw new IOException(ex.Message);
@@ -233,6 +257,7 @@ private void ReadKeysFromContainer(string keyContainer, out ImmutableArray<byte>
         }
 
         /// <exception cref="IOException"></exception>
+        /// <exception cref="ClrStrongNameMissingException"></exception>
         internal override void SignAssembly(StrongNameKeys keys, Stream inputStream, Stream outputStream)
         {
             Debug.Assert(inputStream is TempFileStream);
@@ -265,7 +290,30 @@ internal override void SignAssembly(StrongNameKeys keys, Stream inputStream, Str
         // internal for testing
         internal IClrStrongName GetStrongNameInterface()
         {
-            return TestStrongNameInterfaceFactory?.Invoke() ?? ClrStrongName.GetInstance();
+            var factoryCreated = TestStrongNameInterfaceFactory?.Invoke();
+
+            if (factoryCreated != null)
+            {
+                return factoryCreated;
+            }
+
+            try
+            {
+                return ClrStrongName.GetInstance();
+            }
+            catch (MarshalDirectiveException) when (PathUtilities.IsUnixLikePlatform)
+            {
+                // CoreCLR, when not on Windows, doesn't support IClrStrongName (or COM in general).
+                // This is really hard to detect/predict without false positives/negatives.
+                // It turns out that CoreCLR throws a MarshalDirectiveException when attempting
+                // to get the interface (Message "Cannot marshal 'return value': Unknown error."),
+                // so just catch that and state that it's not supported.
+
+                // We're deep in a try block that reports the exception's Message as part of a diagnostic.
+                // This exception will skip through the IOException wrapping by `Sign` (in this class),
+                // then caught by Compilation.SerializeToPeStream or DesktopStringNameProvider.CreateKeys
+                throw new ClrStrongNameMissingException();
+            }
         }
 
         internal ImmutableArray<byte> GetPublicKey(string keyContainer)
@@ -294,6 +342,11 @@ private void Sign(string filePath, string keyName)
                 int unused;
                 strongName.StrongNameSignatureGeneration(filePath, keyName, IntPtr.Zero, 0, null, out unused);
             }
+            catch (ClrStrongNameMissingException)
+            {
+                // pipe it through so it's catchable directly by type
+                throw;
+            }
             catch (Exception ex)
             {
                 throw new IOException(ex.Message, ex);
@@ -313,6 +366,11 @@ private unsafe void Sign(string filePath, ImmutableArray<byte> keyPair)
                     strongName.StrongNameSignatureGeneration(filePath, null, (IntPtr)pinned, keyPair.Length, null, out unused);
                 }
             }
+            catch (ClrStrongNameMissingException)
+            {
+                // pipe it through so it's catchable directly by type
+                throw;
+            }
             catch (Exception ex)
             {
                 throw new IOException(ex.Message, ex);

src/Test/Utilities/Desktop/ConditionalFactAttribute.cs

@@ -5,6 +5,7 @@
 using Microsoft.CodeAnalysis.Test.Utilities;
 using Microsoft.Win32;
 using Xunit;
+using Roslyn.Utilities;
 
 namespace Roslyn.Test.Utilities
 {
@@ -15,6 +16,20 @@ public class WindowsOnly : ExecutionCondition
         public override string SkipReason { get { return "Test not supported on Mono"; } }
     }
 
+    public class UnixLikeOnly : ExecutionCondition
+    {
+        public override bool ShouldSkip { get { return !PathUtilities.IsUnixLikePlatform; } }
+
+        public override string SkipReason { get { return "Test not supported on Windows"; } }
+    }
+
+    public class NotMonoOnly : ExecutionCondition
+    {
+        public override bool ShouldSkip { get { return MonoHelpers.IsRunningOnMono(); } }
+
+        public override string SkipReason { get { return "Test not supported on Mono"; } }
+    }
+
     public class Framework35Installed : ExecutionCondition
     {
         public override bool ShouldSkip

src/Test/Utilities/Portable/ConditionalFactAttribute.cs

+// Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
+
+using System;
+using System.IO;
+using Microsoft.CodeAnalysis.Test.Utilities;
+using Microsoft.Win32;
+using Xunit;
+using Roslyn.Utilities;
+
+namespace Roslyn.Test.Utilities
+{
+    public class WindowsOnly : ExecutionCondition
+    {
+        public override bool ShouldSkip { get { return Path.DirectorySeparatorChar != '\\'; } }
+
+        public override string SkipReason { get { return "Test not supported on Mono"; } }
+    }
+
+    public class UnixLikeOnly : ExecutionCondition
+    {
+        public override bool ShouldSkip { get { return !PathUtilities.IsUnixLikePlatform; } }
+
+        public override string SkipReason { get { return "Test not supported on Windows"; } }
+    }
+
+    public class NotMonoOnly : ExecutionCondition
+    {
+        public override bool ShouldSkip { get { return MonoHelpers.IsRunningOnMono(); } }
+
+        public override string SkipReason { get { return "Test not supported on Mono"; } }
+    }
+}

src/Test/Utilities/Portable/TestUtilities.csproj

@@ -119,6 +119,7 @@
   </ItemGroup>
   <ItemGroup>
     <Compile Include="CommonTestBase.cs" />
+    <Compile Include="ConditionalFactAttribute.cs" />
     <Compile Include="CoreCLRRuntimeEnvironment.cs" />
     <Compile Include="Exceptions.cs" />
     <Compile Include="ICompilationVerifier.cs" />