Cake support #932

bjorkstromm · 2017-08-02T20:55:02Z

Opening PR to get feedback.

Omnisharp.Cake piggybacks on OmniSharp.Roslyn.CSharp services, but will intercept and handle buffer updates before sending them to C# services... This is how it works.

We have this custom tool Bakery which is doing Cake script analysis and CodeGen
When OmniSharp starts up, a bakery process is started and a communication channel is opened between omnisharp.exe and cake.bakery.exe
Whenever a buffer update occurs for a *.cake file it is sent to the bakery process in order to get analysed and a Roslyn compatible script is returned.
The roslyn compatible script is added to the buffer manager and so on....

All feedback is welcome! @david-driscoll @DustinCampbell @filipw

here is a small example project if you like to test it out. Please note that Cake.Bakery (which can be fetched from our MyGet feed) needs to be located in the workspace before starting OmniSharp. It should be located in the tools folder. E.g. ${workspaceRoot}/tools/ folder or any other folder specified in cake.config.

~~Will send accompanying PR in Omnisharp-VSCode repo soon.~~

TODO:

~~Don't hardcode Cake.Bakery.exe path to $(toolsdir)/Cake.Bakery/tools/Cake.Bakery.exe~~
~~Fix lineoffsets when loading nested .cake files~~
~~Add documentation provider when loading addins~~
~~Load downloaded addins added after server was started~~
~~Add Unit Tests~~
~~Add integration tests~~
~~Cleanup~~

bjorkstromm · 2017-08-02T21:11:31Z

Here's a small preview.

bjorkstromm · 2017-08-02T21:55:16Z

src/OmniSharp.Cake/CakeProjectSystem.cs

+        {
+            var name = Path.GetFileName(filePath);
+
+            var assembly = AssemblyLoader.LoadFrom(cakeScript.Host.AssemblyPath);


Use IAssemblyLoader

bjorkstromm · 2017-10-05T19:20:44Z

src/OmniSharp.Cake/OmniSharp.Cake.csproj

+  <ItemGroup>
+    <PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="2.3.1" />
+    <PackageReference Include="Microsoft.CodeAnalysis.CSharp.Scripting" Version="2.3.1" />
+    <PackageReference Include="Cake.Scripting.Transport" Version="0.1.0-unstable0091" />


Should be changed to 0.1.0 once Bakery is finalized.

david-driscoll · 2017-10-06T15:08:41Z

NuGet.Config

@@ -5,5 +5,6 @@
        <add key="NuGet" value="https://api.nuget.org/v3/index.json" />
        <add key="dotnet-core" value="https://dotnet.myget.org/F/dotnet-core/api/v3/index.json" />
        <add key="dotnet-cli" value="https://dotnet.myget.org/F/cli-deps/api/v3/index.json" />
+        <add key="cake-nightly" value="https://www.myget.org/F/cake/api/v3/index.json" />


Is this feed permanent? I assume it not, but just checking. :)

Not permanent. Only used until Cake.Scripting bits (alias bakery) is finalized and pushed to NuGet.

david-driscoll · 2017-10-07T13:41:11Z

src/OmniSharp.Cake/Configuration/CakeConfiguration.cs

+            var configurationPath = Path.Combine(environment.TargetDirectory, "cake.config");
+            if (File.Exists(configurationPath))
+            {
+                var parser = new ConfigurationParser();


Parser will throw if the config is found among other things, if this throws in the constructor that may be a very very bad thing when booting up MEF.

The parser should probably either not throw, or should we should catch specific exceptions that don't matter.

david-driscoll · 2017-10-07T13:42:13Z

src/OmniSharp.Cake/Configuration/Parser/ConfigurationParser.cs

+            var value = tokens.Current.Value;
+            if (ContainsWhiteSpace(value))
+            {
+                throw new InvalidOperationException("Sections cannot contain whitespace.");


What does cake do in this scenario, does it throw if the config has invalid values or does it default those values?

Configuration handling is "copy-pasted-with-pride" from the Cake repo. Now that I look at it, I should remove any exception throwing and just set sensible defaults.

A second set of 👀 always helps!

I went the Catch-and-Log route instead. Makes more sense than silently continue and keeps code in sync with code in Cake.

- Allow updating metadatareferences and using on script change

bjorkstromm · 2017-10-18T18:17:47Z

@filipw @david-driscoll sounds like a great idea!

DustinCampbell · 2017-10-18T18:34:51Z

FWIW, Travis CI failed on Linux. It looks like a real test failure in the Cake unit tests.

Examples:

39;49m�[31m    OmniSharp.Cake.Tests.AutoCompleteFacts.ShouldGenerateFromHostObject [FAIL]
�[39;49m�[37m      System.NullReferenceException : Object reference not set to an instance of an object
�[39;49m�[30m      Stack Trace:
�[39;49m�[37m          at OmniSharp.Cake.Services.CakeScriptService.Generate (Cake.Scripting.Abstractions.Models.FileChange fileChange) [0x00001] in <c4e6a1d4b56f401e87edf025a609f888>:0 
�[39;49m�[37m          at OmniSharp.Cake.Services.RequestHandlers.Buffer.UpdateBufferHandler+<Handle>d__3.MoveNext () [0x0016f] in <c4e6a1d4b56f401e87edf025a609f888>:0 
�[39;49m�[37m        --- End of stack trace from previous location where exception was thrown ---
�[39;49m�[37m          at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw () [0x0000c] in <c9f8153c41de4f8cbafd0e32f9bf6b28>:0 
�[39;49m�[37m          at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess (System.Threading.Tasks.Task task) [0x0003e] in <c9f8153c41de4f8cbafd0e32f9bf6b28>:0 
�[39;49m�[37m          at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification (System.Threading.Tasks.Task task) [0x00028] in <c9f8153c41de4f8cbafd0e32f9bf6b28>:0 
�[39;49m�[37m          at System.Runtime.CompilerServices.TaskAwaiter.ValidateEnd (System.Threading.Tasks.Task task) [0x00008] in <c9f8153c41de4f8cbafd0e32f9bf6b28>:0 
�[39;49m�[37m          at System.Runtime.CompilerServices.TaskAwaiter`1[TResult].GetResult () [0x00000] in <c9f8153c41de4f8cbafd0e32f9bf6b28>:0 
�[39;49m�[37m          at OmniSharp.Cake.Tests.AutoCompleteFacts+<FindCompletionsAsync>d__5.MoveNext () [0x00273] in <a83d90541b74416a88991defa1662a99>:0 
�[39;49m�[37m        --- End of stack trace from previous location where exception was thrown ---
�[39;49m�[37m          at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw () [0x0000c] in <c9f8153c41de4f8cbafd0e32f9bf6b28>:0 
�[39;49m�[37m          at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess (System.Threading.Tasks.Task task) [0x0003e] in <c9f8153c41de4f8cbafd0e32f9bf6b28>:0 
�[39;49m�[37m          at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification (System.Threading.Tasks.Task task) [0x00028] in <c9f8153c41de4f8cbafd0e32f9bf6b28>:0 
�[39;49m�[37m          at System.Runtime.CompilerServices.TaskAwaiter.ValidateEnd (System.Threading.Tasks.Task task) [0x00008] in <c9f8153c41de4f8cbafd0e32f9bf6b28>:0 
�[39;49m�[37m          at System.Runtime.CompilerServices.TaskAwaiter`1[TResult].GetResult () [0x00000] in <c9f8153c41de4f8cbafd0e32f9bf6b28>:0 
�[39;49m�[37m          at OmniSharp.Cake.Tests.AutoCompleteFacts+<ShouldGenerateFromHostObject>d__4.MoveNext () [0x00072] in <a83d90541b74416a88991defa1662a99>:0 
�[39;49m�[37m        --- End of stack trace from previous location where exception was thrown ---
�[39;49m�[37m          at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw () [0x0000c] in <c9f8153c41de4f8cbafd0e32f9bf6b28>:0 
�[39;49m�[37m          at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess (System.Threading.Tasks.Task task) [0x0003e] in <c9f8153c41de4f8cbafd0e32f9bf6b28>:0 
�[39;49m�[37m          at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification (System.Threading.Tasks.Task task) [0x00028] in <c9f8153c41de4f8cbafd0e32f9bf6b28>:0 
�[39;49m�[37m          at System.Runtime.CompilerServices.TaskAwaiter.ValidateEnd (System.Threading.Tasks.Task task) [0x00008] in <c9f8153c41de4f8cbafd0e32f9bf6b28>:0 
�[39;49m�[37m          at System.Runtime.CompilerServices.TaskAwaiter.GetResult () [0x00000] in <c9f8153c41de4f8cbafd0e32f9bf6b28>:0 
�[39;49m�[37m        --- End of stack trace from previous location where exception was thrown ---
�[39;49m�[37m          at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw () [0x0000c] in <c9f8153c41de4f8cbafd0e32f9bf6b28>:0 
�[39;49m�[37m          at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess (System.Threading.Tasks.Task task) [0x0003e] in <c9f8153c41de4f8cbafd0e32f9bf6b28>:0 
�[39;49m�[37m          at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification (System.Threading.Tasks.Task task) [0x00028] in <c9f8153c41de4f8cbafd0e32f9bf6b28>:0 
�[39;49m�[37m          at System.Runtime.CompilerServices.TaskAwaiter.ValidateEnd (System.Threading.Tasks.Task task) [0x00008] in <c9f8153c41de4f8cbafd0e32f9bf6b28>:0 
�[39;49m�[37m          at System.Runtime.CompilerServices.TaskAwaiter.GetResult () [0x00000] in <c9f8153c41de4f8cbafd0e32f9bf6b28>:0 
�[39;49m�[37m        --- End of stack trace from previous location where exception was thrown ---
�[39;49m�[37m          at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw () [0x0000c] in <c9f8153c41de4f8cbafd0e32f9bf6b28>:0 
�[39;49m�[37m          at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess (System.Threading.Tasks.Task task) [0x0003e] in <c9f8153c41de4f8cbafd0e32f9bf6b28>:0 
�[39;49m�[37m          at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification (System.Threading.Tasks.Task task) [0x00028] in <c9f8153c41de4f8cbafd0e32f9bf6b28>:0 
�[39;49m�[37m          at System.Runtime.CompilerServices.TaskAwaiter.ValidateEnd (System.Threading.Tasks.Task task) [0x00008] in <c9f8153c41de4f8cbafd0e32f9bf6b28>:0 
�[39;49m�[37m          at System.Runtime.CompilerServices.TaskAwaiter.GetResult () [0x00000] in <c9f8153c41de4f8cbafd0e32f9bf6b28>:0

�[39;49m�[31m    OmniSharp.Cake.Tests.LineIndexHelperFacts.TranslateToGenerated_Should_Translate_Index_In_Single_File(index: 0, expected: 8208) [FAIL]
�[39;49m�[37m      System.InvalidOperationException : Could not load manifest resource stream.
�[39;49m�[30m      Stack Trace:
�[39;49m�[37m          at OmniSharp.Cake.Tests.LineIndexHelperFacts.GetResourceContent (System.String resourceName) [0x00017] in <a83d90541b74416a88991defa1662a99>:0 
�[39;49m�[37m          at OmniSharp.Cake.Tests.LineIndexHelperFacts.GetGeneratedFileContent (System.String name) [0x00016] in <a83d90541b74416a88991defa1662a99>:0 
�[39;49m�[37m          at OmniSharp.Cake.Tests.LineIndexHelperFacts+<TranslateToGenerated_Should_Translate_Index_In_Single_File>d__12.MoveNext () [0x0001a] in <a83d90541b74416a88991defa1662a99>:0 
�[39;49m�[37m        --- End of stack trace from previous location where exception was thrown ---
�[39;49m�[37m          at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw () [0x0000c] in <c9f8153c41de4f8cbafd0e32f9bf6b28>:0 
�[39;49m�[37m          at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess (System.Threading.Tasks.Task task) [0x0003e] in <c9f8153c41de4f8cbafd0e32f9bf6b28>:0 
�[39;49m�[37m          at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification (System.Threading.Tasks.Task task) [0x00028] in <c9f8153c41de4f8cbafd0e32f9bf6b28>:0 
�[39;49m�[37m          at System.Runtime.CompilerServices.TaskAwaiter.ValidateEnd (System.Threading.Tasks.Task task) [0x00008] in <c9f8153c41de4f8cbafd0e32f9bf6b28>:0 
�[39;49m�[37m          at System.Runtime.CompilerServices.TaskAwaiter.GetResult () [0x00000] in <c9f8153c41de4f8cbafd0e32f9bf6b28>:0 
�[39;49m�[37m        --- End of stack trace from previous location where exception was thrown ---
�[39;49m�[37m          at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw () [0x0000c] in <c9f8153c41de4f8cbafd0e32f9bf6b28>:0 
�[39;49m�[37m          at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess (System.Threading.Tasks.Task task) [0x0003e] in <c9f8153c41de4f8cbafd0e32f9bf6b28>:0 
�[39;49m�[37m          at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification (System.Threading.Tasks.Task task) [0x00028] in <c9f8153c41de4f8cbafd0e32f9bf6b28>:0 
�[39;49m�[37m          at System.Runtime.CompilerServices.TaskAwaiter.ValidateEnd (System.Threading.Tasks.Task task) [0x00008] in <c9f8153c41de4f8cbafd0e32f9bf6b28>:0 
�[39;49m�[37m          at System.Runtime.CompilerServices.TaskAwaiter.GetResult () [0x00000] in <c9f8153c41de4f8cbafd0e32f9bf6b28>:0 
�[39;49m�[37m        --- End of stack trace from previous location where exception was thrown ---
�[39;49m�[37m          at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw () [0x0000c] in <c9f8153c41de4f8cbafd0e32f9bf6b28>:0 
�[39;49m�[37m          at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess (System.Threading.Tasks.Task task) [0x0003e] in <c9f8153c41de4f8cbafd0e32f9bf6b28>:0 
�[39;49m�[37m          at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification (System.Threading.Tasks.Task task) [0x00028] in <c9f8153c41de4f8cbafd0e32f9bf6b28>:0 
�[39;49m�[37m          at System.Runtime.CompilerServices.TaskAwaiter.ValidateEnd (System.Threading.Tasks.Task task) [0x00008] in <c9f8153c41de4f8cbafd0e32f9bf6b28>:0 
�[39;49m�[37m          at System.Runtime.CompilerServices.TaskAwaiter.GetResult () [0x00000] in <c9f8153c41de4f8cbafd0e32f9bf6b28>:0

bjorkstromm · 2017-10-18T18:37:11Z

@DustinCampbell yes, it feels like some problem using cake.bakery.exe on Linux within the unit tests. Will try to spin up the old virtual machine and give it a go later tonight.

bjorkstromm · 2017-10-22T20:51:34Z

@david-driscoll @filipw @DustinCampbell removed [WIP] tag now 😉

filipw · 2017-10-23T06:16:06Z

src/OmniSharp.Cake/Services/RequestHandlers/Navigation/GotoDefinitionHandler.cs

+            }
+            else
+            {
+                lineIndex = request.Line + 7;


these arbitrary offsets look scary 😄

Yes it is scary, but it has a simple explanation. As GotoDefinition would normally point to the generated DSL, we are instead rerouting to the original extension method (from which the DSL was generated). Ugly, but it works 😄 7 line offset for properties, 3 line offset for methods.

filipw · 2017-10-23T06:22:55Z

src/OmniSharp.Cake/CakeProjectSystem.cs

+            {
+                var document = solution.GetDocument(documentId);
+                var project = document.Project;
+                var compilationOptions = GetCompilationOptions(e.Usings);


this is going to be very inefficient, why not just call compilationOptions.WithUsings()?

Ah, good catch!

DustinCampbell · 2017-10-24T17:25:36Z

I just took a look at the changes to launch cake.bakery.exe. Looks good to me!

bjorkstromm · 2017-10-24T17:29:00Z

@DustinCampbell nice! I’ll just fix @filipw’s comments then. @david-driscoll do you have anything to add?

david-driscoll · 2017-10-24T17:39:25Z

Nothing else from me.

filipw · 2017-10-24T18:28:40Z

Out of curiosity, why do some files have a header:

// 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.

are they copied from somewhere?

bjorkstromm · 2017-10-24T18:37:21Z

@filipw, yes. They are copied from Cake, which is licensed under MIT license. We didn’t want OmniSharp to take on a dependency on Cake.Core and therefore it’ copied. It should only be the configuration bits.

DustinCampbell · 2017-10-24T21:25:50Z

Per the comment, shouldn't we include the MIT License in the folder with these files? I did the same thing for OmniSharp.DotNet.ProjectModel after copying all of the code over from the .NET CLI.

bjorkstromm · 2017-10-24T21:27:50Z

Sure, I can do that!

gep13 · 2017-10-25T06:01:55Z

Great work @mholo65!

Can't wait to get this merged in, and start to roll it out!

filipw

filipw · 2017-10-25T06:16:27Z

Since everyone signed off, I will merge it 🎉

bjorkstromm mentioned this pull request Aug 2, 2017

[WIP] Cake support dotnet/vscode-csharp#1681

Merged

bjorkstromm commented Aug 2, 2017

View reviewed changes

bjorkstromm mentioned this pull request Aug 4, 2017

Move OmniSharp to target net46 only #915

Merged

gep13 mentioned this pull request Aug 4, 2017

[QUESTION] Cake support in Omnisharp dotnet/vscode-csharp#1684

Closed

bjorkstromm commented Oct 5, 2017

View reviewed changes

david-driscoll requested changes Oct 7, 2017

View reviewed changes

bjorkstromm added 23 commits October 10, 2017 22:43

Adds OmniSharp.Cake project and Cake project system

a651d6a

Project System now loads Cake files

d9cd7a6

Remove Cake.Core dependency

a1bcaba

Use Cake Configuration

054ecf9

Adds Buffer handlers for Cake

3fec513

Added more services

3635771

Adds context model to Cake

ec9af89

Fix hostObjectType loading for Cake

e6225ac

Update dependencies

c46a223

Remove target netstandard1.6

fa78f27

Use IAssemblyLoader

b4b9b3f

Use TargetFramework instead of TargetFrameworks

5c6760e

Add DocumentationProvider

392c2b8

Reorder request handlers

6f66afa

Refactor Cake Script Service

e78d168

- Allow updating metadatareferences and using on script change

Fix line number translations to work when a cake file is loaded.

063fe25

Cache references and using and react on changes

55b6b27

Find Cake.Bakery if path contains version

f207ceb

Fix translation of lines for requests and responses

f616e2e

Added unit test project and fixed some translations.

e7b24c4

Support method and property aliases in gotodefinition handler.

3f61d56

Git ignore launchSettings.json

7299149

Git ignore Rider stuff

7340b9f

bjorkstromm and others added 8 commits October 18, 2017 22:03

Address code review comments.

e126ff4

Merge branch 'master' into feature/cake

54c3bab

Make Cake Bakery resolution case insensitive

e2dd7df

Update Cake.Scripting and Bakery versions

a6194f6

Add mono wrapper for executing bakery

9fecd07

Fix LineIndexHelper and tests on Linux/OSX

eb02fde

Add gotodefinition tests.

f6afebf

Update to release bits of Cake.Scripting and Bakery

e0cdfa5

bjorkstromm changed the title ~~[WIP] Cake support~~ Cake support Oct 22, 2017

filipw reviewed Oct 23, 2017

View reviewed changes

bjorkstromm added 3 commits October 24, 2017 22:03

CakeConfiguration was never used.

50ad596

Enhance compilation options usage in project system.

04825e0

Removed some magic integers.

48a2e73

Added MIT License for code copied from Cake repo

8a59058

filipw approved these changes Oct 25, 2017

View reviewed changes

filipw merged commit 329d28a into OmniSharp:master Oct 25, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cake support #932

Cake support #932

bjorkstromm commented Aug 2, 2017 •

edited

bjorkstromm commented Aug 2, 2017

bjorkstromm Aug 2, 2017

bjorkstromm Oct 5, 2017

david-driscoll Oct 6, 2017

bjorkstromm Oct 8, 2017

david-driscoll Oct 7, 2017

david-driscoll Oct 7, 2017

bjorkstromm Oct 8, 2017

david-driscoll Oct 9, 2017

bjorkstromm Oct 10, 2017

bjorkstromm commented Oct 18, 2017

DustinCampbell commented Oct 18, 2017

bjorkstromm commented Oct 18, 2017

bjorkstromm commented Oct 22, 2017

filipw Oct 23, 2017

bjorkstromm Oct 23, 2017

filipw Oct 23, 2017

bjorkstromm Oct 23, 2017

DustinCampbell commented Oct 24, 2017

bjorkstromm commented Oct 24, 2017

david-driscoll commented Oct 24, 2017

filipw commented Oct 24, 2017

bjorkstromm commented Oct 24, 2017

DustinCampbell commented Oct 24, 2017

bjorkstromm commented Oct 24, 2017

gep13 commented Oct 25, 2017

filipw left a comment

filipw commented Oct 25, 2017

Cake support #932

Cake support #932

Conversation

bjorkstromm commented Aug 2, 2017 • edited

bjorkstromm commented Aug 2, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bjorkstromm commented Oct 18, 2017

DustinCampbell commented Oct 18, 2017

bjorkstromm commented Oct 18, 2017

bjorkstromm commented Oct 22, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DustinCampbell commented Oct 24, 2017

bjorkstromm commented Oct 24, 2017

david-driscoll commented Oct 24, 2017

filipw commented Oct 24, 2017

bjorkstromm commented Oct 24, 2017

DustinCampbell commented Oct 24, 2017

bjorkstromm commented Oct 24, 2017

gep13 commented Oct 25, 2017

filipw left a comment

Choose a reason for hiding this comment

filipw commented Oct 25, 2017

bjorkstromm commented Aug 2, 2017 •

edited