oleg-shilo / cs-script.npp

CS-Script (C# Intellisense) plugin for Notepad++ (x86/x64)
MIT License
248 stars 52 forks source link

cs-script.npp loads Microsoft.Build.Framework.dll from the wrong directory #5

Closed sbogdano-quest closed 6 years ago

sbogdano-quest commented 7 years ago

Here is the code:

//css_searchdir %ProgramFiles(x86)%\Microsoft Visual Studio\2017\Enterprise\MSBuild\15.0\Bin
using System;
using Microsoft.Build.Framework;
public class Program
{
    static public void Main(string[] args)
    {
        Console.WriteLine(typeof(IBuildEngine).Assembly.Location);
    }
}

If this code is run by Notepad++ then output is: C:\Program Files (x86)\Notepad++\plugins\CSScriptNpp\Roslyn\Microsoft.Build.Framework.dll

If this code is run by cscs.exe then output is correct: C:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise\MSBuild\15.0\Bin\Microsoft.Build.Framework.dll

So CS-Script Plugin loads Microsoft.Build.Framework.dll from the incorrect directory.

oleg-shilo commented 7 years ago

It is the same as https://github.com/oleg-shilo/cs-script/issues/86.

While it is effectively a duplicate I will keep it open until the original issue is addressed on https://github.com/oleg-shilo/cs-script .

sbogdano-quest commented 7 years ago

Seems the issue is not the same as https://github.com/oleg-shilo/cs-script/issues/86 Looks like the root cause here is that cs-script.npp implicitly adds C:\Program Files (x86)\Notepad++\plugins\CSScriptNpp\Roslyn\ to the search path list.

oleg-shilo commented 6 years ago

Sorry Sergey, somehow I missed your above response.

You are right, the issues are closely related but not the same.

Interestingly enough what you found was a double-issue. One with the script engine itself (https://github.com/oleg-shilo/cs-script/issues/86) and it is corrected now. Another one was with the plugin default search dirs.

The second one is the one that is still there. Though it's not exactly a problem but rather an inconvenience. The plugin uses its own embedded Roslyn thus it needs to include this folder in the search dirs. Though in your case you are using the assembly from another "Roslyn location". There is nothing wrong with that except that it creates a "conflict of interest" situation.

This challenge cannot be solved elegantly as CS-Script search dirs let only limited control (user can add them but not to priorities them). Prioritizing probing dirs is a direct solution but it is the one that has a very low value to effort ratio. It is the first time when the need for it has raised during ~13 years CS-Script history. :)

I think the most beneficial is a compromised solution when user would be able to exclude some already configured probing directories. I have created a feature request: https://github.com/oleg-shilo/cs-script/issues/93. When it's implemented the syntax will be as below:

//css_dir %ProgramFiles(x86)%\Microsoft Visual Studio\2017\Enterprise\MSBuild\15.0\Bin
//css_dir !%CSScriptNpp_dir%\Roslyn
using System;
using Microsoft.Build.Framework;
public class Program
{
    static public void Main(string[] args)
    {
    Console.WriteLine(typeof(IBuildEngine).Assembly.Location);
    }
}

But until it's done you can either use explicit //css_ref or disable plugin's default probing dir. This is how you can do it:

Open settings file image

Change DefaultSearchDir to any invalid path image

sbogdano-quest commented 6 years ago

Yes, the issue is definitely minor. Actually it did not affect me somehow, my code successfully works with both cs-script.npp's and VS2017's Microsoft.Build.Framework.dll.

Given the fact that there is a possibility to set DefaultSearchDirs in cs-script.npp I'm even not sure that the feature "_//cssdir should allow directory exclusions #93" makes sense.

This challenge cannot be solved elegantly as CS-Script search dirs let only limited control (user can add them but not to priorities them).

As variant CS-Script can set search paths in the order 1. "//css_searchdir directories", 2. "css.exe -dir directories", 3. DefaultSearchDirs. Such order seems to be more valid than the current (currently CS-Script places DefaultSearchDirs at first place if I understand correctly) and seems such approach solves the problem.

oleg-shilo commented 6 years ago

Given the fact that there is a possibility to set DefaultSearchDirs in cs-script.npp I'm even not sure that the feature "//css_dir should allow directory exclusions #93" makes sense.

It still does make sense but I agree it's not something that I can call essential.

...Such order seems to be more valid than the current...

Actually there is a problem. After checking I discovered that the intended probing dirs order is not always maintained. I have opened the issue (https://github.com/oleg-shilo/cs-script/issues/94) and it is already fixed. Will be delivered to N++ soon.

oleg-shilo commented 6 years ago

I am closing this one as the problem now is being addressed via CS-Script issues https://github.com/oleg-shilo/cs-script/issues/94 and https://github.com/oleg-shilo/cs-script/issues/23.