-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Fix issue where output path set in construction never updated the project update state #74791
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
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
38 changes: 38 additions & 0 deletions
38
.../Core/Test/ProjectSystemShim/VisualStudioProjectTests/ProjectSystemProjectFactoryTests.vb
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| ' Licensed to the .NET Foundation under one or more agreements. | ||
| ' The .NET Foundation licenses this file to you under the MIT license. | ||
| ' See the LICENSE file in the project root for more information. | ||
|
|
||
| Imports System.Collections.Immutable | ||
| Imports System.Threading | ||
| Imports Microsoft.CodeAnalysis | ||
| Imports Microsoft.CodeAnalysis.Test.Utilities | ||
| Imports Microsoft.CodeAnalysis.Workspaces.ProjectSystem | ||
| Imports Microsoft.VisualStudio.LanguageServices.Implementation.ProjectSystem | ||
| Imports Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim.Framework | ||
| Imports Roslyn.Test.Utilities | ||
|
|
||
| Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim | ||
| <[UseExportProvider]> | ||
| Public Class ProjectSystemProjectFactoryTests | ||
| <WpfFact, WorkItem("https://github.com/dotnet/vscode-csharp/issues/7402")> | ||
| Public Async Function ProjectInstantiatedWithCompilationOutputAssemblyFilePathCanBeChanged() As Task | ||
| Using environment = New TestEnvironment() | ||
| Dim creationInfo = New VisualStudioProjectCreationInfo() | ||
| creationInfo.CompilationOutputAssemblyFilePath = "C:\output\project.dll" | ||
| Dim project1 = Await environment.ProjectFactory.CreateAndAddToWorkspaceAsync( | ||
| "project1", LanguageNames.CSharp, creationInfo, CancellationToken.None) | ||
|
|
||
| Dim projectInSolution = environment.Workspace.CurrentSolution.GetProject(project1.Id) | ||
|
|
||
| Assert.Equal(creationInfo.CompilationOutputAssemblyFilePath, projectInSolution.CompilationOutputInfo.AssemblyPath) | ||
|
|
||
| ' Change the path and ensure it's updated | ||
| Dim newOutputPath = "C:\output\new\project.dll" | ||
| project1.CompilationOutputAssemblyFilePath = newOutputPath | ||
|
|
||
| Dim newProjectInSolution As Project = environment.Workspace.CurrentSolution.GetProject(project1.Id) | ||
| Assert.Equal(newOutputPath, newProjectInSolution.CompilationOutputInfo.AssemblyPath) | ||
| End Using | ||
| End Function | ||
| End Class | ||
| End Namespace |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -102,8 +102,7 @@ public async Task<ProjectSystemProject> CreateAndAddToWorkspaceAsync(string proj | |
| assemblyName, | ||
| creationInfo.CompilationOptions, | ||
| creationInfo.FilePath, | ||
| creationInfo.ParseOptions, | ||
| creationInfo.CompilationOutputAssemblyFilePath); | ||
| creationInfo.ParseOptions); | ||
|
|
||
| var versionStamp = creationInfo.FilePath != null | ||
| ? VersionStamp.Create(File.GetLastWriteTimeUtc(creationInfo.FilePath)) | ||
|
|
@@ -164,6 +163,15 @@ await ApplyChangeToWorkspaceAsync(w => | |
| onAfterUpdate: null); | ||
| }).ConfigureAwait(false); | ||
|
|
||
| // Set this value early after solution is created so it is available to Razor. This will get updated | ||
dibarbet marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // when the command line is set, but we want a non-null value to be available as soon as possible. | ||
| // | ||
| // Set the property in a batch; if we set the property directly we'll be taking a synchronous lock here and | ||
| // potentially block up thread pool threads. Doing this in a batch means the global lock will be acquired asynchronously. | ||
| var disposableBatchScope = await project.CreateBatchScopeAsync(CancellationToken.None).ConfigureAwait(false); | ||
| await using var _ = disposableBatchScope.ConfigureAwait(false); | ||
| project.CompilationOutputAssemblyFilePath = creationInfo.CompilationOutputAssemblyFilePath; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could this have essentially been the only change needed here?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Possibly - but going through the same property setter as everything else feels safer. |
||
|
|
||
| return project; | ||
| } | ||
|
|
||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure why Razor is being commented here -- this is just doing the thing the API is asking us to do....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the early set was initially added for Razor here - 863962d
It does get set anyway later on when the cmdline is set.