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

Using ModuleSpec syntax in RequiredModules causes incorrect "cyclic dependency" failures #2607

Closed
Jaykul opened this issue Nov 3, 2016 · 14 comments
Assignees
Labels
Issue-Bug Issue has been identified as a bug in the product Resolution-Fixed The issue is fixed. WG-Engine core PowerShell engine, interpreter, and runtime
Milestone

Comments

@Jaykul
Copy link
Contributor

Jaykul commented Nov 3, 2016

If a module (A) specifies the version of it's dependencies (the way it should), like:

RequiredModules = @(@{ModuleName='B'; ModuleVersion='1.0'; })

Another module which takes the same dependency, and also depends on A will cause incorrect cyclic dependency errors.

Background

We recently split up a module into a few smaller modules, and created a meta-module to load them, such that a module "QMHelper" became: QM.Assembly, QM.Database, QM.Configuration, QM.Security, and ... QMHelper

Each of these modules depends on a few others, in a strictly linear fashion. E.g.:

QM.Assembly has no dependencies, but everything depends on it.
QM.Database depends only on QM.Assembly
QM.Configuration depends on QM.Database (and QM.Assembly)
...
QMHelper, of course, depends on every module, since it exists purely as an easy way to install them and import everything to maintain backwards compatibility.

Each of these dependencies is documented in the module's manifest with the minimum version and GUID. E.g. @{ModuleName='QM.Database'; ModuleVersion='1.0'; GUID='0b8a2968-ea14-4e63-9961-d1dbee54faa5'}

We believed that this was the right way to design large modules so that they can be installed easily, but only the parts that you need must be imported.

Bug Report:

A user script which had a #requires -modules QMHelper in it failed to run. The error says that QMHelper was not loaded, but they can't figure out why. If they Import-Module QMHelper first, it imports fine, and the script will run with no problems.

It turns out that a #requires statement counts as a level of dependency (just like RequiresModule), and when there are module versions involved, PowerShell gives up after just a few levels. Specifically, 5?

In our case, we were just under the limit, and everything worked fine as long as you explicitly called Import-Module -- but if you used a #requires -modules statement, or added our top-level module to your RequiredModules, it would refuse to import, and give a confusing and incorrect error message.

Steps to reproduce

I wrote a script to generate samples in a lot of different ways, but the simplest repro case is this:

$Env:PSModulePath += ";$pwd"
$last = @();
foreach($module in mkdir Test1, Test2, Test3, Test4, Test5 -Force) {
   New-ModuleManifest ($module.Name + "\" + $module.Name + ".psd1") -RequiredModules $last
   $global:last += @{ ModuleName = $Module.Name; ModuleVersion = '1.0' }
}
Import-Module Test5

Expected behavior

Import-Module should work, since the dependencies are simply linear.

Actual behavior

You'll get an error like this:

The required module 'Test3' is not loaded. 
The module 'Test3' has a requiredModule 'Test2' in its module manifest 
'C:\ModuleTestFolder\Test3\Test3.psd1' that points to a cyclic dependency.

There is, of course, no cyclic dependency, and just changing the RequiresModules hashtable to a simple module name will clear up the error.

$last = @();
foreach($module in mkdir Test1, Test2, Test3, Test4, Test5 -Force) {
   New-ModuleManifest ($module.Name + "\" + $module.Name + ".psd1") -RequiredModules $last.ModuleName
   $global:last += @{ ModuleName = $Module.Name; ModuleVersion = '1.0' }
}
Import-Module Test5

Without specifying the ModuleVersion in the manifest, you can go hundreds of layers deep.

Environment data

> $PSVersionTable
Name                           Value                   
----                           -----                   
PSVersion                      5.1.14393.206           
PSEdition                      Desktop                 
BuildVersion                   10.0.14393.206          
CLRVersion                     4.0.30319.42000         
WSManStackVersion              3.0                     
PSRemotingProtocolVersion      2.3                     
SerializationVersion           1.1.0.1                 
@Jaykul
Copy link
Contributor Author

Jaykul commented Nov 3, 2016

I have to say, the error message when trying to run a script which has a dependency, like

$last = @();
foreach($module in mkdir Test1, Test2, Test3, Test4 -Force) {
   New-ModuleManifest ($module.Name + "\" + $module.Name + ".psd1") -RequiredModules $last
   $global:last += @{ ModuleName = $Module.Name; ModuleVersion = '1.0' }
}

Note that only three of those actually have dependencies, and only 2 of them actually need to have ModuleSpec to cause the problem.

Given a script test.ps1 like this:

#requires -modules Test4
Get-Module Test*

Running it produces a completely unhelpful error (and two entries in $Error) -- I'm considering changing our guidance to say that we recommend never using #requires -module to import modules. Thoughts?

.\test.ps1 : The script 'test.ps1' cannot be run because the following modules that are specified by the "#requires"
statements of the script are missing: Test4.
At line:7 char:1
+ .\test.ps1
+ ~~~~~~~~~~
    + CategoryInfo          : ResourceUnavailable: (test.ps1:String) [], ParentContainsErrorRecordException
    + FullyQualifiedErrorId : ScriptRequiresMissingModules

@HemantMahawar HemantMahawar added the WG-Engine core PowerShell engine, interpreter, and runtime label Nov 18, 2016
@f0rt
Copy link

f0rt commented Feb 14, 2017

I can confirm the issue. I have the following setup:

  • Manifests with more than 1 dependencies
  • Dependency level of 5+
  • Dependencies in the full format like @{ ModuleName = ""; ModuleVersion = "" }

Note that if I leave only 1 dependency in the manifest files it works fine.

@joeyaiello joeyaiello added the Issue-Bug Issue has been identified as a bug in the product label Feb 16, 2017
@joeyaiello joeyaiello added this to the 6.0.0-beta milestone Feb 16, 2017
@joeyaiello
Copy link
Contributor

This breaks some production PowerShellGet module dependency traversal scenarios and is definitely needed for 6.0.0-beta at the latest.

@joeyaiello joeyaiello added this to Priority-High in Developer Experience/SDK Mar 20, 2017
daxian-dbw pushed a commit that referenced this issue May 1, 2017
This fixes issue #2607.
'RequiredModules' is a field in module manifest that can reference other modules using ModuleSpecification format.
The basic version of this format (just module name) was working fine, however, there was a problem when a more detailed version of the format was used (the one that uses module versions or/and GUIDs).
During module import, there is a check for cyclic references through 'RequiredModules' field. The bug was in this check for cyclic references, related to comparison rules for ModuleSpecification objects - as a result, the code was incorrectly reporting 'cyclic reference' error in cases when there was none.
Also, added tests for different ModuleSpecification formats and a test for error when there is actually a cyclic reference.
@daxian-dbw
Copy link
Member

Resolved via #3594

@daxian-dbw daxian-dbw added the Resolution-Fixed The issue is fixed. label May 1, 2017
@Jaykul
Copy link
Contributor Author

Jaykul commented May 13, 2017

Is there any chance that this fix will be patched to Windows PowerShell (5.x)?

@anmenaga
Copy link
Contributor

@Jaykul I doubt it; but @SteveL-MSFT or @joeyaiello might have more info on this.

@anmenaga
Copy link
Contributor

By the way, big 'thank you' to @Jaykul and @f0rt for this very detailed bug report and specifically repro steps. This saved a lot of time when working on the fix.

@SteveL-MSFT
Copy link
Member

@Jaykul at this time, there is no plan to port this to Windows PowerShell v5.1

@joeyaiello joeyaiello moved this from Priority-High to Completed in Developer Experience/SDK May 18, 2017
@Jaykul
Copy link
Contributor Author

Jaykul commented Dec 6, 2017

I just want to come back here and point at AzureRM and scream.

Did you realize that AzureRM (the "meta module" we all use to install all the Azure modules) doesn't have any RequiredModules specified in it's manifest? Instead, it has to use Import-Module about 50 times to load specific versions of each of it's dependencies.

Because, you know, if they used RequiredModules they would run into this bug.

And they are NEVER going to be able to stop doing that.

Not only do I not have any metadata about the dependencies of AzureRM (which is, basically, the entire reason for the existence of this module), but the Azure team is actually having to bypass the PowerShellGet module to publish to the gallery every time: they're adding the dependency information to the NuGet package even though it's not in the module.

As a PowerShell scripter, there's no way for me to figure out that AzureRm.profile 3.4 is what shipped as part of AzureRm 4.4, and that AzureRm.profile 4.0 wasn't included until AzureRm version 5.0.0 -- except to query the gallery metadata

As a PowerShell module author, I'm going to have to duplicate their hack to build metamodule nuspec files that don't reflect my module manifest...

@sanderweltje
Copy link

Is there any workaround for PowerShell 5.1

@PrzemyslawKlys
Copy link

Can this be ported back 5.1 - we're going to be stuck with 5.1 for years and it's just poor experience.

@FriedrichWeinmann
Copy link

Stuck with the same issue in ADMF
Really would love seeing this fixed in 5.1

@sanderweltje : The only thing you can do right now is not declare the dependency and handle it internally (e.g. with warning during import or maybe even automatically installing it if missing - the latter being risky if you want to give your code to others, who may try it out in an offline scenario.

@Jaykul
Copy link
Contributor Author

Jaykul commented Sep 15, 2020

Yeah. This bug forces anyone with a slightly complex chain of modules to do what the Azure team does. Compare the Az.psd1 source to Az.psd1 as published.

  1. Publish a cleaned .psd1 with no RequiredModules or NestedModules
  2. Use Import-Module instead -- see New-ModulePsm1.
  3. Basically, you have to hack together your own publishing (or see PSPublishModule) to let you generate your nuspec by hand and add the dependencies there.

@potatoqualitee
Copy link

Oh, this is anguishing 😩

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Bug Issue has been identified as a bug in the product Resolution-Fixed The issue is fixed. WG-Engine core PowerShell engine, interpreter, and runtime
Projects
Development

No branches or pull requests