RamblingCookieMonster / PSDepend

PowerShell Dependency Handler
MIT License
285 stars 76 forks source link

New PSGalleryModule with support for semver/VersionFilter/Pre-Release #96

Closed gaelcolas closed 4 years ago

gaelcolas commented 5 years ago

Backward compatible.

This should fix #68 adding Pre-release option Fix #65 by adding a filter like: -gt 1.2 -and (-le 1.7 -or gt 2.0 ) Also supports SemVer1/2/NuGet2 versions and filter Should fix #44 by piping the module to Ipmo -force -scope global

@RamblingCookieMonster might not be 100% for merging, but asking for feedback

gaelcolas commented 5 years ago

I've changed and simplified the Get-SemVerFilter and added relevant tests. The Version Filter provided in the PSDepend PSD1s need to have quotes around their versions otherwise the PowerShell parser can't get the long versions correctly (and I don't want to write a parser). So it should be -gt "1.2" -and (-le "1.7" -or -gt "2.0.0-beta")

I think I'm now happy with this to be merged if you can confirm it's working as it should for you.

RamblingCookieMonster commented 5 years ago

Nice! I'll try to give this a quick review over the weekend, if all looks good will merge it in - thanks : D

JustinGrote commented 5 years ago

@gaelcolas, per #97, would recommend that if the version is pinned (not 'latest') and it already exists locally (checked with get-module), then skip the find-module step for a performance improvement. EDIT: I misread it, looks like it should do as expected. I'll test! 👍

JustinGrote commented 5 years ago

@gaelcolas Line 174 has Get-Module -Listavailable -All. It should just be Get-Module -Listavailable.

-All returns every single module in the system (and loads all their DLLs) and it takes forever on a system with a lot of modules with no benefit.

Here's the performance testing script I'm using, run it from the directory of your "new" test module


remove-module psdepend -erroraction SilentlyContinue
$oldmodule = (get-module psdepend -listavailable | sort version -Descending)[0]
$psdependparams = @{
    InputObject = @{
        PSDependOptions = @{
            Target = 'CurrentUser'
        }
        Pester                          = @{
            Version     = '4.4.2'
            Parameters  = @{
                SkipPublisherCheck = $true
            }
        }
        PowershellGet                   = @{
            Version     = '2.0.3'
            Parameters  = @{
                SkipPublisherCheck = $true
            }
        }
        BuildHelpers                    = '2.0.1'
        'powershell-yaml'               = '0.3.7'
        'Microsoft.Powershell.Archive'  = '1.2.2.0'
        PSScriptAnalyzer                = '1.17.1'
        Plaster                         = '1.1.3'
    }
    Install = $true
    Verbose = $true
}

$confirmpreference = 'none'

#Warmup
$m = ipmo $oldmodule -force -passthru
$cmd = get-command -module $m Invoke-psdepend
& $cmd @psdependparams
remove-module $m

#Test Old Module
$om = ipmo $oldmodule -force -PassThru -erroraction stop
$cmdold = get-command -module $om Invoke-psdepend
$oldtime = measure-command {& $cmdold @psdependparams} | % totalseconds
remove-module $om

#Test New Module (should be faster)
$nm = ipmo .\PSDepend.psd1 -force -PassThru -erroraction stop
$cmdnew = get-command -module $nm Invoke-psdepend
$newtime = measure-command {& $cmdnew @psdependparams} | % totalseconds
remove-module $nm

write-host -fore green "$($om.name) $($om.Version) (Old) Runtime - $oldtime"
write-host -fore green "$($nm.name) $($nm.Version) (New) Runtime - $newtime"
gaelcolas commented 5 years ago

Yeah, not on a PC ATM but iirc that is because only last version is returned with get-module -ListAvailable, so when you have a specific rule/filter on the version you accept it's not enough. Might need to be a bit more clever/restrictive as to when the -All is summoned!

gaelcolas commented 5 years ago

And by 'last version' I think it doesn't even comply with semver iirc... 🙄 Just doing a string compare... Like 1.2.3-rc1.12 < 1.2.3-rc1.2 (wrong 12 > 2).

JustinGrote commented 5 years ago

Hi Gael,

Latest version is only returned if you don't use -listavailable. -listavailable returns all modules with the name in $psmodulepath

-All looks for submodules, etc. which is not the point of PSDepend (for instance if a module has a submodule the same version I want, it will report as "installed" even if I can't call that directly). Thats why it has to load the module first and takes forever (try having all the Az modules on your system and running it)

But yes, get-module in its current state in both WPS5.1 and PSCore6 is not prerelease-aware, you have to get the prerelease tag with

(get-module powercd).privatedata.psdata.prerelease

I'll take a look at your source branch and PR my recommendations

ChrisLGardner commented 5 years ago

-ListAvailable is also required when you want to list modules that aren't imported into the current session.


From: Justin Grote notifications@github.com Sent: Tuesday, December 11, 2018 8:07:17 PM To: RamblingCookieMonster/PSDepend Cc: Subscribed Subject: Re: [RamblingCookieMonster/PSDepend] New PSGalleryModule with support for semver/VersionFilter/Pre-Release (#96)

Hi Gael,

Latest version is only returned if you don't use -listavailable. -listavailable returns all modules with the name in $psmodulepath

-All looks for submodules, etc. which is not the point of PSDepend (for instance if a module has a submodule the same version I want, it will report as "installed" even if I can't call that directly). Thats why it has to load the module first and takes forever (try having all the Az modules on your system and running it)

But yes, get-module in its current state in both WPS5.1 and PSCore6 is not prerelease-aware, you have to get the prerelease tag with

(get-module powercd).privatedata.psdata.prerelease

I'll take a look at your source branch and PR my recommendations

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/RamblingCookieMonster/PSDepend/pull/96#issuecomment-446343715, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AQDqFphELawZGy22t-EJ_IWpNBadBipUks5u4BB1gaJpZM4Y7hHE.

gaelcolas commented 5 years ago

Worse than just this... In 5.1 (haven't checked for 6.x yet) you can't install a pre-release side by side with a normal release. You have to -force over existing pre-release... (same for Save-Module). Never realised the limitation. Then as @JustinGrote wrote, Get-Module does not give you the pre-release in the version object, need to grab it from .PrivateData.PSData.Prerelease...

Will send an update soon-ish.

tmeckel commented 4 years ago

@RamblingCookieMonster when will PSDepnd finally support prerelease versions of Powershell modules? @gaelcolas

gaelcolas commented 4 years ago

This should be closed. Was going the wrong way. Jaykul's approach is better