PowerShell / PowerShellStandard

MIT License
158 stars 24 forks source link

Powershell Standard and "Native" Assemblies in Core #72

Open JustinGrote opened 4 years ago

JustinGrote commented 4 years ago

Just curious if this approach is correct.

PS6 includes System.Runtime.CompilerServices.Unsafe 4.0.5.0, but PSStandard 5.1 doesn't reference this.

Reproduce

  1. Build a psmodule package and take Microsoft.Extensions.Primitives as a package dependency (which in turn depends on System.Runtime.CompilerServices.Unsafe).
  2. Package will build fine, but upon attempting to load module in PS6, you get Could not load file or assembly 'System.Runtime.CompilerServices.Unsafe, Version=4.0.5.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a because this assembly already exists in the PS6 executable directory (but doesn't load by default)

However loads just fine on PS5 because it doesn't have this dependency.

Is Powershell Standard supposed to reference this dependency and its missing? Or am I just supposed to be aware of what assemblies are "native" in the new core? If so, how do I ensure my module doesn't try to load an assembly that's native in Core but not native in WinPS?

Further Example

If I specify versions as: 'System.Runtime.CompilerServices.Unsafe'='4.5.2' 'Microsoft.Extensions.Primitives'='*'

I now get the expected error NU1605: Detected package downgrade: System.Runtime.CompilerServices.Unsafe from 4.6.0 to 4.5.2. Reference the package directly from the project to select a different version.

So that I can resolve it with: 'System.Runtime.CompilerServices.Unsafe'='4.5.2' 'Microsoft.Extensions.Primitives'='2.2.0' And this configuration works on all platforms, because the CompilerServices.Unsafe assembly is the same on PS6 and PS7 (and thus gets "ignored" when loading) and loads in PS5.

However, I think PSStandard metapackage should have listed this as a non-private dependency, so I didn't have to find it out at runtime :)

jzabroski commented 4 years ago

@SteveL-MSFT @rkeithhill Does anybody monitor this repository? I tried updating a .netcoreapp2.2 project to .netcoreapp3.1 and am getting .net assembly load errors related to framework reference assemblies. This issue seems somewhat similar, but, as best as I can tell, no activity in this project for >5 months.

I am failing on Import-Module MyModule.psd1 where my psd1 has a RootModule = 'MyModule.dll' and the MyModule.dll.deps.json file references System.Management.Automation.

Import-Module : Could not load file or assembly 'System.Management.Automation, Version=6.2.4.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35'. The system cannot find the file specified.

I feel like I am hot on the trail as to what the root cause is... but I just don't know how to fix it. I suspect that as part of .NET Core 3.0 or 3.1, the mass popularization of FrameworkReference has created scenarios where the dlls are no longer deployed in the same directory as the binary cmdlet dll I'm trying to load. Unfortunately, the PowerShell Docs only explain how to "write" a binary cmdlet, not prove how to import it/use it.

SeeminglyScience commented 4 years ago

@jzabroski what's your csproj look like?

refactorsaurusrex commented 4 years ago

Does anybody monitor this repository?

I'm wondering this same question.

jzabroski commented 4 years ago

Unrelated to above reply, it turns out/seems that PowerShellStandard is meant for legacy PowerShell 5 stuff. It's in the README but the name is terribly chosen given .NETStandard implying something that should run on .NET Core and .NET Framework

SeeminglyScience commented 4 years ago

@jzabroski It does run on both, that's how all my projects are set up. Feel free to post the error you're getting and I'll see if I can help.

@refactorsaurusrex The PS team doesn't respond a whole lot here. Since the library is mostly "complete", they're probably prioritizing other projects over specifically fielding support issues. If you file an issue though, I'll probably answer.

JustinGrote commented 4 years ago

@SeeminglyScience Right but the goal of Powershell standard (as right in the README) is to enable modules to work cross platform. This issue is an example of where they don't and should be fixed, so this is an actionable item.

jzabroski commented 4 years ago

@all if helpful, see my repro here: https://github.com/jzabroski/Microsoft.PowerShell.SDK.Issue

JamesWTruher commented 4 years ago

now that 7 is released, we will be releasing a new PowerShellStandard.Library. It will be targeting support between 7 and 5 (not 6). We'll also be changing the name to reflect that. As for monitoring this repo, I and others, do, but not as often as some might like. I will produce a pre-release which I hope the participants in this thread will make sure it works with their scenarios. You can all see our tests, and I hope that if you see a gap, you open a PR :)

SeeminglyScience commented 4 years ago

@JustinGrote Sorry I was replying to @jzabroski who has an unrelated issue. That said, the issue you present is tricky. Technically speaking, the dependency for SRC.Unsafe isn't PowerShell's, but dotnet core's. I think it's worth having a bigger discussion on how this problem should be solved, but just adding a fake dependency is not the answer in my opinion.

This is a problem inherit to writing netstandard libraries for any target host. If you add a dependency that is not in netstandard, is in a specific runtime and is polyfilled with a nuget library, you'll run into this even outside of PowerShell. As a workaround, whenever I run into this I tend to try to load my lib in every runtime, and if I run into this problem I decrements package versions until it works.

I'm not suggesting the issue be closed, though. Ideally it would definitely be easier. I'm going to /cc @JamesWTruher and @TylerLeonhardt for visibility, but keep in mind that this is a sort of niche case that may not get priority.

@jzabroski Looks like that project references the SDK. The nuget for this project is PowerShellStandard.Library.

jzabroski commented 4 years ago

@SeeminglyScience I tried both - the latest cut of the repro was to reference the SDK since that seemed more logical to do. I have pushed my org off PowerShell 5 entirely. The primary reason is the remoting bridge can cause unexpected errors in production. Standardize on 7.0 if you can.

JamesWTruher commented 4 years ago

I'm not sure that the initial problem as stated can be solved by this package. It doesn't seem reasonable that this little library can know the sum totality of how it may be used and how one could construct their project and what non-portable code is introduced into an arbitrary project. As @SeeminglyScience mentioned, this is an extremely tricky problem which I'm not sure I should be focussed on.

JustinGrote commented 3 years ago

As an update I came back to this for powerconfig and now it's backwards.

I'm bringing in Microsoft.Extensions.Configuration 5.0.0 as a dependency.

My PS7 build (using net5.0 target) works fine for PS7, but my WinPS build for net461 target which is supported by MEC (and netstandard2.0) will fail upon calling the Build method of a microsoft.extension.configuration with: Could not load file or assembly 'System.Runtime.CompilerServices.Unsafe, Version=4.0.4.1, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a' or one of its dependencies. The system cannot find the file specified. as it appears the unsafe assembly got bumped to 5.0.0

@SeeminglyScience not sure if there's a good way around this or not, I'm fine with separate builds to maintain 5.1 compatibility for this module. I probably have to override the binding redirect again, I think that's how I fixed this previously.

JustinGrote commented 3 years ago

The binding redirect did fix it: image

Here is the code for that:

if ($PSEdition -eq 'Desktop') {
    $bindingRedirectHandler = [ResolveEventHandler]{
        param($sender,$assembly)
        try {
            Write-Debug "BindingRedirectHandler: Resolving $($assembly.name)"
            $assemblyShortName = $assembly.name.split(',')[0]
            $matchingAssembly = [System.AppDomain]::CurrentDomain.GetAssemblies() | Where-Object fullname -match ("^" + [Regex]::Escape($assemblyShortName))
            if ($matchingAssembly.count -eq 1) {
                Write-Host "BindingRedirectHandler: Redirecting $($assembly.name) to $($matchingAssembly.Location)"
                return $MatchingAssembly
            }
        } catch {
            #Write-Error will blackhole, which is why write-host is required. This should never occur so it should be a red flag
            write-host -fore red "BindingRedirectHandler ERROR: $PSITEM"
            return $null
        }
        return $null
    }
    [Appdomain]::CurrentDomain.Add_AssemblyResolve($bindingRedirectHandler)
}
nightroman commented 8 months ago

@JustinGrote Thank you for the suggested workaround. I engaged it in my PS binary modules and it worked for a while until I faced a problem with this approach (the assembly may not be loaded yet, so GetAssemblies() does not get it).

This slightly different way worked for me in all (my) cases:

public class ModuleAssemblyInitializer : IModuleAssemblyInitializer
{
    public void OnImport()
    {
    }

    static ModuleAssemblyInitializer()
    {
        AppDomain.CurrentDomain.AssemblyResolve += AssemblyResolve;
    }

    // Workaround for Desktop
    static Assembly AssemblyResolve(object sender, ResolveEventArgs args)
    {
        if (args.Name.StartsWith("System.Runtime.CompilerServices.Unsafe"))
        {
            var root = Path.GetDirectoryName(typeof(ModuleAssemblyInitializer).Assembly.Location);
            var path = Path.Combine(root, "System.Runtime.CompilerServices.Unsafe.dll");
            var assembly = Assembly.LoadFrom(path);
            return assembly;
        }
        return null;
    }
}