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

Bugfix Update-ModuleManifest throws null pointer exception #1538

Merged
merged 26 commits into from
Feb 5, 2024

Conversation

anamnavi
Copy link
Member

@anamnavi anamnavi commented Jan 23, 2024

Enable Update-PSModuleManifest and Get-PSScriptFileInfo tests that got disabled along the way.
Fix bug where Update-PSModuleManifest throws null pointer, fixes #1487

Handle Update-PSModuleManifest calls with -Prerelease, -RequireLicenseAcceptance, and ExternalModuleDependencies parmaters on Windows PowerShell. Since Update-PSModuleManifest calls New-ModuleManifest but those parameters weren't supported for New-ModuleManifest on Windows PowerShell, we need to create the PrivateData entry with the parameter values and then replace the default PrivateData entry in the .psd1 with the correct/updated one.

Support cases where -PrivateData is a nested hashtable (this was a bug stemming from New-ModuleManifest and can't be fixed here but I've added bugfix so that Update-PSModuleManifest` can handle it on PSCore and WinPS.

PR Summary

PR Context

PR Checklist

@anamnavi anamnavi changed the title Enable Update-ModuleManifest and Get-PSScriptFileInfo tests that aren… Bugfix Update-ModuleManifest throws null pointer exception Jan 23, 2024
test/PSScriptFileInfoTests/GetPSScriptFileInfo.Tests.ps1 Outdated Show resolved Hide resolved
@@ -86,7 +88,7 @@ Describe 'Test Update-PSModuleManifest' {
}
}

It "Update module manifest given Prerelease parameter" {
It "Update module manifest given Prerelease parameter" -Skip:$IsWindowsPowerShell {
Copy link
Member

Choose a reason for hiding this comment

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

Why is this not working on Windows PS?

Copy link
Member Author

Choose a reason for hiding this comment

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

the -Prerelease parameter was not included for New-ModuleManifest cmdlet in WindowsPowerShell per the docs

ExternalModuleDependencies and RequireLicenseAcceptance are the other paramaters also not included

Copy link
Member

Choose a reason for hiding this comment

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

Okay, considering we have to be backwards compatible with Windows PS 5.1 we should add error handling if someone on Windows PowerShell tries to use those parameters and specify that it's a feature only available on later versions of PS.

src/code/UpdateModuleManifest.cs Show resolved Hide resolved
@anamnavi anamnavi requested a review from alerickson January 31, 2024 05:53
src/code/UpdateModuleManifest.cs Outdated Show resolved Hide resolved
src/code/UpdateModuleManifest.cs Show resolved Hide resolved
src/code/UpdateModuleManifest.cs Outdated Show resolved Hide resolved
src/code/UpdateModuleManifest.cs Show resolved Hide resolved
src/code/UpdateModuleManifest.cs Show resolved Hide resolved
src/code/UpdateModuleManifest.cs Show resolved Hide resolved
src/code/UpdateModuleManifest.cs Outdated Show resolved Hide resolved
src/code/UpdateModuleManifest.cs Outdated Show resolved Hide resolved
src/code/UpdateModuleManifest.cs Show resolved Hide resolved
anamnavi and others added 2 commits January 31, 2024 13:42
Copy link
Member

@alerickson alerickson left a comment

Choose a reason for hiding this comment

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

Just added one comment, but looks good otherwise

src/code/Utils.cs Outdated Show resolved Hide resolved
src/code/UpdateModuleManifest.cs Outdated Show resolved Hide resolved
src/code/UpdateModuleManifest.cs Outdated Show resolved Hide resolved
@anamnavi anamnavi merged commit 9738f15 into PowerShell:master Feb 5, 2024
10 checks passed
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
3 participants