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

Nothing works on ProjectSystem project #2970

Closed
vasily-kirichenko opened this issue May 1, 2017 · 10 comments
Closed

Nothing works on ProjectSystem project #2970

vasily-kirichenko opened this issue May 1, 2017 · 10 comments
Labels
Bug Impact-Medium (Internal MS Team use only) Describes an issue with moderate impact on existing code. Ready
Milestone

Comments

@vasily-kirichenko
Copy link
Contributor

1

@saul I may find interesting the strange behavior with trying to save the project file.

@saul
Copy link
Contributor

saul commented May 1, 2017

I'm also seeing this, see #2788 (comment).

I only really see it in the debug hive - I've noticed that if I build from build.cmd instead of Visual Studio, it seems to work better. "Sequence contains more than one element" is an error I saw when trying to add a reference to a project a week ago or so... so that may be another way to reproduce this.

@saul
Copy link
Contributor

saul commented May 1, 2017

Okay now I'm seeing this on my normal instance - it was affecting every project. It was triggered by reloading the solution. Restarting VS fixed it however 😕

@cartermp cartermp modified the milestones: VS 2017 RTM, VS 2017 Updates May 1, 2017
@dsyme dsyme added Impact-Medium (Internal MS Team use only) Describes an issue with moderate impact on existing code. Ready labels May 8, 2017
@dsyme
Copy link
Contributor

dsyme commented May 9, 2017

@vasily-kirichenko Things are now working in this project for me, please reopen if you continue to have problems

(Intellisense take a long while to first appear due to need to first typecheck the rest of the solution)

@dsyme dsyme closed this as completed May 9, 2017
@dsyme
Copy link
Contributor

dsyme commented May 9, 2017

(The issues in this project were all about the behavior of IDE features in the presence of missing transitive assembly references. It is worth testing new IDE features with this project now and then as a test case to see if a new IDE feature works)

@vasily-kirichenko
Copy link
Contributor Author

Now I have this issue on OSS VSIX release as well.

  1. git clean -dfx
  2. build vs debug (otherwise nothing will work for any project in VFT solution)
  3. build vs
  4. release\net40\bin\VisualFSharpOpenSource.vsix

result

image

@dsyme
Copy link
Contributor

dsyme commented May 10, 2017

build vs debug (otherwise nothing will work for any project in VFT solution)

This should not be needed. Will look at it.

@dsyme
Copy link
Contributor

dsyme commented May 10, 2017

@vasily-kirichenko OK I can repro some of these problems

  1. git clean -dfx
  2. build vs
  3. VSIXInstaller.exe /u:"VisualFSharp" (admin prompt)
  4. VSIXInstaller.exe release\net40\bin\VisualFSharpOpenSource.vsix (admin prompt)

and opened a script and things worked ok. But when opening VisualFSharp.sln I saw the problems where it tries to reference the "Debug" FSharp.Core DLL

@dsyme
Copy link
Contributor

dsyme commented May 10, 2017

There are twoproblems

  1. Major: The treatment of cached project options and other tables in LanguageService.fs and ProjectSitesAndFiles.fs is wrong when the active solution configurations change . (Note, that code is also known to be the cause of our major problems with other changes such as solution unload/load/reload). In this case, VisualStudio starts up with an empty and/or Debug configuration and then sends an OnActiveProjectCfgChange notification. When processing this we do something wrong - either we fail to flush the project options table, or something like that. Hence even when opening VisualFSharp.sln in "Release" configuration we still get the error about missing "Debug" dll.

  2. Minor: When editing VIsualFSharp.sln (i.e. when FSharp.Core is a project reference) we are requiring a physical FSharp.Core.dll on disk rather than using a virtualized FCS cross-project assembly reference. The requirement for a physical file on disk stems from fsharpBinariesDirValue. This is not a big deal as it only affects compiler developers since other users don't have FSharp.Core.fsproj as a cross-project reference

The systematic problem we as a group are having is that we have too easily been introducing changes, features and performance work in the Language Service (e.g. LanguageService.fs) under the assumption that projects, solutions and the active configuration are static things. However in reality projects change over time - as does the active configuration.

@saul's PR #2909 may be helpful and seems to do things much more correctly. However I also suspect that we should first do a less ambitious PR focused entirely on correctness under changing the active configuration, solution load/reload, and/remove project, add/remove project references, add/remove files, and then later focus on performance.

We actually have hundreds of tests for this sort of thing but they exercise the "old" non-Roslyn logic. So it feels like we have lost significant test coverage and/or basic testability in the process of Roslynizing the code.

Lessons:

  1. We have to dig out a number of significant regressions and do a lot more testing around changing configurations etc.

  2. We have to make this stuff more testable and perhaps try to recover our unit testing

  3. Further churn may not help things, we have to be careful not to make the situation worse,

@dsyme
Copy link
Contributor

dsyme commented May 10, 2017

@saul When you get the chance could you DM me on twitter , or email me? I'd like to skype-chat to run a few questions by you about how/where to maintain the correct FSharpProjectOptions objects in the face of configuration changes, project reloads etc.

@dsyme
Copy link
Contributor

dsyme commented Jun 20, 2017

This was fixed a while back

@dsyme dsyme closed this as completed Jun 20, 2017
@cartermp cartermp modified the milestones: VS 2017 Updates, 15.3 Jun 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Impact-Medium (Internal MS Team use only) Describes an issue with moderate impact on existing code. Ready
Projects
None yet
Development

No branches or pull requests

4 participants