GustavEikaas / easy-dotnet.nvim

Neovim plugin written in Lua for working with .Net projects in Neovim. Makes it easier to run/test/build/debug projects. Supports both F# and C#
MIT License
85 stars 7 forks source link

On OSX, DLL path discovery is not correct #143

Closed alexciarlillo closed 2 weeks ago

alexciarlillo commented 3 weeks ago

Took a bit of debugging but I discovered what appears to be incorrect path generation for DLL paths - at least using .NET 8.0.402 on OSX.

❯ dotnet msbuild ./ -getProperty:OutputPath -getProperty:TargetExt -getProperty:AssemblyName -getProperty:TargetFramework
{
  "Properties": {
    "OutputPath": "/Users/alex/GitHub/redacted/channel-notif-preferences/services/channel-notif-preferences/src/../bin/ChannelNotifPreferences/Debug/net8.0/",
    "TargetExt": ".dll",
    "AssemblyName": "ChannelNotifPreferences",
    "TargetFramework": "net8.0"
  }
}

Notice the OutputPath is already absolute.

This means in csproj-parse.lua we should not join the OutputPath with the project_file_path. Additionally in runner.lua we should not join getcwd with the dll path either.

I would put up a PR for this but I am guessing maybe pathing is different on Windows or other OS's which is why this is not broken for everyone, so it likely needs a more delicate fix.

It might simplify things though to ensure get_dll_path() is always returning the absolute path so the logic for that is isolated into a single place.

I am not super familiar with how the vim.os path methods are supposed to work here, maybe it's supposed to handle this already , but regardless it was not working and generating some funky looking paths for me.

GustavEikaas commented 3 weeks ago

Works for me on windows so I guess the behaviour is different. I can easily add a linux/windows check to ensure correct path resolution I just need to verify the behaviours on both windows and linux first.

Thanks for bringing this to my attention, should have a fix shortly

GustavEikaas commented 3 weeks ago

I tested this using both Windows and Ubuntu, results below. This indicates that OSX is the outlier, not the question is how can we detect if a machine is OSX?

Windows

❯ dotnet msbuild ./ -getProperty:OutputPath -getProperty:TargetExt -getProperty:AssemblyName -getProperty:TargetFramework
{
  "Properties": {
    "OutputPath": "bin\\Debug\\net8.0\\",
    "TargetExt": ".dll",
    "AssemblyName": "NeovimDebugProject.Api",
    "TargetFramework": "net8.0"
  }
}

Ubuntu WSL

❯ dotnet msbuild ./ -getProperty:OutputPath -getProperty:TargetExt -getProperty:AssemblyName -getProperty:TargetFramework
{
  "Properties": {
    "OutputPath": "bin/Debug/net8.0/",
    "TargetExt": ".dll",
    "AssemblyName": "dotnet-api",
    "TargetFramework": "net8.0"
  }
}
GustavEikaas commented 3 weeks ago

@alexciarlillo Can you test out the branch I made when you got time, hopefully it fixes your issue

GustavEikaas commented 3 weeks ago

I made some changes in the code and now it checks for a starting / to determine if the Output path is an absolute path or relative, this should hopefully work for all environments, I will test for both windows and linux

GustavEikaas commented 3 weeks ago

After doing some digging it might be better to use the msbuild property TargetPath

Can you run the following command and post the output?

> dotnet msbuild ./ -getProperty:OutputPath -getProperty:TargetExt -getProperty:AssemblyName -getProperty:TargetFramework -getProperty:TargetDir -getProperty:TargetPath

{
  "Properties": {
    "OutputPath": "bin\\Debug\\net8.0\\",
    "TargetExt": ".dll",
    "AssemblyName": "NeovimDebugProject.Api",
    "TargetFramework": "net8.0",
    "TargetDir": "C:\\Users\\Gustav\\repo\\NeovimDebugProject\\src\\NeovimDebugProject.Api\\bin\\Debug\\net8.0\\",
    "TargetPath": "C:\\Users\\Gustav\\repo\\NeovimDebugProject\\src\\NeovimDebugProject.Api\\bin\\Debug\\net8.0\\NeovimDebugProject.Api.dll"
  }
}
alexciarlillo commented 3 weeks ago

TargetPath looks good

❯ dotnet msbuild ./ -getProperty:OutputPath -getProperty:TargetExt -getProperty:AssemblyName -getProperty:TargetFramework -getProperty:TargetDir -getProperty:TargetPath
{
  "Properties": {
    "OutputPath": "/Users/alex/GitHub/repo/channel-notif-preferences/services/channel-notif-preferences/src/../bin/ChannelNotifPreferences/Debug/net8.0/",
    "TargetExt": ".dll",
    "AssemblyName": "ChannelNotifPreferences",
    "TargetFramework": "net8.0",
    "TargetDir": "/Users/alex/GitHub/repo/channel-notif-preferences/services/channel-notif-preferences/bin/ChannelNotifPreferences/Debug/net8.0/",
    "TargetPath": "/Users/alex/GitHub/repo/channel-notif-preferences/services/channel-notif-preferences/bin/ChannelNotifPreferences/Debug/net8.0/ChannelNotifPreferences.dll"
  }
}
GustavEikaas commented 3 weeks ago

Great!

I had a colleague test the command on OSX, and he got the same output as I do on windows and linux. Is it possible you have some custom MSBuild variables or configuration in your .csproj file resulting in this OutputPath. Can you create a new console app with dotnet new and run the msbuild command aswell as running dotnet msbuild --version and post the results?

alexciarlillo commented 3 weeks ago
❯ dotnet new console
The template "Console App" was created successfully.

Processing post-creation actions...
Restoring /Users/alex/test-proj/test-proj.csproj:
  Determining projects to restore...
  Restored /Users/alex/test-proj/test-proj.csproj (in 34 ms).
Restore succeeded.
❯ dotnet msbuild --version
MSBuild version 17.11.4+37eb419ad for .NET
17.11.4.40609
❯ dotnet msbuild ./ -getProperty:OutputPath -getProperty:TargetExt -getProperty:AssemblyName -getProperty:TargetFramework -getProperty:TargetDir -getProperty:TargetPath
{
  "Properties": {
    "OutputPath": "bin\\Debug/net8.0/",
    "TargetExt": ".dll",
    "AssemblyName": "test-proj",
    "TargetFramework": "net8.0",
    "TargetDir": "/Users/alex/test-proj/bin/Debug/net8.0/",
    "TargetPath": "/Users/alex/test-proj/bin/Debug/net8.0/test-proj.dll"
  }
}

it does indeed look like in this case OutputPath is relative instead of absolute.

I checked in my project and grep'd for OutputPath and got the following:

❯ rg OutputPath *
Directory.Build.props
19:    <BaseIntermediateOutputPath>$(MSBuildProjectDirectory)/../obj/$(MSBuildProjectName)/</BaseIntermediateOutputPath>
20:    <BaseOutputPath>$(MSBuildProjectDirectory)/../bin/$(MSBuildProjectName)/</BaseOutputPath>

services/channel-notif-preferences/Directory.Build.props
15:    <BaseIntermediateOutputPath>$(MSBuildProjectDirectory)/../obj/$(MSBuildProjectName)/</BaseIntermediateOutputPath>
16:    <BaseOutputPath>$(MSBuildProjectDirectory)/../bin/$(MSBuildProjectName)/</BaseOutputPath>

So it does appear to be something specific to this project on my end. There props files are generated by some template my employer uses so unfortunately I do not think I can change them to suit my needs for neovim without potentially causing issues in our CI pipelines (or maybe this is normal? i really do not know 🤣 ). It's also not fair to make your project cater to these types of situations. I would still love to use it though, so if you can think of any way to accommodate custom paths here I would be really appreciative.

Sorry I did not catch this earlier. This is really my first time being onboarded to a large-ish dotnet project - I am normally in the NodeJS world so a lot of this is unfamiliar to me.

GustavEikaas commented 2 weeks ago

I think TargetPath makes more sense than OutputPath so ill likely make the plugin use this in the near future.

Just for fun you could try this.

  1. Create a <yourProjectName>.csproj.user
  2. Add the following

    
    <?xml version="1.0" encoding="utf-8"?>
    <Project ToolsVersion="Current" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
    <!-- Reset to default for Debug -->
    <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' ">
    <OutputPath>bin/Debug/</OutputPath>
    </PropertyGroup>
    
    <!-- Reset to default for Release -->
    <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' ">
    <OutputPath>bin/Release/</OutputPath>
    </PropertyGroup>



.csproj.user files should be gitignored so this would work although its not recommended to override the OutputPath like this as it will likely cause confusion
GustavEikaas commented 2 weeks ago

It might take me a while to implement using TargetPath over OutputPath but in the meantime you can use this snippet in your nvim-dap config


    dap.configurations["cs"] = {
      {
        type = "coreclr",
        name = "Program",
        request = "launch",
        env = function()
          local dll = ensure_dll()
          local vars = dotnet.get_environment_variables(dll.project_name, dll.relative_project_path)
          return vars or nil
        end,
        program = function()
          local dll = ensure_dll()
          local co = coroutine.running()
          rebuild_project(co, dll.project_path)
          local dll_path = vim.fn.system(string.format("dotnet msbuild '%s' -getProperty:TargetPath", dll.project_path))
              :gsub("\n", "")
          print(vim.inspect(dll_path))
          return dll_path
        end,
        cwd = function()
          local dll = ensure_dll()
          return dll.relative_project_path
        end
      }
    }
  end
GustavEikaas commented 2 weeks ago

Im closing this issue and making a new one for the implementation of TargetPath.

Let me know if you have any more questions or issues.

Succeeded by #150

alexciarlillo commented 2 weeks ago

Thank you. Sorry I went dark there for a bit - work got insane last week. Appreciate your responsiveness here!

GustavEikaas commented 2 weeks ago

No worries! We have all been there