Rationalize how temporary directory works today (#68226)

* Rationalize how temporary directory works today

Before fixing #65415 I needed to better rationalize how
`Path.GetTempPath` and temp paths in general worked in the compiler.
This change ended up addressing the following problems:

**Path.GetTempPath**

`Path.GetTempPath` should never be used in the compiler libraries.
That uses a number of environment variables, like working directory,
that can change during the lifetime of a build operation. That makes
compilation non-deterministic and can lead to sporadic errors. All
uses of `GetTempPath` need to come from the initial csc / vbc
invocation and be passed through as arguments.

This was solved by adding a new BannedSymbols file for compiler
libraries that banned `Path.GetTempPath` and working through the
remaining case I found.

This introduced one new error because `Path.GetTempPath` was being used
as a fall back in legacy signing if csc / vbc couldn't calculate a
temp path at invocation time. This is a "this should never actually
happen" problem but I added an error code as a precaution.

I did not force the temp directory to non-null for all compilations
because the vast majority of compilations don't actually need a temp
directory. It is only used in legacy signing which is very rare.

**Where is temp required**

It was unclear in our code base where a temp directory was actually
required for execution. To get a better handle on this, now and going
forward, I enabled NRT in files that handled temp directories. After
working through it there are only two cases where it's required:

1. When using legacy strong named signing.
2. When starting the compiler server.

For (2) it's required as a place to shadow copy analyzers /
generators. There is no reasonable fall back if the server can't
calculate a temp path. Further if the client can't calculate a temp path
then there is a very strong chance (basically guaranteed) that the
server can't either. Therefore I added a few places where we simply
don't even try to communicate with the compiler server if we can't find
a temporary directory. Basically fail fast vs. failing slow.

Once this change is in I can move forward with #65415. The impact of
that change will be much clearer after this goes through because it
changes both what we depend on and where shadow copy happens.

* Change

* PR feedback

* undelete settings.json

* Clarify stream relationship

* PR feedback

---------

Co-authored-by: Jared Parsons <[email protected]>

src/Compilers/CSharp/Test/Emit/Emit/DesktopStrongNameProviderTests.cs

@@ -21,14 +21,14 @@ public void RespectCustomTempPath()
         {
             var tempDir = Temp.CreateDirectory();
             var provider = new DesktopStrongNameProvider(tempPath: tempDir.Path);
-            Assert.Equal(tempDir.Path, provider.FileSystem.GetTempPath());
+            Assert.Equal(tempDir.Path, provider.FileSystem.GetSigningTempPath());
         }
 
         [Fact]
-        public void RespectDefaultTempPath()
+        public void RespectNullTempPath()
         {
             var provider = new DesktopStrongNameProvider(tempPath: null);
-            Assert.Equal(Path.GetTempPath(), provider.FileSystem.GetTempPath());
+            Assert.Null(provider.FileSystem.GetSigningTempPath());
         }
 
         [Fact]

src/Compilers/CSharp/Test/Emit2/Attributes/EmitTestStrongNameProvider.cs

@@ -22,7 +22,7 @@ public partial class InternalsVisibleToAndStrongNameTests : CSharpTestBase
         public void ExceptionInReadAllBytes()
         {
             var ex = new Exception("Crazy exception you could never have predicted!");
-            var fileSystem = new TestStrongNameFileSystem()
+            var fileSystem = new TestStrongNameFileSystem(_signingTempDirectory.Path)
             {
                 ReadAllBytesFunc = _ => throw ex
             };

src/Compilers/CSharp/Test/Emit2/Attributes/InternalsVisibleToAndStrongNameTests.cs

@@ -26,6 +26,8 @@ namespace Microsoft.CodeAnalysis.CSharp.UnitTests
 {
     public partial class InternalsVisibleToAndStrongNameTests : CSharpTestBase
     {
+        private readonly TempDirectory _signingTempDirectory;
+
         public static IEnumerable<object[]> AllProviderParseOptions
         {
             get
@@ -48,14 +50,18 @@ public static IEnumerable<object[]> AllProviderParseOptions
         public InternalsVisibleToAndStrongNameTests()
         {
             SigningTestHelpers.InstallKey();
+            _signingTempDirectory = Temp.CreateDirectory();
         }
 
         private static readonly string s_keyPairFile = SigningTestHelpers.KeyPairFile;
         private static readonly string s_publicKeyFile = SigningTestHelpers.PublicKeyFile;
         private static readonly ImmutableArray<byte> s_publicKey = SigningTestHelpers.PublicKey;
+        private static readonly StrongNameProvider s_providerNoSigningTempPath = new DesktopStrongNameProvider(
+            ImmutableArray<string>.Empty,
+            new VirtualizedStrongNameFileSystem(tempPath: null));
 
-        private static StrongNameProvider GetProviderWithPath(string keyFilePath) =>
-            new DesktopStrongNameProvider(ImmutableArray.Create(keyFilePath), strongNameFileSystem: new VirtualizedStrongNameFileSystem());
+        private StrongNameProvider GetProviderWithPath(string keyFilePath) =>
+            new DesktopStrongNameProvider(ImmutableArray.Create(keyFilePath), strongNameFileSystem: new VirtualizedStrongNameFileSystem(_signingTempDirectory.Path));
 
         #endregion
 
@@ -178,6 +184,35 @@ public void PubKeyFromKeyFileAttribute_AssemblyKeyFileResolver_RelativeToCurrent
             Assert.True(ByteSequenceComparer.Equals(s_publicKey, comp.Assembly.Identity.PublicKey));
         }
 
+        [Fact]
+        public void PubKeyFromKeyFileAttribute_SigningTempPathNotAvailable()
+        {
+            string code = String.Format("{0}{1}{2}", @"[assembly: System.Reflection.AssemblyKeyFile(@""", s_keyPairFile, @""")] public class C {}");
+
+            var options = TestOptions.SigningReleaseDll.WithStrongNameProvider(s_providerNoSigningTempPath);
+            Assert.Null(options.StrongNameProvider.FileSystem.GetSigningTempPath());
+            var compilation = CreateCompilation(code, options: options, parseOptions: TestOptions.Regular);
+            compilation.VerifyEmitDiagnostics();
+
+            compilation = CreateCompilation(code, options: options, parseOptions: TestOptions.RegularWithLegacyStrongName);
+            compilation.VerifyEmitDiagnostics(
+                Diagnostic(ErrorCode.ERR_PublicKeyFileFailure).WithArguments(s_keyPairFile, CodeAnalysisResources.SigningTempPathUnavailable));
+        }
+
+        [Fact]
+        public void PubKeyFromKeyFileAttribute_SigningTempPathAvailable()
+        {
+            string code = String.Format("{0}{1}{2}", @"[assembly: System.Reflection.AssemblyKeyFile(@""", s_keyPairFile, @""")] public class C {}");
+
+            var options = TestOptions.SigningReleaseDll.WithStrongNameProvider(SigningTestHelpers.DefaultDesktopStrongNameProvider);
+            Assert.NotNull(options.StrongNameProvider.FileSystem.GetSigningTempPath());
+            var compilation = CreateCompilation(code, options: options, parseOptions: TestOptions.Regular);
+            compilation.VerifyEmitDiagnostics();
+
+            compilation = CreateCompilation(code, options: options, parseOptions: TestOptions.RegularWithLegacyStrongName);
+            compilation.VerifyEmitDiagnostics();
+        }
+
         [ConditionalFact(typeof(WindowsOnly), Reason = ConditionalSkipReason.TestHasWindowsPaths)]
         public void SigningNotAvailable001()
         {
@@ -188,7 +223,7 @@ public void SigningNotAvailable001()
             var syntaxTree = Parse(s, @"IVTAndStrongNameTests\AnotherTempDir\temp.cs", TestOptions.RegularWithLegacyStrongName);
             var provider = new TestDesktopStrongNameProvider(
                 ImmutableArray.Create(PathUtilities.CombineAbsoluteAndRelativePaths(keyFileDir, @"TempSubDir\")),
-                new VirtualizedStrongNameFileSystem())
+                new VirtualizedStrongNameFileSystem(_signingTempDirectory.Path))
             {
                 GetStrongNameInterfaceFunc = () => throw new DllNotFoundException("aaa.dll not found.")
             };
@@ -391,6 +426,20 @@ public void KeyContainerAttributeOptionConflict(CSharpParseOptions parseOptions)
             Assert.True(ByteSequenceComparer.Equals(s_publicKey, other.Assembly.Identity.PublicKey));
         }
 
+        [ConditionalTheory(typeof(WindowsOnly))]
+        [MemberData(nameof(AllProviderParseOptions))]
+        public void KeyContainerSigningTempPathMissing(CSharpParseOptions parseOptions)
+        {
+            string source = @"class C { }";
+            var options = TestOptions.SigningReleaseDll
+                .WithCryptoKeyContainer("RoslynTestContainer")
+                .WithStrongNameProvider(s_providerNoSigningTempPath);
+            var compilation = CreateCompilation(source, options: options, parseOptions: parseOptions);
+
+            compilation.VerifyEmitDiagnostics(
+                Diagnostic(ErrorCode.ERR_PublicKeyContainerFailure).WithArguments("RoslynTestContainer", CodeAnalysisResources.SigningTempPathUnavailable));
+        }
+
         [Theory]
         [MemberData(nameof(AllProviderParseOptions))]
         public void KeyFileAttributeEmpty(CSharpParseOptions parseOptions)
@@ -2799,7 +2848,7 @@ public void ConsistentErrorMessageWhenProvidingEmptyKeyFile_PublicSign(CSharpPar
         [ConditionalFact(typeof(WindowsOnly), Reason = ConditionalSkipReason.TestExecutionHasCOMInterop)]
         public void LegacyDoesNotUseBuilder()
         {
-            var provider = new TestDesktopStrongNameProvider(fileSystem: new VirtualizedStrongNameFileSystem())
+            var provider = new TestDesktopStrongNameProvider(fileSystem: new VirtualizedStrongNameFileSystem(_signingTempDirectory.Path))
             {
                 SignBuilderFunc = delegate { throw null; }
             };

src/Compilers/CSharp/csc/Program.cs

@@ -2,8 +2,6 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
-#nullable disable
-
 using System;
 using System.Diagnostics;
 using System.IO;
@@ -39,7 +37,7 @@ private static int MainCore(string[] args)
             return BuildClient.Run(args, RequestLanguage.CSharpCompile, Csc.Run, BuildClient.GetCompileOnServerFunc(logger), logger);
         }
 
-        public static int Run(string[] args, string clientDir, string workingDir, string sdkDir, string tempDir, TextWriter textWriter, IAnalyzerAssemblyLoader analyzerLoader)
+        public static int Run(string[] args, string clientDir, string workingDir, string sdkDir, string? tempDir, TextWriter textWriter, IAnalyzerAssemblyLoader analyzerLoader)
             => Csc.Run(args, new BuildPaths(clientDir: clientDir, workingDir: workingDir, sdkDir: sdkDir, tempDir: tempDir), textWriter, analyzerLoader);
     }
 }

src/Compilers/Core/CommandLine/BuildProtocol.cs

@@ -74,7 +74,7 @@ public BuildRequest(RequestLanguage language,
         public static BuildRequest Create(RequestLanguage language,
                                           IList<string> args,
                                           string workingDirectory,
-                                          string tempDirectory,
+                                          string? tempDirectory,
                                           string compilerHash,
                                           Guid? requestId = null,
                                           string? keepAlive = null,

src/Compilers/Core/Portable/BannedSymbols.CompilerLibraries.txt

@@ -0,0 +1 @@
+M:System.IO.Path.GetTempPath(); Cannot be used safely in APIs or compiler server as underlying environment variables can change during build.

src/Compilers/Core/Portable/CodeAnalysisResources.resx

@@ -777,4 +777,7 @@
   <data name="Nothing" xml:space="preserve">
     <value>Nothing</value>
   </data>
+  <data name="SigningTempPathUnavailable" xml:space="preserve">
+    <value>The temporary path for legacy file signing is unavailable.</value>
+  </data>
 </root>

src/Compilers/Core/Portable/CommandLine/CommandLineParser.cs

@@ -706,11 +706,11 @@ internal static IEnumerable<string> ParseResponseLines(IEnumerable<string> lines
         /// </param>
         internal static bool TryParseClientArgs(
             IEnumerable<string> args,
-            out List<string>? parsedArgs,
+            [NotNullWhen(true)] out List<string>? parsedArgs,
             out bool containsShared,
             out string? keepAliveValue,
             out string? pipeName,
-            out string? errorMessage)
+            [NotNullWhen(false)] out string? errorMessage)
         {
             containsShared = false;
             keepAliveValue = null;

src/Compilers/Core/Portable/CommandLine/CommonCompiler.CompilerEmitStreamProvider.cs

@@ -45,8 +45,6 @@ public void Close(DiagnosticBag diagnostics)
                 }
             }
 
-            public override Stream? Stream => null;
-
             protected override Stream? CreateStream(DiagnosticBag diagnostics)
             {
                 Debug.Assert(_streamToDispose == null);

src/Compilers/Core/Portable/CommandLine/CommonCompiler.cs

@@ -44,7 +44,7 @@ internal readonly struct BuildPaths
         internal string? SdkDirectory { get; }
 
         /// <summary>
-        /// The temporary directory a compilation should use instead of <see cref="Path.GetTempPath"/>.  The latter
+        /// The temporary directory a compilation should use instead of Path.GetTempPath.  The latter
         /// relies on global state individual compilations should ignore.
         /// </summary>
         internal string? TempDirectory { get; }