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

Loading some addins with loaddependencies prevents cake file from being loaded #163

Closed
nikola-nignite opened this issue Dec 10, 2021 · 12 comments · Fixed by #165
Closed
Labels
Milestone

Comments

@nikola-nignite
Copy link

nikola-nignite commented Dec 10, 2021

If I try to reference some more recent Microsoft nuget packages, VS Code simply does not offer any intellisense or debug support.

I tracked it down to how Cake.Bakery loads references.

In my case, I tried following:
#addin "nuget:?package=Microsoft.Data.SqlClient&loaddependencies=true&version=4.0.0

I added some logging to file to Cake.Bakery.Scripting and got following:

While finding aliases in ref c:/temp/base.artifacts/tools/Addins/Microsoft.Data.SqlClient.SNI.runtime.3.0.0/runtimes/win-x64/native/Microsoft.Data.SqlClient.SNI.dll
System.BadImageFormatException: Format of the executable (.exe) or library (.dll) is invalid.
at Mono.Cecil.PE.ImageReader.ReadOptionalHeaders(UInt16& subsystem, UInt16& dll_characteristics)
at Mono.Cecil.PE.ImageReader.ReadImage()
at Mono.Cecil.PE.ImageReader.ReadImage(Disposable1 stream, String file_name) at Mono.Cecil.ModuleDefinition.ReadModule(Disposable1 stream, String fileName, ReaderParameters parameters)
at Mono.Cecil.ModuleDefinition.ReadModule(String fileName, ReaderParameters parameters)
at Mono.Cecil.AssemblyDefinition.ReadAssembly(String fileName, ReaderParameters parameters)
at Cake.Scripting.CodeGen.CakeScriptAliasFinder.FindAliases(FilePath path) in C:\temp\bakery-develop\src\Cake.Scripting\CodeGen\CakeScriptAliasFinder.cs:line 33
at Cake.Bakery.Scripting.CachingScriptAliasFinder.FindAliases(FilePath path) in C:\temp\bakery-develop\src\Cake.Bakery\Scripting\CachingScriptAliasFinder.cs:line 28
at Cake.Scripting.CodeGen.CakeScriptGenerator.Generate(FileChange fileChange) in C:\temp\bakery-develop\src\Cake.Scripting\CodeGen\CakeScriptGenerator.cs:line 152

However, if I do not loaddependencies, then i will have to list them each manually in order for the script to execute correctly :(
I have to note that script does execute correctly even with loaddependencies switched on.

By the way, is there a guide on how to view log output of Cake.Bakery? I did not manage to find output in any of the VS Code windows, so I ended up manually writing to file :(

@gep13
Copy link
Member

gep13 commented Dec 10, 2021

@bjorkstromm any thoughts on this one?

I believe I am right in saying that you will see output from Bakery in the Omnisharp log which you can switch to here:

2021-12-10_07-19-46

@bjorkstromm
Copy link
Member

According to logs, Bakery is trying to look at a native library when searching for addins. This should be fixed. Only CLR assemblies should be inspected by Mono Cecil. We already have this check in Cake, so it should be easy to port to Bakery.

@nikola-nignite
Copy link
Author

Analysis of cake file isn't full story of this issue. While adding logging, I simply skipped over files which could not be loaded, but there is also an issue in later part of the file. I.e. after ignoring loading in part I described above, I still get error. But this time at least I can see it in OmniSharp Log pane.

image

@bjorkstromm, I don't see messages from Cake.Bakery in OmniSharp Log pane. I also tried changing VS Code settings.json, and playing with console, switching between internalConsole, externalTerminal and integratedTerminal.

{
    "security.workspace.trust.untrustedFiles": "open",
    "omnisharp.path": "latest",
    "cake.codeLens": {
        "showCodeLens": true,
        "installNetTool": true,
        "scriptsIncludePattern": "**/*.cake",
        "taskRegularExpression": "Task\\s*?\\(\\s*?\"(.*?)\"\\s*?\\)",
        "debugTask": {
            "verbosity": "diagnostic",
            "debugType": "coreclr",
            "request": "launch",
            "program": null,
            "cwd": "${workspaceRoot}",
            "stopAtEntry": true,
            "console": "internalConsole",
            "logging": {
                "exceptions": true,
                "moduleLoad": true,
                "programOutput": true,
                "engineLogging": true,
                "browserStdOut": true
            }
        }
    },
    "cake.taskRunner": {
        "autoDetect": true,
        "installNetTool": true,
        "scriptsIncludePattern": "**/*.cake",
        "scriptsExcludePattern": "",
        "taskRegularExpression": "Task\\s*?\\(\\s*?\"(.*?)\"\\s*?\\)",
        "launchCommand": null,
        "verbosity": "diagnostic"
    },
}

@nikola-nignite
Copy link
Author

I realized Cake.Bakery logging depends also on OmniSharp logging level. But this realization did not help much :D

image

After setting OmniSharp level to Debug and Trace, Cake.Bakery still doesn't log anything. Note that log level is correctly passed to Omnisharp.exe, but it's not propagated further to Cake.Bakery.exe.

2021-12-10 14_42_39-Process Explorer - Sysinternals_ www sysinternals com  LMBW2K_lmbran0

@gep13
Copy link
Member

gep13 commented Dec 10, 2021

@nikola-nignite this is completely unrelated to your problem, but where are you getting those screenshots for the arguments that were passed into the executable? I have never seen these before, and now I feel like a noob!

@bjorkstromm
Copy link
Member

Analysis of cake file isn't full story of this issue. While adding logging, I simply skipped over files which could not be loaded, but there is also an issue in later part of the file. I.e. after ignoring loading in part I described above, I still get error. But this time at least I can see it in OmniSharp Log pane.

image

These logs seems to indicate that native assemblies are passed as metadata references to the compilation. This is a bug. Only CLR assemblies should be passed. Native deps should be filtered out in Bakery. Even better if we could filter out all runtime-dependencies. IIRC, Cake.NuGet "should" already support this, so don't know why this is getting through all the way to the OmniSharp compilation.

@bjorkstromm
Copy link
Member

IIRC, Cake.NuGet "should" already support this, so don't know why this is getting through all the way to the OmniSharp compilation.

My bad... In Cake we ACTUALLY want runtime deps. In Bakery we don't. So this might be the issue.

@nikola-nignite
Copy link
Author

@gep13 Apologies about that. True, they are unrelated issues.
Screenshots come from Process Explorer. It also allows you to see process tree, so that you don't confuse matching instances of OmniSharp.exe and Cake.Bakery.exe

Submitted separate issue #164 . Albeit, it's debatable whether it's Cake.Bakery or OmniSharp bug.

image

devlead added a commit to devlead/bakery that referenced this issue Dec 10, 2021
augustoproiete added a commit that referenced this issue Dec 10, 2021
GH163: Only find aliases in CLR assemblies
@devlead devlead added the Bug label Dec 11, 2021
@devlead devlead added this to the 0.9.1 milestone Dec 11, 2021
@cake-build-bot
Copy link

🎉 This issue has been resolved in version 0.9.1 🎉

The release is available on:

Your GitReleaseManager bot 📦🚀

@nikola-nignite
Copy link
Author

nikola-nignite commented Dec 11, 2021

Thanks for pushing a fast update.
But unfortunately I still experience same issue, with the new release.

One note when testing this fix:
OmniSharp (seems) to cache some of the provided IntelliSense hints.
Looking at screen shot provided in PR, it seems that IntelliSense is working. This happens to me as well under following conditions (it may have happened under different conditions in case of that screen shot):

  • Add #addin reference without loaddependencies
  • Try intellisense, and it works correctly
  • Add loaddependencies parameter to #addin directive, and intellisense keeps working

But now in OmniSharp log I get logged following:

************ Response (92.0336ms) ************
{
"Request_seq": 250,
"Command": "/updatebuffer",
"Running": true,
"Success": false,
"Message": ""System.BadImageFormatException: PE image doesn't contain managed metadata.\r\n at Microsoft.CodeAnalysis.PEModule.InitializeMetadataReader()\r\n at Microsoft.CodeAnalysis.PEModule.get_MetadataReader()\r\n at Microsoft.CodeAnalysis.PEAssembly..ctor(AssemblyMetadata owner, ImmutableArray1 modules)\\r\\n at Microsoft.CodeAnalysis.AssemblyMetadata.GetOrCreateData()\\r\\n at Microsoft.CodeAnalysis.AssemblyMetadata.GetModules()\\r\\n at OmniSharp.Services.MetadataFileReferenceCache.GetMetadataReference(String filePath) in D:\\\\a\\\\1\\\\s\\\\src\\\\OmniSharp.Roslyn\\\\Services\\\\MetadataFileReferenceCache.cs:line 49\\r\\n at OmniSharp.Cake.CakeProjectSystem.<GetMetadataReferences>d__39.MoveNext() in D:\\\\a\\\\1\\\\s\\\\src\\\\OmniSharp.Cake\\\\CakeProjectSystem.cs:line 328\\r\\n at OmniSharp.Cake.CakeProjectSystem.ScriptReferencesChanged(Object sender, ReferencesChangedEventArgs e) in D:\\\\a\\\\1\\\\s\\\\src\\\\OmniSharp.Cake\\\\CakeProjectSystem.cs:line 212\\r\\n at OmniSharp.Cake.Services.CakeScriptService.Generate(FileChange fileChange) in D:\\\\a\\\\1\\\\s\\\\src\\\\OmniSharp.Cake\\\\Services\\\\CakeScriptService.cs:line 99\\r\\n at OmniSharp.Cake.Services.RequestHandlers.Buffer.UpdateBufferHandler.<Handle>d__3.MoveNext() in D:\\\\a\\\\1\\\\s\\\\src\\\\OmniSharp.Cake\\\\Services\\\\RequestHandlers\\\\Buffer\\\\UpdateBufferHandler.cs:line 60\\r\\n--- End of stack trace from previous location where exception was thrown ---\\r\\n at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()\\r\\n at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)\\r\\n at OmniSharp.Endpoint.EndpointHandler2.d__19.MoveNext()\r\n--- End of stack trace from previous location where exception was thrown ---\r\n at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()\r\n at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)\r\n at OmniSharp.Endpoint.EndpointHandler2.<HandleRequestForLanguage>d__20.MoveNext() in D:\\\\a\\\\1\\\\s\\\\src\\\\OmniSharp.Host\\\\Endpoint\\\\EndpointHandler.cs:line 230\\r\\n--- End of stack trace from previous location where exception was thrown ---\\r\\n at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()\\r\\n at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)\\r\\n at OmniSharp.Endpoint.EndpointHandler2.d__16.MoveNext() in D:\\a\\1\\s\\src\\OmniSharp.Host\\Endpoint\\EndpointHandler.cs:line 131\r\n--- End of stack trace from previous location where exception was thrown ---\r\n at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()\r\n at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)\r\n at OmniSharp.Stdio.Host.d__14.MoveNext() in D:\\a\\1\\s\\src\\OmniSharp.Stdio\\Host.cs:line 218"",
"Body": null,
"Seq": 269,
"Type": "response"
}

Software I used:
VS Code 1.63.0
OmniSharp 1.37.17
Cake.Bakery 0.91 - This I verified multiple times. Did not believe result, so I also manually downloaded release ZIP and copied over. One interesting thing (for me), was that Cake.Bakery.exe is not binary identical between .nupkg and .zip. And also that in Details tab of File properties it says that version of Cake.Bakery.exe is 1.0.0. I believe this part you knew about

image

@devlead
Copy link
Member

devlead commented Dec 11, 2021

nupkg contains a CILproxy exe to work cross-platform with Omnisharps embedded mono runtime which is used on non-Windows OSs.
It just launches Cake.Bakery.dll which is the actual .NET 6 assembly.
It would seem we need to exclude non CIL assemblies earlier

@gep13
Copy link
Member

gep13 commented Dec 13, 2021

@nikola-nignite said...
Apologies about that. True, they are unrelated issues.

There is absolutely no need to apologise here, it was me that was apologising! I was the one asking the question that had nothing to do with this thread. I had no idea that you could get information about the command line argument passed through to the process within Process Explorer. Thank you for sharing that information!

And thank you for creating the follow up issue.

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

Successfully merging a pull request may close this issue.

5 participants