Skip to content
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

Editing csproj with invalid SDKs results in multiple error dialogs #3240

Closed
dasMulli opened this issue Feb 11, 2018 · 11 comments
Closed

Editing csproj with invalid SDKs results in multiple error dialogs #3240

dasMulli opened this issue Feb 11, 2018 · 11 comments
Labels
Area-External Owned by another feature area and not this repo. Likely be closed in lieu of issue against it. Bug This is a functional issue in already written code. Feature-Project-File-Editing Editing the project either while open in Visual Studio, or via other mechanism.

Comments

@dasMulli
Copy link

dasMulli commented Feb 11, 2018

Not sure if this is a project system or VS issue.

Steps to reproduce:

  • Use VS 15.6 Preview 4
  • Open a valid csproj file.
  • Add or edit an SDK specification that can't be resolved via NuGet or file system.
  • Multiple warning dialogs pop up
  • Edit the csproj to be valid again
  • Still a few dialogs pop up

Expected:
Warn only once when the project becomes unloadable. Don't warn when it is valid again.

vssdkloadissue-720p

@Pilchie
Copy link
Member

Pilchie commented Feb 11, 2018

Tagging @jviau and @jeffkl

@Pilchie Pilchie added Bug This is a functional issue in already written code. Feature-Project-File-Editing Editing the project either while open in Visual Studio, or via other mechanism. labels Feb 11, 2018
@Pilchie
Copy link
Member

Pilchie commented Feb 13, 2018

Ping @jviau - is this an expected outcome of the reloading changes you made? @sharwell also noticed that if he edited a shared .targets file, he would get a dialog per project (152 in the case of Roslyn.sln).

Is this something that should be looked at more in CPS?

@jviau
Copy link
Contributor

jviau commented Feb 13, 2018

To be honest, I have never seen the <Sdk> tag before. Does it get translated to an import?

@jeffkl
Copy link
Contributor

jeffkl commented Feb 13, 2018

Yes its just a better way of specifying a version

Identitical:

<Project Sdk="My.Custom.Sdk/1.0.0">
</Project>
<Project>
  <Sdk Name="My.Custom.Sdk" Version="1.0.0" />
</Project>

More info is here: https://docs.microsoft.com/en-us/visualstudio/msbuild/how-to-use-project-sdk

@jviau
Copy link
Contributor

jviau commented Feb 13, 2018

Debugging this it appears Sdk elements do not respect IgnoreMissingImports/IgnoreEmptyImports/IgnoreInvalidImports. @jeffkl do you think it should respect those flags?

If not, my only other idea is to veto changes to the Sdk and ask the user to reload the project.

@sharwell do you have an example of the edit you did to a targets file that caused so many dialogs? We should display only 1 dialog, if we are displaying multiple that is pretty bad.

@jeffkl
Copy link
Contributor

jeffkl commented Feb 13, 2018

That seems odd to me. The <Sdk /> element just adds <Import /> elements. Are you seeing a different behavior?

@jviau
Copy link
Contributor

jviau commented Feb 13, 2018

Yes, CPS projects include all 3 of those flags, but ReevaluateIfNecessary() throws with something along the lines of SDK not found exception.

@jeffkl
Copy link
Contributor

jeffkl commented Feb 13, 2018

Yeah definitely. We just discussed this and it makes total sense for us to not throw an InvalidProjectFileException if the SDK can't be found. The project will get unloaded since "expected imports are not found" from what we can tell. Let me code something up and see if we can get it in 15.6

fyi @AndyGerlicher

@jviau
Copy link
Contributor

jviau commented Feb 13, 2018

The full exception is:

The SDK 'Broken/1.0' specified could not be found.  C:\Users\javia\source\repos\ConsoleApp6\ConsoleApp6\ConsoleApp6.csproj 
   at Microsoft.Build.Shared.ProjectErrorUtilities.ThrowInvalidProject(String errorSubCategoryResourceName, IElementLocation elementLocation, String resourceName, Object[] args)
   at Microsoft.Build.Evaluation.Evaluator`4.ExpandAndLoadImportsFromUnescapedImportExpressionConditioned(String directoryOfImportingFile, ProjectImportElement importElement, List`1& projects, Boolean throwOnFileNotExistsError)
   at Microsoft.Build.Evaluation.Evaluator`4.ExpandAndLoadImports(String directoryOfImportingFile, ProjectImportElement importElement)
   at Microsoft.Build.Evaluation.Evaluator`4.EvaluateImportElement(String directoryOfImportingFile, ProjectImportElement importElement)
   at Microsoft.Build.Evaluation.Evaluator`4.PerformDepthFirstPass(ProjectRootElement currentProjectOrImport)
   at Microsoft.Build.Evaluation.Evaluator`4.Evaluate(ILoggingService loggingService, BuildEventContext buildEventContext)
   at Microsoft.Build.Evaluation.Evaluator`4.Evaluate(IEvaluatorData`4 data, ProjectRootElement root, ProjectLoadSettings loadSettings, Int32 maxNodeCount, PropertyDictionary`1 environmentProperties, ILoggingService loggingService, IItemFactory`2 itemFactory, IToolsetProvider toolsetProvider, ProjectRootElementCache projectRootElementCache, BuildEventContext buildEventContext, ProjectInstance projectInstanceIfAnyForDebuggerOnly, ISdkResolverService sdkResolverService, Int32 submissionId)
   at Microsoft.Build.Evaluation.Project.Reevaluate(ILoggingService loggingServiceForEvaluation, ProjectLoadSettings loadSettings)
   at Microsoft.Build.Evaluation.Project.ReevaluateIfNecessary(ILoggingService loggingServiceForEvaluation, ProjectLoadSettings loadSettings)
   at Microsoft.Build.Evaluation.Project.ReevaluateIfNecessary()
   at Microsoft.VisualStudio.ProjectSystem.ProjectLockService.PrepareResourceForConcurrentAccessAsync(Project resource, CancellationToken cancellationToken)
   at Microsoft.VisualStudio.Threading.AsyncReaderWriterResourceLock`2.Helper.<.ctor>b__8_2(Task prev, Object state)
   at System.Threading.Tasks.ContinuationResultTaskFromTask`1.InnerInvoke()
   at System.Threading.Tasks.Task.Execute()

@jeffkl
Copy link
Contributor

jeffkl commented Feb 13, 2018

PR is sent, we'll take the bug to shiproom for approval

@Pilchie
Copy link
Member

Pilchie commented Feb 14, 2018

Thanks @jeffkl - resolving this as external.

@Pilchie Pilchie closed this as completed Feb 14, 2018
@Pilchie Pilchie added the Area-External Owned by another feature area and not this repo. Likely be closed in lieu of issue against it. label Feb 14, 2018
AndyGerlicher pushed a commit to dotnet/msbuild that referenced this issue Feb 14, 2018
…noreMissingImports is specified (#2991)

* Do not throw an InvalidProjectFileException if ProjectLoadSettings.IgnoreMissingImports is specified

Related to dotnet/project-system#3240
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-External Owned by another feature area and not this repo. Likely be closed in lieu of issue against it. Bug This is a functional issue in already written code. Feature-Project-File-Editing Editing the project either while open in Visual Studio, or via other mechanism.
Projects
None yet
Development

No branches or pull requests

4 participants