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
Stop referencing Microsoft.PowerShell.Security when the core snapin is used #17771
Conversation
It seems there are more dependences on the module :-) Easy to remove. |
/cc @iSazonov @SteveL-MSFT @JamesWTruher It turns out it's sort-of intentional to load
This is because the processing of The use-nested-module.zip is a simple repro of this behavior. Unzip it, and run However, it will work fine when using Both PowerShell 7.0 and 7.2 has the same behavior. But on Windows PowerShell, both QUESTION: Changing the loading order of nested module would be a non-trivial work which is quite risky. Given that, shall we revert #16355? |
@daxian-dbw Your question raise more questions:
|
… to RequiredAssemblies
A workaround is to add I have pushed a commit with that change. Although it looks weirdly redundant, I think it should be acceptable.
I don't think it's a bug or oversight. My guess is:
|
NestedModules = "Microsoft.PowerShell.Security.dll" | ||
# 'Security.types.ps1xml' refers to types from 'Microsoft.PowerShell.Security.dll' and thus requiring to load the assembly before processing the type file. | ||
# We declare 'Microsoft.PowerShell.Security.dll' in 'RequiredAssemblies' so as to make sure it's loaded before the type file processing. | ||
RequiredAssemblies = "Microsoft.PowerShell.Security.dll" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The workaround with RequiredAssemblies
is low risk and I think this is a change we should have to decouple them
🎉 Handy links: |
PR Summary
This is a follow-up change of #16355.
The strong reference to
Microsoft.PowerShell.Security
is not needed anymore after merging #16355.There are whitespace only changes in the PR, so it's easier to review by ignoring the whitespaces:
https://github.com/PowerShell/PowerShell/pull/17771/files?w=1
PR Checklist
.h
,.cpp
,.cs
,.ps1
and.psm1
files have the correct copyright headerWIP:
or[ WIP ]
to the beginning of the title (theWIP
bot will keep its status check atPending
while the prefix is present) and remove the prefix when the PR is ready.