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

project file unification take 2 #325

Merged
merged 43 commits into from
Jan 20, 2019
Merged

Conversation

baronfel
Copy link
Contributor

Ok lets try this again. Smooshed the projects together again, fixed up some dependency shenanigans, did some conditional includes where appropriate, and added some type annotations so that things would build on recent mono. Integration tests are green on my box, let's see what CI thinks.

@baronfel
Copy link
Contributor Author

Green! How does this look to everyone?

build.fsx Show resolved Hide resolved
@Krzysztof-Cieslak
Copy link
Member

@baronfel, can you try enabling also .Net Core tests?

global.json Outdated Show resolved Hide resolved
Copy link
Contributor

@enricosada enricosada left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i see

E:\github\fsharp\FsAutoComplete\src\FsAutoComplete\FsAutoComplete.fsproj : warning NU1701: Package 'FSharp.Compiler.Service.ProjectCracker 25.0.1' was restored using '.NETFramework,Version=v4.6.1' instead of the project target framework '.NETCoreApp,Version=v2.0'. This package may not be fully compatible with your project. [E:\github\fsharp\FsAutoComplete\FsAutoComplete.sln]

that shouldnt happen, projectcracker it shouldnt be added to the .net core version at all, otherwise pull msbuild assemblies

i checked and it add FSharp.Compiler.Service.ProjectCracker to references

seems to be pulled in by FSharpLint.Core 0.9.0-beta who has wrong deps, fixed version is 0.9.0

@ghost
Copy link

ghost commented Jan 16, 2019

On the project build side, I beleive that a mono paket is currently distributed in .paket, this means that a netcore only build will fail (dotnet build -f netcoreapp2.0). We now have paket as a dotnet cli tool, (on my machine I can just call paket restore), so it should be possible to preference that and fall back to mono if it doesn't exist?

$ dotnet build -f netcoreapp2.0
Microsoft (R) Build Engine version 15.9.20+g88f5fadfbe for .NET Core
Copyright (C) Microsoft Corporation. All rights reserved.

  /bin/sh: 2: /tmp/tmpbe2242f8ef15483fbb04c8dfed7fde4f.exec.cmd: mono: not found
/home/user/extra/FsAutoComplete/.paket/Paket.Restore.targets(111,5): error MSB3073: The command "mono --runtime=v4.0.30319 "/home/user/extra/FsAutoComplete/.paket/paket.exe" restore" exited with code 127. [/home/user/extra/FsAutoComplete/src/FsAutoComplete.SymbolCache/FsAutoComplete.SymbolCache.fsproj]

@enricosada
Copy link
Contributor

@abk-x yes, but afaik paket shouldnt be installed as global (-g), otherwise is not possible to use a specific version in the repo (and can be not in sync with the paket.restore.target or at least behave differently than ci)

@ghost
Copy link

ghost commented Jan 16, 2019

I got the build working by doing rm -rf .paket && paket install then dotnet build -f netstandard2.0 src/FsAutoComplete/FsAutoComplete.fsproj

@enricosada
Copy link
Contributor

enricosada commented Jan 17, 2019

linter exception:

  "Data": "Something went wrong during linter: System.TypeInitializationException: The type initializer for 'FSharpLint.Framework.Configuration' threw an exception. 
---> System.TypeInitializationException: The type initializer for '<StartupCode$FSharpLint-Core>.$Configuration' threw an exception. 
---> System.Exception: Resource 'DefaultConfiguration.FSharpLint' not found in assembly 'FSharpLint.Core, Version=0.9.0.0, Culture=neutral, PublicKeyToken=null'\n   at Microsoft.FSharp.Core.PrintfModule.PrintFormatToStringThenFail@1645.Invoke(String message)\n   at <StartupCode$FSharpLint-Core>.$Configuration..cctor()\n   
--- End of inner exception stack trace 
---\n   at FSharpLint.Framework.Configuration..cctor()\n   
--- End of inner exception stack trace ---\n   at FSharpLint.Application.ConfigurationManager.ConfigurationManager.GetConfigurationForProject(String projectFilePath)\n   at <StartupCode$FsAutoComplete-Core>.$Commands.Lint@544.Invoke(Unit unitVar) in E:/github/fsharp/FsAutoComplete/src/FsAutoComplete.Core/Commands.fs:line 558"

because the fsharplint v0.9.0 has the wrong resource name

image

@enricosada
Copy link
Contributor

status:

there is a .net core integration test failing ( https://ci.appveyor.com/project/rneatherway/fsautocomplete/builds/21679761#L585 ).
dunno the reason, i cannot repro locally.
tomorrow i'll try to RDP into appveyor and see, rerunning the same fsi will show the error probably

@Krzysztof-Cieslak
Copy link
Member

@enricosada, It's probably the discussion for a different issue but... Why do we have msbuild in PATH dependency for FSAC-netcore? As far as I can tell our msbuild detection (https://github.com/fsharp/FsAutoComplete/blob/master/src/FsAutoComplete.Core/Environment.fs#L63-L84) is accurate, so can't we just use the path we find with it?

@enricosada
Copy link
Contributor

enricosada commented Jan 17, 2019

@Krzysztof-Cieslak because it was the only way atm to choose what msbuild to use, if multiple are installed. the fsproj need to be built with the right msbuild.
Atm we cannot choose it (that's why the PR #251 )
The user can just open code from the correct vs command prompt. i use it like this a lot at work

but yes seems it is too fragile, so if not found in the PATH it should fallback to that detection and use the latest.

@enricosada
Copy link
Contributor

enricosada commented Jan 20, 2019

some test fails because fsac goes in deadlock, waiting for Project.Response in Commands.fs#L339

    member __.Response with get() = agent.PostAndReply GetResponse

i think that PostandReply deadlock, because the mailboxprocess raise an unhandled exception

the ci job show the failed test, but doesnt end.
that's another thing, dunno if that fake test runner handle well a dead locked fsautocomplete

@enricosada
Copy link
Contributor

enricosada commented Jan 20, 2019

@Krzysztof-Cieslak trying to use the msbuild of FSAC with c7a8917 instead of the one in PATH

@enricosada enricosada merged commit 231d449 into ionide:master Jan 20, 2019
@baronfel
Copy link
Contributor Author

Way to bring it home, @enricosada!

@enricosada
Copy link
Contributor

Ok merged, reenabled .net core std integration tests too.

Awesome work @baronfel 👍

atm linter on .NET FSAC doesnt work, see info in commit message
Let's continue work from master to fix disabled tests and maybe reenable http tests too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants