Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug fix for Import-PSGetRepository in Windows PS #1460

Merged
merged 3 commits into from
Oct 27, 2023

Conversation

alerickson
Copy link
Member

PR Summary

Bug fix for Import-PSGetRepository in Windows PowerShell not actually porting PowerShellGet v2 repositories to PSResourceGet. .count property does not work on the object returned in Windows PS (however it does in PS 6+). Simple change to check if the $repos returned is empty or not checks to see whether repositories need to be transferred over.

PR Context

Resolves #1442

PR Checklist

@@ -31,7 +31,7 @@ function Import-PSGetRepository {

Microsoft.PowerShell.Utility\Write-Verbose ('Selected {0} NuGet repositories.' -f $repos.Count)
Copy link
Contributor

@kborowinski kborowinski Oct 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alerickson: The issue is also here, as $repos.Count will return $null on PowerShell 5.1, when only one repository was found, and therefore the verbose message will be misleading - "VERBOSE: Selected NuGet repositories" .

I suggest changing it either to @($repos).Count, or maybe, to make the code cleaner, we should assign the repos count to a variable, like this;

...
$reposCount = @(repos).Count
Microsoft.PowerShell.Utility\Write-Verbose ('Selected {0} NuGet repositories.' -f $reposCount)

if ($reposCount) {
...

or just convert the filtering result to an array:

...
$repos = @(
    $PSGetRepositories.Values |
        Microsoft.PowerShell.Core\Where-Object {$_.PackageManagementProvider -eq 'NuGet'-and $_.Name -ne 'PSGallery'} |
        Microsoft.PowerShell.Utility\Select-Object Name, Trusted, SourceLocation
)

Microsoft.PowerShell.Utility\Write-Verbose ('Selected {0} NuGet repositories.' -f $repos.Count)

if ($repos.Count) {
...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching that, let me fix that

@alerickson alerickson requested review from adityapatwardhan, kborowinski and anamnavi and removed request for kborowinski October 25, 2023 17:26
@@ -25,13 +25,14 @@ function Import-PSGetRepository {
Microsoft.PowerShell.Utility\Write-Verbose ('Found {0} registered PowerShellGet repositories.' -f $PSGetRepositories.Count)

if ($PSGetRepositories.Count) {
$repos = $PSGetRepositories.Values |
$repos = @( $PSGetRepositories.Values |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @alerickson for nitpicking, but do we need extra space here? And wouldn't it look better if the $PSGetRepositories.Values got moved to a new line instead:

$repos = @(
    $PSGetRepositories.Values |

Copy link
Contributor

@kborowinski kborowinski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks! 🎉

@alerickson alerickson merged commit b22fe33 into PowerShell:master Oct 27, 2023
3 checks passed
@alerickson alerickson deleted the importRepoBugFix branch October 27, 2023 19:15
alerickson pushed a commit to alerickson/PSResourceGet that referenced this pull request Apr 1, 2024
…ell#1615)

Add Name and Repository pipeline by property name (PowerShell#1451)

Bugfix script parse whitespace (PowerShell#1457)

Bug fix for Import-PSGetRepository in Windows PS (PowerShell#1460)

Update README.md (PowerShell#1458)

update chANGELOG, psd1, csproj for release 1.0.1 (PowerShell#1473)

Bump System.Text.Json from 6.0.0 to 8.0.0 in /src/code (PowerShell#1475)

Verify whether SourceLocation is a UNC path and select the appropriate ApiVersion (PowerShell#1479)

TryConvertFromXml: Prevent NRE when NormalizedVersion is missing (PowerShell#1503)

Update InstallHelper.cs (PowerShell#1510)

Bump BenchmarkDotNet.Diagnostics.Windows in /test/perf/benchmarks (PowerShell#1528)

enable isJFrogRepo flag for domains containing `artifactory` (PowerShell#1532)

Fix 'name' bug with v2 JFrog Artifactory (PowerShell#1535)

Bugfix Update-ModuleManifest throws null pointer exception (PowerShell#1538)

Add tests for ADO v2 server (PowerShell#1539)

Bugfix - Test if InstalledScriptInfos folder exists and create if needed (PowerShell#1542)

* Test InstalledScriptInfos folder and create if needed

* Update src/code/InstallHelper.cs

Co-authored-by: Aditya Patwardhan <[email protected]>

* Update src/code/InstallHelper.cs

Co-authored-by: Aditya Patwardhan <[email protected]>

---------

Co-authored-by: Aditya Patwardhan <[email protected]>

Remove redeclaration of s_tempHome (PowerShell#1544)

Bug fix for Update-PSResource not updating from correct repository (PowerShell#1549)

Update changelog, version, releasenotes
alerickson added a commit to alerickson/PSResourceGet that referenced this pull request Apr 1, 2024
Add verbose and debug messages for Container Registry Server (PowerShell#1615)

Add Name and Repository pipeline by property name (PowerShell#1451)

Bugfix script parse whitespace (PowerShell#1457)

Bug fix for Import-PSGetRepository in Windows PS (PowerShell#1460)

Update README.md (PowerShell#1458)

update chANGELOG, psd1, csproj for release 1.0.1 (PowerShell#1473)

Bump System.Text.Json from 6.0.0 to 8.0.0 in /src/code (PowerShell#1475)

Verify whether SourceLocation is a UNC path and select the appropriate ApiVersion (PowerShell#1479)

TryConvertFromXml: Prevent NRE when NormalizedVersion is missing (PowerShell#1503)

Update InstallHelper.cs (PowerShell#1510)

Bump BenchmarkDotNet.Diagnostics.Windows in /test/perf/benchmarks (PowerShell#1528)

enable isJFrogRepo flag for domains containing `artifactory` (PowerShell#1532)

Fix 'name' bug with v2 JFrog Artifactory (PowerShell#1535)

Bugfix Update-ModuleManifest throws null pointer exception (PowerShell#1538)

Add tests for ADO v2 server (PowerShell#1539)

Bugfix - Test if InstalledScriptInfos folder exists and create if needed (PowerShell#1542)

* Test InstalledScriptInfos folder and create if needed

* Update src/code/InstallHelper.cs

Co-authored-by: Aditya Patwardhan <[email protected]>

* Update src/code/InstallHelper.cs

Co-authored-by: Aditya Patwardhan <[email protected]>

---------

Co-authored-by: Aditya Patwardhan <[email protected]>

Remove redeclaration of s_tempHome (PowerShell#1544)

Bug fix for Update-PSResource not updating from correct repository (PowerShell#1549)

Update changelog, version, releasenotes
alerickson added a commit to alerickson/PSResourceGet that referenced this pull request Apr 1, 2024
…ncies' in PSResourceInfo object (PowerShell#1604)

Create OneBranch build and release pipeline (PowerShell#1605)

Add verbose and debug messages for Container Registry Server (PowerShell#1615)

Add Name and Repository pipeline by property name (PowerShell#1451)

Bugfix script parse whitespace (PowerShell#1457)

Bug fix for Import-PSGetRepository in Windows PS (PowerShell#1460)

Update README.md (PowerShell#1458)

update chANGELOG, psd1, csproj for release 1.0.1 (PowerShell#1473)

Bump System.Text.Json from 6.0.0 to 8.0.0 in /src/code (PowerShell#1475)

Verify whether SourceLocation is a UNC path and select the appropriate ApiVersion (PowerShell#1479)

TryConvertFromXml: Prevent NRE when NormalizedVersion is missing (PowerShell#1503)

Update InstallHelper.cs (PowerShell#1510)

Bump BenchmarkDotNet.Diagnostics.Windows in /test/perf/benchmarks (PowerShell#1528)

enable isJFrogRepo flag for domains containing `artifactory` (PowerShell#1532)

Fix 'name' bug with v2 JFrog Artifactory (PowerShell#1535)

Bugfix Update-ModuleManifest throws null pointer exception (PowerShell#1538)

Add tests for ADO v2 server (PowerShell#1539)

Bugfix - Test if InstalledScriptInfos folder exists and create if needed (PowerShell#1542)

* Test InstalledScriptInfos folder and create if needed

* Update src/code/InstallHelper.cs

Co-authored-by: Aditya Patwardhan <[email protected]>

* Update src/code/InstallHelper.cs

Co-authored-by: Aditya Patwardhan <[email protected]>

---------

Co-authored-by: Aditya Patwardhan <[email protected]>

Remove redeclaration of s_tempHome (PowerShell#1544)

Bug fix for Update-PSResource not updating from correct repository (PowerShell#1549)

Update changelog, version, releasenotes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Import-PSGetRepository fails silently to import UNC based PowerShellGet v2 repositories on PowerShell 5.1
3 participants