Skip to content

Bug fix for publishing modules with RequiredModules specified in .psd1 #640

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
May 18, 2022
Merged

Bug fix for publishing modules with RequiredModules specified in .psd1 #640

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a9baabe
Add bug fix for Publish-PSResource
alerickson May 4, 2022
8a0ba94
Add small bug fix and tests
alerickson May 4, 2022
f576a48
Update test
alerickson May 4, 2022
a558a23
Fix spelling error
alerickson May 4, 2022
4dd2e53
Incorporate code review suggestions
alerickson May 9, 2022
a8600af
Update src/code/PublishPSResource.cs
alerickson May 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 71 additions & 34 deletions src/code/PublishPSResource.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,13 @@
using System;
using System.Collections;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.IO;
using System.Linq;
using System.Management.Automation;
using System.Management.Automation.Language;
using System.Net.Http;
using System.Threading;
using System.Xml;

namespace Microsoft.PowerShell.PowerShellGet.Cmdlets
Expand Down Expand Up @@ -160,9 +162,10 @@ public PSCredential ProxyCredential {

#region Members

private CancellationToken _cancellationToken;
private NuGetVersion _pkgVersion;
private string _pkgName;
private static char[] _PathSeparators = new [] { System.IO.Path.DirectorySeparatorChar, System.IO.Path.AltDirectorySeparatorChar };
private static char[] _PathSeparators = new[] { System.IO.Path.DirectorySeparatorChar, System.IO.Path.AltDirectorySeparatorChar };
public const string PSDataFileExt = ".psd1";
public const string PSScriptFileExt = ".ps1";

Expand All @@ -172,6 +175,8 @@ public PSCredential ProxyCredential {

protected override void BeginProcessing()
{
_cancellationToken = new CancellationToken();

// Create a respository story (the PSResourceRepository.xml file) if it does not already exist
// This is to create a better experience for those who have just installed v3 and want to get up and running quickly
RepositorySettings.CheckRepositoryStore();
Expand Down Expand Up @@ -317,15 +322,13 @@ protected override void ProcessRecord()
return;
}

string repositoryUri = repository.Uri.AbsoluteUri;

// Check if dependencies already exist within the repo if:
// 1) the resource to publish has dependencies and
// 2) the -SkipDependenciesCheck flag is not passed in
if (dependencies != null && !SkipDependenciesCheck)
{
// If error gets thrown, exit process record
if (!CheckDependenciesExist(dependencies, repositoryUri))
if (!CheckDependenciesExist(dependencies, repository.Name))
{
return;
}
Expand Down Expand Up @@ -366,14 +369,14 @@ protected override void ProcessRecord()
// pack into a nupkg
try
{
if (!PackNupkg(outputDir, outputNupkgDir, nuspec))
{
if (!PackNupkg(outputDir, outputNupkgDir, nuspec))
{
return;
}
}
}
catch (Exception e)
{
var message = string.Format("Error packing into .nupkg: '{0}'.", e.Message);
var message = string.Format("Error packing into .nupkg: '{0}'.", e.Message);
var ex = new ArgumentException(message);
var ErrorPackingIntoNupkg = new ErrorRecord(ex, "ErrorPackingIntoNupkg", ErrorCategory.NotSpecified, null);
WriteError(ErrorPackingIntoNupkg);
Expand Down Expand Up @@ -403,6 +406,7 @@ protected override void ProcessRecord()
}

// This call does not throw any exceptions, but it will write unsuccessful responses to the console
string repositoryUri = repository.Uri.AbsoluteUri;
PushNupkg(outputNupkgDir, repository.Name, repositoryUri);

}
Expand All @@ -428,28 +432,48 @@ private bool IsValidModuleManifest(string moduleManifestPath)
// locally on the machine. Consider adding a -Syntax param to Test-ModuleManifest so that it only checks that
// the syntax is correct. In build/release pipelines for example, the modules listed under RequiredModules may
// not be locally available, but we still want to allow the user to publish.
var results = pwsh.AddCommand("Test-ModuleManifest").AddParameter("Path", moduleManifestPath).Invoke();
Collection<PSObject> results = null;
try
{
results = pwsh.AddCommand("Test-ModuleManifest").AddParameter("Path", moduleManifestPath).Invoke();
}
catch (Exception e)
{
ThrowTerminatingError(new ErrorRecord(
new ArgumentException("Error occured while running 'Test-ModuleManifest': " + e.Message),
"ErrorExecutingTestModuleManifest",
ErrorCategory.InvalidArgument,
this));
}

if (pwsh.HadErrors)
{
var message = string.Empty;
if (string.IsNullOrWhiteSpace((results[0].BaseObject as PSModuleInfo).Author))
{
message = "No author was provided in the module manifest. The module manifest must specify a version, author and description.";
}
else if (string.IsNullOrWhiteSpace((results[0].BaseObject as PSModuleInfo).Description))

if (results.Any())
{
message = "No description was provided in the module manifest. The module manifest must specify a version, author and description.";
if (string.IsNullOrWhiteSpace((results[0].BaseObject as PSModuleInfo).Author))
{
message = "No author was provided in the module manifest. The module manifest must specify a version, author and description. Run 'Test-ModuleManifest' to validate the file.";
}
else if (string.IsNullOrWhiteSpace((results[0].BaseObject as PSModuleInfo).Description))
{
message = "No description was provided in the module manifest. The module manifest must specify a version, author and description. Run 'Test-ModuleManifest' to validate the file.";
}
else if ((results[0].BaseObject as PSModuleInfo).Version == null)
{
message = "No version or an incorrectly formatted version was provided in the module manifest. The module manifest must specify a version, author and description. Run 'Test-ModuleManifest' to validate the file.";
}
}
else

if (string.IsNullOrEmpty(message) && pwsh.Streams.Error.Count > 0)
{
// This will handle version errors
var error = pwsh.Streams.Error;
message = error[0].ToString();
message = pwsh.Streams.Error[0].ToString() + "Run 'Test-ModuleManifest' to validate the module manifest.";
}
var ex = new ArgumentException(message);
var InvalidModuleManifest = new ErrorRecord(ex, "InvalidModuleManifest", ErrorCategory.InvalidData, null);
WriteError(InvalidModuleManifest);
ThrowTerminatingError(InvalidModuleManifest);
isValid = false;
}
}
Expand Down Expand Up @@ -569,7 +593,7 @@ private string CreateNuspec(
metadataElementsDictionary.Add("description", parsedMetadataHash["description"].ToString().Trim());
}

if (parsedMetadataHash.ContainsKey("releasenotes"))
if (parsedMetadataHash.ContainsKey("releasenotes"))
{
metadataElementsDictionary.Add("releaseNotes", parsedMetadataHash["releasenotes"].ToString().Trim());
}
Expand All @@ -579,7 +603,7 @@ private string CreateNuspec(
metadataElementsDictionary.Add("copyright", parsedMetadataHash["copyright"].ToString().Trim());
}

string tags = isScript ? "PSScript" : "PSModule";
string tags = isScript ? "PSScript" : "PSModule";
if (parsedMetadataHash.ContainsKey("tags"))
{
if (parsedMetadataHash["tags"] != null)
Expand Down Expand Up @@ -762,7 +786,7 @@ Example cmdlet here
{
// expecting only one or two comments
var commentText = token.Text;
parsedComments.AddRange(commentText.Split(new string[] { "\n\n" }, StringSplitOptions.RemoveEmptyEntries) );
parsedComments.AddRange(commentText.Split(new string[] { "\n\n" }, StringSplitOptions.RemoveEmptyEntries));
}
}

Expand All @@ -775,15 +799,15 @@ Example cmdlet here

var key = newlist[0].TrimStart(TrimBeginning);
var value = newlist.Length > 1 ? newlist[1].Trim() : string.Empty;
parsedMetadataHash.Add(key,value);
parsedMetadataHash.Add(key, value);
}
}
}

return parsedMetadataHash;
}

private bool CheckDependenciesExist(Hashtable dependencies, string repositoryUri)
private bool CheckDependenciesExist(Hashtable dependencies, string repositoryName)
{
// Check to see that all dependencies are in the repository
// Searches for each dependency in the repository the pkg is being pushed to,
Expand All @@ -792,19 +816,32 @@ private bool CheckDependenciesExist(Hashtable dependencies, string repositoryUri
{
// Need to make individual calls since we're look for exact version numbers or ranges.
var depName = new[] { (string)dependency };
var depVersion = (string)dependencies[dependency];
var type = new[] { "module", "script" };
var repository = new[] { repositoryUri };

// test version
string depVersion = dependencies[dependency] as string;
depVersion = string.IsNullOrWhiteSpace(depVersion) ? "*" : depVersion;

VersionRange versionRange = null;
if (!Utils.TryParseVersionOrVersionRange(depVersion, out versionRange))
{
// This should never be true because Test-ModuleManifest will throw an error if dependency versions are incorrectly formatted
// This is being left as a safeguard for parsing a version from a string to a version range.
ThrowTerminatingError(new ErrorRecord(
new ArgumentException(string.Format("Error parsing dependency version {0}, from the module {1}", depVersion, depName)),
"IncorrectVersionFormat",
ErrorCategory.InvalidArgument,
this));
}

// Search for and return the dependency if it's in the repository.
// TODO: When find is complete, uncomment beginFindHelper method below (resourceNameParameterHelper)
//var dependencyFound = findHelper.beginFindHelper(depName, type, depVersion, true, null, null, repository, Credential, false, false);
// TODO: update the type from PSObject to PSResourceInfo
List<PSObject> dependencyFound = null;
FindHelper findHelper = new FindHelper(_cancellationToken, this);
bool depPrerelease = depVersion.Contains("-");

var repository = new[] { repositoryName };
var dependencyFound = findHelper.FindByResourceName(depName, ResourceType.Module, depVersion, depPrerelease, null, repository, Credential, false);
if (dependencyFound == null || !dependencyFound.Any())
{
var message = String.Format("Dependency '{0}' was not found in repository '{1}'. Make sure the dependency is published to the repository before publishing this module.", dependency, repositoryUri);
var ex = new ArgumentException(message); // System.ArgumentException vs PSArgumentException
var message = String.Format("Dependency '{0}' was not found in repository '{1}'. Make sure the dependency is published to the repository before publishing this module.", dependency, repositoryName);
var ex = new ArgumentException(message);
var dependencyNotFound = new ErrorRecord(ex, "DependencyNotFound", ErrorCategory.ObjectNotFound, null);

WriteError(dependencyNotFound);
Expand Down
44 changes: 31 additions & 13 deletions test/PublishPSResource.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ Describe "Test Publish-PSResource" {
# Remove-Item $pkgsToDelete -Recurse
}


It "Publish a module with -Path to the highest priority repo" {
$version = "1.0.0"
New-ModuleManifest -Path (Join-Path -Path $script:PublishModuleBase -ChildPath "$script:PublishModuleName.psd1") -ModuleVersion $version -Description "$script:PublishModuleName module"
Expand All @@ -74,33 +73,29 @@ Describe "Test Publish-PSResource" {
(Get-ChildItem $script:repositoryPath2).FullName | Should -Be $expectedPath
}

<# Temporarily comment this test out until Find Helper is complete and code within PublishPSResource is uncommented
It "Publish a module with dependencies" {
# Create dependency module
$dependencyVersion = "2.0.0"
New-ModuleManifest -Path (Join-Path -Path $script:DependencyModuleBase -ChildPath "$script:DependencyModuleName.psd1") -ModuleVersion $dependencyVersion -Description "$script:DependencyModuleName module"

Publish-PSResource -LiteralPath $script:DependencyModuleBase
Publish-PSResource -Path $script:DependencyModuleBase

# Create module to test
$version = "1.0.0"
New-ModuleManifest -Path (Join-Path -Path $script:PublishModuleBase -ChildPath "$script:PublishModuleName.psd1") -ModuleVersion $version -Description "$script:PublishModuleName module" -NestedModules "$script:PublishModuleName.psm1" -RequiredModules @{ModuleName = "$script:DependencyModuleName"; ModuleVersion = "$dependencyVersion" }
New-ModuleManifest -Path (Join-Path -Path $script:PublishModuleBase -ChildPath "$script:PublishModuleName.psd1") -ModuleVersion $version -Description "$script:PublishModuleName module" -RequiredModules @(@{ModuleName = 'PackageManagement'; ModuleVersion = '2.0.0' })

Publish-PSResource -LiteralPath $script:PublishModuleBase
Publish-PSResource -Path $script:PublishModuleBase

$expectedPath = Join-Path -Path $script:repositoryPath -ChildPath "$script:PublishModuleName.$version.nupkg"
Get-ChildItem $script:repositoryPath | select-object -Last 1 | Should -Be $expectedPath
$nupkg = Get-ChildItem $script:repositoryPath | select-object -Last 1
$nupkg.Name | Should Be "$script:PublishModuleName.$version.nupkg"
}
#>

It "Publish a module with a dependency that is not published, should throw" {
$version = "1.0.0"
$dependencyVersion = "2.0.0"
New-ModuleManifest -Path (Join-Path -Path $script:PublishModuleBase -ChildPath "$script:PublishModuleName.psd1") -ModuleVersion $version -Description "$script:PublishModuleName module" -RequiredModules @(@{ModuleName="PackageManagement"; ModuleVersion="$dependencyVersion"})

Publish-PSResource -Path $script:PublishModuleBase -ErrorAction SilentlyContinue
New-ModuleManifest -Path (Join-Path -Path $script:PublishModuleBase -ChildPath "$script:PublishModuleName.psd1") -ModuleVersion $version -Description "$script:PublishModuleName module" -RequiredModules @(@{ModuleName = 'PackageManagement'; ModuleVersion = '1.4.4' })

$Error[0].FullyQualifiedErrorId | Should -be "DependencyNotFound,Microsoft.PowerShell.PowerShellGet.Cmdlets.PublishPSResource"
{Publish-PSResource -Path $script:PublishModuleBase -ErrorAction Stop} | Should -Throw -ErrorId "DependencyNotFound,Microsoft.PowerShell.PowerShellGet.Cmdlets.PublishPSResource"
}


Expand All @@ -114,7 +109,7 @@ Describe "Test Publish-PSResource" {
$expectedPath = Join-Path -Path $script:repositoryPath -ChildPath "$script:PublishModuleName.$version.nupkg"
(Get-ChildItem $script:repositoryPath).FullName | select-object -Last 1 | Should -Be $expectedPath
}

<# The following tests are related to passing in parameters to customize a nuspec.
# These parameters are not going in the current release, but is open for discussion to include in the future.
It "Publish a module with -Nuspec" {
Expand Down Expand Up @@ -290,4 +285,27 @@ Describe "Test Publish-PSResource" {
$expectedPath = Join-Path -Path $script:repositoryPath2 -ChildPath "$script:PublishModuleName.$version.nupkg"
(Get-ChildItem $script:repositoryPath2).FullName | Should -Be $expectedPath
}

<# The following tests required manual testing because New-ModuleManifest will not allow for incorrect syntax/formatting,
At the moment, 'Update-ModuleManifest' is not yet complete, but that too should call 'Test-ModuleManifest', which also
does not allow for incorrect syntax.
It's still important to validate these scenarios because users may alter the module manifest manually, or the file may
get corrupted. #>
<#
It "Publish a module with that has an invalid version format, should throw -- Requires Manual testing" {
$incorrectVersion = '1..0.0'
$correctVersion = '1.0.0.0'
New-ModuleManifest -Path (Join-Path -Path $script:PublishModuleBase -ChildPath "$script:PublishModuleName.psd1") -ModuleVersion $incorrectVersion -Description "$script:PublishModuleName module" -RequiredModules @(@{ModuleName = 'PackageManagement'; ModuleVersion = $correctVersion })

{Publish-PSResource -Path $script:PublishModuleBase -ErrorAction Stop} | Should -Throw -ErrorId "InvalidModuleManifest,Microsoft.PowerShell.PowerShellGet.Cmdlets.PublishPSResource"
}

It "Publish a module with a dependency that has an invalid version format, should throw -- Requires Manual testing" {
$correctVersion = '1.0.0'
$incorrectVersion = '1..0.0'
New-ModuleManifest -Path (Join-Path -Path $script:PublishModuleBase -ChildPath "$script:PublishModuleName.psd1") -ModuleVersion $correctVersion -Description "$script:PublishModuleName module" -RequiredModules @(@{ModuleName = 'PackageManagement'; ModuleVersion = $incorrectVersion })

{Publish-PSResource -Path $script:PublishModuleBase -ErrorAction Stop} | Should -Throw -ErrorId "InvalidModuleManifest,Microsoft.PowerShell.PowerShellGet.Cmdlets.PublishPSResource"
}
#>
}