search

PowerShell / SHiPS

Simple Hierarchy in PowerShell - developing PowerShell provider got so much easier
MIT License
185 stars 31 forks source link

Switch to using PowerShellStandard.Library to allow a single cross-platform binary #109

Closed Jaykul closed 5 years ago

Jaykul commented 6 years ago

Addresses #104

I haven't looked at how you put the actual module together -- this only addresses compilation...

Jaykul commented 6 years ago

Also, the build will "fail" because there's only a "Core" build now, no separate "Desktop" build is required or desired.

jianyunt commented 5 years ago

@Jaykul I updated build, setup, and test scripts. Now we passed all checks. Please review the changes. BTW, thanks for your contriubtions. really appreciated.

jianyunt commented 5 years ago

@stefanstranger, @jzabroski @Chaganti @dfinke @johnzabroski @DexterPOSH, @ravikanth, once we checkin this PR, SHiPS will depends on .net 4.7.1 on Windows desktop. Any concerns if we go for it?

cc @edyoung @HemantMahawar

jzabroski commented 5 years ago

Hmm... does PowerShell allow assembly redirects?

I just woke up and am not sure why it can't target 4.6.x ...

That said, I think it is a great idea to ask lots of downstream consumers if they're impacted. Simply put, I am not.

jzabroski commented 5 years ago

One other comment. PSModuleInfo has a DotNetFraneworkVersion, that was [introduced in PowerShell 3.0](https://msdn.microsoft.com/en-us/library/system.management.automation.psmoduleinfo_members(v=vs.85).aspx#Public Properties). I'm on my mobile phone so reviewing code is hard but the psm1 should be updated with new version number to indicate this.

jianyunt commented 5 years ago

@jzabroski about a year, .Net team announced .net framework 4.7.1 release. It supports downlevel OSs to Win7. Also the .Net 4.7.1 has builtin support for .NET Standard 2.0.

We can think of the .NET standard as "abstract layer" of .net 4.7.1 and .net core. For project like SHiPS if we compile against the .Net standard, its library will run on both .net core and .net full (here .Net 4.7.1) - cross platform support. You can also see more info in the issue https://github.com/PowerShell/SHiPS/issues/104.

jzabroski commented 5 years ago

@jianyunt I'm aware. I'm a 10 years experience .NET engineer. I'm merely explicitly calling out the need to reference the dependencies, otherwise the package manager can't do its' job of protecting developers from pulling in bad dependencies.

The PowerShell Module Manifest is documented in many places. A quick google search found this tutorial: How to Write a Module Manifest. In the tutorial, you can clearly see a manifest should take a DotNetFrameworkVersion key.

jzabroski commented 5 years ago

@Jaykul can you please update https://github.com/PowerShell/SHiPS/blob/development/src/Modules/SHiPS.psd1 to take the DotNetFrameworkVersion key 4.7.1

Jaykul commented 5 years ago

Note: I haven't truly tested this with .NET 4.6.1 but it should work, based on version notes and the table here

jianyunt commented 5 years ago

@jzabroski, I added DotNetFrameworkVersion in SHiPS.psd1. Thanks.

jianyunt commented 5 years ago

@Jaykul we need 4.7.1 because 4.6.1 doesn't come with the .Net standard shims, only 4.7.1 includes those. This means that if we target .Net Std2.0 and 4.6.1 we also need to ship netstandard.dll. Or users need to install the .net combability pack. To avoid this hassle, I updated it to 4.7.1.

jzabroski commented 5 years ago

I saw. :) thanks!

On Thu, Aug 16, 2018, 4:41 PM Jianyun notifications@github.com wrote:

Merged #109 https://github.com/PowerShell/SHiPS/pull/109 into master.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/PowerShell/SHiPS/pull/109#event-1793331447, or mute the thread https://github.com/notifications/unsubscribe-auth/AAbT_Vk0nMz6TgEzH9aZP07sCFpghBVMks5uRdkLgaJpZM4U1VGV .