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

Does not parse ProjectReferences? #39

Closed
spiffytech opened this issue Jul 10, 2015 · 16 comments
Closed

Does not parse ProjectReferences? #39

spiffytech opened this issue Jul 10, 2015 · 16 comments

Comments

@spiffytech
Copy link

To reproduce:

  1. Start a new project from the F# Scaffold
  2. Run build.sh a first time to generate the project
  3. Open tests/sample.Tests/Tests.fs with Vim or Emacs, activate F# syntax checking

You'll see the editor complain that the sample module is undefined (sample.Tests.fsproj includes it as a ProjectReference). However, build.sh successfully compiles and runs the unit tests, so the project is set up correctly. I presume this indicates a bug in fsharpbinding?

Emacs complains about an undefined module/namespace. Vim gives me this:

Unable to find the file '/tmp/ProjectScaffold-master/tests/sample.Tests/./bin/Debug/sample.dll' in any of /usr/lib/mono/4.0 /tmp/ProjectScaffold-master/tests/sample.Tests ...

@rneatherway
Copy link
Contributor

Thanks for reporting, but I don't think this is a bug. Basically, it has parsed the project references, which is why it is expecting to find sample.dll under the output directory of the default configuration according to the test project file. This is bin/Debug as indicated by the Vim error message, but the ProjectScaffold builds in the Release configuration, so they aren't found. I'm pretty sure this would happen with Visual Studio as well. To work around it you just need to build in Debug mode (C-c C-c RET in emacs should do it while in Tests.fs), or change the default configuration in your project file to be Release. I think really this mismatch is a bug in ProjectScaffold.

@kjnilsson
Copy link
Contributor

for completeness just run :make in vim to achieve the same.

On Fri, 10 Jul 2015 at 11:42 Robin Neatherway notifications@github.com
wrote:

Thanks for reporting, but I don't think this is a bug. Basically, it has
parsed the project references, which is why it is expecting to find
sample.dll under the output directory of the default configuration
according to the test project file. This is bin/Debug as indicated by the
Vim error message, but the ProjectScaffold builds in the Release
configuration, so they aren't found. I'm pretty sure this would happen with
Visual Studio as well. To work around it you just need to build in Debug
mode (C-c C-c RET in emacs should do it while in Tests.fs), or change the
default configuration in your project file to be Release. I think really
this mismatch is a bug in ProjectScaffold.


Reply to this email directly or view it on GitHub
#39 (comment)
.

@dsyme
Copy link
Contributor

dsyme commented Jul 10, 2015

@rneatherway - I believe it should be possible to use FCS inter-project references to get multiple projects working without requiring the compiled DLLs on disk (Visual Studio F# Tools does require dependent DLLs on-disk for its checking functionality, but Visual F# Power Tools added functionality does not).

Are you using GetProjectOptionsFromProjectFile which does the recursive parsing of referenced project files?

@rneatherway
Copy link
Contributor

@dsyme thanks for mentioning it. I'm currently not, but it should be a simple change. I thought that it was still in testing, so I hadn't tried using it yet. If you think it is worth using I will definitely switch, that will be a nice improvement.

@dsyme
Copy link
Contributor

dsyme commented Jul 10, 2015

Yes, give it go, it should be stable.

The only area I know of that might need to be looked is what happens if a project in Release mode references a project in Debug mode (assuming that's even allowed). I've never quite worked out exactly how Visual Studio manages such things - is that info recorded in the solution file? I think it must be, since it seems absent from the project files.

@rneatherway
Copy link
Contributor

Yes, that's right, it is stored in the solution file. Projects contain a default value (which I have only ever seen to be Debug) so that they can be built separately. I expect that the referencing project would take priority, as the Configuration would already be set when the referenced project was imported.

We may have to consider parsing/using solution files at some point, see another discussion about implementing multi-project symbol usage.

@7sharp9
Copy link
Contributor

7sharp9 commented Jul 10, 2015

Juts a note: using GetProjectOptionsFromProjectFile adds quite a degree of latency (can be 3-4 seconds), I use an LRU cache with expiration based on the project changing.

@7sharp9
Copy link
Contributor

7sharp9 commented Jul 10, 2015

(And it currently doesn't work on Android, iOS classic, iOS unified, Mac unified, and MonoMac projects)

@dsyme
Copy link
Contributor

dsyme commented Jul 10, 2015

@7sharp9 - Does that apply specifically to GetProjectOptionsFromProjectFile (which @rneatherway is not yet using) or also to FSharpProjectFileInfo.Parse (which @rneatherway is already using)

@7sharp9
Copy link
Contributor

7sharp9 commented Jul 10, 2015

Either, its the parse which calls msbuild which causes the latency. To use it without caching I had to set the time to 10 seconds.

@7sharp9
Copy link
Contributor

7sharp9 commented Jul 10, 2015

Oh, and also if you do multiple calls without some form of lock it can cause a memory leak.

@dsyme
Copy link
Contributor

dsyme commented Jul 10, 2015

@7sharp9 - OK, thanks. Re locking - yes, this should be moved to the FCS reactor thread. I'll add an FCS issue on that. It would also make sense to add an FCS cache for this though I suppose that's lower priority.

@7sharp9
Copy link
Contributor

7sharp9 commented Jul 10, 2015

The thing with the cache is a project needs to be removed if it changes where it would result in different results, and possibly dependent projects may need to be recalculated depending on the change. At the moment I have project change event simply clear the cache.

@rneatherway
Copy link
Contributor

I assume the project change event is a Xamarin Studio thing? Perhaps I could use a FileSystemWatcher for the same thing.

@7sharp9
Copy link
Contributor

7sharp9 commented Jul 10, 2015

I currently clear on:

    x.OnFileAddedToProject
    x.OnFileRemovedFromProject
    x.OnFileRenamedInProject
    x.OnFilePropertyChangedInProject
    x.OnReferenceAddedToProject
    x.OnReferenceRemovedFromProject

@rneatherway
Copy link
Contributor

OK, I plan to fix this, but it will involve significant changes to the way that I get the project options, and I am looking at how best to remove the background agent anyway, so that will happen first.

rneatherway added a commit to rneatherway/FsAutoComplete that referenced this issue Jul 16, 2015
rneatherway added a commit to rneatherway/FsAutoComplete that referenced this issue Jul 27, 2015
* FSharp.CompilerBinding removed, and used parts absorbed. Fixes ionide#17.
* ScriptCheckerOptions fetched with no timeout, and also stores them.
  Fixes ionide#18, ionide#28.
* If a .fs file is not in a loaded project, produce an incomplete
  typecheck environment for it to give basic results.
* Update helptext command to return { Name = ""; Text = "" }. Fixes ionide#35.
* Update parsing of project options to include ProjectReferences. Fixes ionide#39.
* Separate parsing of commands, main command loop, and formatting of
  response message into separate modules.
rneatherway added a commit to rneatherway/FsAutoComplete that referenced this issue Jul 27, 2015
Backwards-incompatible changes:

* Update helptext command to return { Name = ""; Text = "" }. Fixes ionide#35.
* `project` command response now has 'null' for OutputFile and
  TargetFramework if a value cannot be determined.

Other changes:

* FSharp.CompilerBinding removed, and used parts absorbed. Fixes ionide#17.
* ScriptCheckerOptions fetched with no timeout, and also stores them.
  Fixes ionide#18, ionide#28.
* If a .fs file is not in a loaded project, produce an incomplete
  typecheck environment for it to give basic results.
* Update parsing of project options to include ProjectReferences. Fixes ionide#39.
* Separate parsing of commands, main command loop, and formatting of
  response message into separate modules.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants