Merge pull request #75674 from tmat/FixCompilerGeneratedFilesOutputPath

Fix flow of CompilerGeneratedFilesOutputPath from CPS
dotnet · Nov 1, 2024 · be03640 · be03640
2 parents 9589188 + 4b30d25
commit be03640
Show file tree

Hide file tree

Showing 5 changed files with 68 additions and 23 deletions.
diff --git a/src/Compilers/Core/MSBuildTask/Microsoft.Managed.Core.targets b/src/Compilers/Core/MSBuildTask/Microsoft.Managed.Core.targets
@@ -319,8 +319,11 @@
     
     CompilerGeneratedFilesOutputPath controls the location the files will be output to.
     The compiler will not emit any generated files when the path is empty, and defaults to a /generated directory in $(IntermediateOutputPath) if $(IntermediateOutputPath) has a value.
+    A relative path is considered relative to the project directory.
+
     EmitCompilerGeneratedFiles allows the user to control if anything is emitted by clearing the property when not true.
     When EmitCompilerGeneratedFiles is true, we ensure that CompilerGeneatedFilesOutputPath has a value and issue a warning if not.
+
     We will create CompilerGeneratedFilesOutputPath if it does not exist.
     -->
   <PropertyGroup>

diff --git a/src/VisualStudio/CSharp/Test/ProjectSystemShim/CPS/AdditionalPropertiesTests.cs b/src/VisualStudio/CSharp/Test/ProjectSystemShim/CPS/AdditionalPropertiesTests.cs
@@ -4,6 +4,7 @@
 
 #nullable disable
 
+using System.IO;
 using System.Linq;
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.CSharp;
@@ -151,5 +152,29 @@ void TestLegacyProject()
                 Assert.Equal(expectedRunAnalyzers, environment.Workspace.CurrentSolution.Projects.Single().State.RunAnalyzers);
             }
         }
+
+        [WpfFact]
+        public async Task SetProperty_CompilerGeneratedFilesOutputPath_CPS()
+        {
+            using (var environment = new TestEnvironment())
+            using (var project = await CSharpHelpers.CreateCSharpCPSProjectAsync(environment, "Test"))
+            {
+                Assert.Null(environment.Workspace.CurrentSolution.Projects.Single().CompilationOutputInfo.GeneratedFilesOutputDirectory);
+
+                // relative path is relative to the project dir:
+                project.SetProperty(BuildPropertyNames.CompilerGeneratedFilesOutputPath, "generated");
+                AssertEx.AreEqual(
+                    Path.Combine(Path.GetDirectoryName(project.ProjectFilePath), "generated"),
+                    environment.Workspace.CurrentSolution.Projects.Single().CompilationOutputInfo.GeneratedFilesOutputDirectory);
+
+                var path = Path.Combine(TempRoot.Root, "generated");
+                project.SetProperty(BuildPropertyNames.CompilerGeneratedFilesOutputPath, path);
+                AssertEx.AreEqual(path, environment.Workspace.CurrentSolution.Projects.Single().CompilationOutputInfo.GeneratedFilesOutputDirectory);
+
+                // empty path:
+                project.SetProperty(BuildPropertyNames.CompilerGeneratedFilesOutputPath, "");
+                Assert.Null(environment.Workspace.CurrentSolution.Projects.Single().CompilationOutputInfo.GeneratedFilesOutputDirectory);
+            }
+        }
     }
 }
diff --git a/src/VisualStudio/CSharp/Test/ProjectSystemShim/CPS/CSharpCompilerOptionsTests.cs b/src/VisualStudio/CSharp/Test/ProjectSystemShim/CPS/CSharpCompilerOptionsTests.cs
@@ -185,8 +185,27 @@ public async Task ChecksumAlgorithm_CPS()
 
             cpsProject.SetOptions(ImmutableArray.Create("/checksumalgorithm:SHA1"));
 
-            var project = environment.Workspace.CurrentSolution.Projects.Single();
             Assert.Equal(SourceHashAlgorithm.Sha1, environment.Workspace.CurrentSolution.Projects.Single().State.ChecksumAlgorithm);
         }
+
+        [WpfFact]
+        public async Task CompilerGeneratedFilesOutputPath_CPS()
+        {
+            using var environment = new TestEnvironment();
+            using var cpsProject = await CSharpHelpers.CreateCSharpCPSProjectAsync(environment, "Test");
+
+            Assert.Null(environment.Workspace.CurrentSolution.Projects.Single().CompilationOutputInfo.GeneratedFilesOutputDirectory);
+
+            var path = Path.Combine(TempRoot.Root, "generated");
+            cpsProject.SetOptions(["/generatedfilesout:" + path]);
+
+            AssertEx.AreEqual(path, environment.Workspace.CurrentSolution.Projects.Single().CompilationOutputInfo.GeneratedFilesOutputDirectory);
+
+            // relative path is relative to the project dir:
+            cpsProject.SetOptions(["/generatedfilesout:gen2"]);
+            AssertEx.AreEqual(
+                Path.Combine(Path.GetDirectoryName(cpsProject.ProjectFilePath), "gen2"),
+                environment.Workspace.CurrentSolution.Projects.Single().CompilationOutputInfo.GeneratedFilesOutputDirectory);
+        }
     }
 }
diff --git a/src/VisualStudio/Core/Impl/ProjectSystem/CPS/CPSProject_IWorkspaceProjectContext.cs b/src/VisualStudio/Core/Impl/ProjectSystem/CPS/CPSProject_IWorkspaceProjectContext.cs
@@ -173,33 +173,30 @@ public void SetProperty(string name, string? value)
             }
             else if (name == BuildPropertyNames.TargetRefPath)
             {
-                // If we don't have a path, always set it to null
+                _projectSystemProject.OutputRefFilePath = GetAbsolutePath(value);
+            }
+            else if (name == BuildPropertyNames.CompilerGeneratedFilesOutputPath)
+            {
+                _projectSystemProject.GeneratedFilesOutputDirectory = GetAbsolutePath(value);
+            }
+
+            string? GetAbsolutePath(string? value)
+            {
                 if (string.IsNullOrEmpty(value))
                 {
-                    _projectSystemProject.OutputRefFilePath = null;
+                    return null;
                 }
-                else
+
+                if (PathUtilities.IsAbsolute(value))
                 {
-                    // If we only have a non-rooted path, make it full. This is apparently working around cases
-                    // where CPS pushes us a temporary path when they're loading. It's possible this hack
-                    // can be removed now, but we still have tests asserting it.
-                    if (!PathUtilities.IsAbsolute(value))
-                    {
-                        var rootDirectory = _projectSystemProject.FilePath != null
-                                            ? Path.GetDirectoryName(_projectSystemProject.FilePath)
-                                            : Path.GetTempPath();
-
-                        _projectSystemProject.OutputRefFilePath = Path.Combine(rootDirectory, value);
-                    }
-                    else
-                    {
-                        _projectSystemProject.OutputRefFilePath = value;
-                    }
+                    return value;
                 }
-            }
-            else if (name == BuildPropertyNames.CompilerGeneratedFilesOutputPath)
-            {
-                _projectSystemProject.GeneratedFilesOutputDirectory = string.IsNullOrWhiteSpace(value) ? null : value;
+
+                var rootDirectory = _projectSystemProject.FilePath != null
+                    ? Path.GetDirectoryName(_projectSystemProject.FilePath)
+                    : Path.GetTempPath();
+
+                return Path.Combine(rootDirectory, value);
             }
         }
 

diff --git a/src/Workspaces/Core/Portable/Workspace/ProjectSystem/ProjectSystemProjectOptionsProcessor.cs b/src/Workspaces/Core/Portable/Workspace/ProjectSystem/ProjectSystemProjectOptionsProcessor.cs
@@ -221,6 +221,7 @@ private void UpdateProjectOptions_NoLock()
             : _commandLineArgumentsForCommandLine.OutputFileName;
 
         _project.CompilationOutputAssemblyFilePath = fullOutputFilePath ?? _project.CompilationOutputAssemblyFilePath;
+        _project.GeneratedFilesOutputDirectory = _commandLineArgumentsForCommandLine.GeneratedFilesOutputDirectory;
         _project.ParseOptions = parseOptions;
         _project.ChecksumAlgorithm = _commandLineArgumentsForCommandLine.ChecksumAlgorithm;
     }