Skip to content

Remove #requires -pssnapin #19320

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
daxian-dbw merged 2 commits into from
Apr 3, 2023
Merged

Remove #requires -pssnapin #19320

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
48eb31e
Remove `#requires -pssnapin`
SteveL-MSFT Mar 13, 2023
5afeff8
remove unused code
SteveL-MSFT Mar 20, 2023
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
109 changes: 8 additions & 101 deletions src/System.Management.Automation/engine/CommandDiscovery.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,92 +316,25 @@ internal static void VerifyRequiredModules(ExternalScriptInfo scriptInfo, Execut
}
}

private static Collection<string> GetPSSnapinNames(IEnumerable<PSSnapInSpecification> PSSnapins)
{
Collection<string> result = new Collection<string>();

foreach (var PSSnapin in PSSnapins)
{
result.Add(BuildPSSnapInDisplayName(PSSnapin));
}

return result;
}

private CommandProcessorBase CreateScriptProcessorForSingleShell(ExternalScriptInfo scriptInfo, ExecutionContext context, bool useLocalScope, SessionStateInternal sessionState)
{
VerifyScriptRequirements(scriptInfo, Context);

IEnumerable<PSSnapInSpecification> requiresPSSnapIns = scriptInfo.RequiresPSSnapIns;
if (requiresPSSnapIns != null && requiresPSSnapIns.Any())
{
Collection<string> requiresMissingPSSnapIns = null;
VerifyRequiredSnapins(requiresPSSnapIns, context, out requiresMissingPSSnapIns);
if (requiresMissingPSSnapIns != null)
{
ScriptRequiresException scriptRequiresException =
new ScriptRequiresException(
scriptInfo.Name,
requiresMissingPSSnapIns,
"ScriptRequiresMissingPSSnapIns",
true);
throw scriptRequiresException;
}
}
else
if (!string.IsNullOrEmpty(scriptInfo.RequiresApplicationID))
{
// If there were no PSSnapins required but there is a shellID required, then we need
// to error
ScriptRequiresException sre =
new ScriptRequiresException(
scriptInfo.Name,
string.Empty,
string.Empty,
"RequiresShellIDInvalidForSingleShell");

if (!string.IsNullOrEmpty(scriptInfo.RequiresApplicationID))
{
ScriptRequiresException sre =
new ScriptRequiresException(
scriptInfo.Name,
string.Empty,
string.Empty,
"RequiresShellIDInvalidForSingleShell");

throw sre;
}
throw sre;
}

return CreateCommandProcessorForScript(scriptInfo, Context, useLocalScope, sessionState);
}

private static void VerifyRequiredSnapins(IEnumerable<PSSnapInSpecification> requiresPSSnapIns, ExecutionContext context, out Collection<string> requiresMissingPSSnapIns)
{
requiresMissingPSSnapIns = null;
Dbg.Assert(context.InitialSessionState != null, "PowerShell should be hosted with InitialSessionState");

foreach (var requiresPSSnapIn in requiresPSSnapIns)
{
var loadedPSSnapIn = context.InitialSessionState.GetPSSnapIn(requiresPSSnapIn.Name);
if (loadedPSSnapIn is null)
{
requiresMissingPSSnapIns ??= new Collection<string>();
requiresMissingPSSnapIns.Add(BuildPSSnapInDisplayName(requiresPSSnapIn));
}
else
{
// the requires PSSnapin is loaded. now check the PSSnapin version
Dbg.Assert(loadedPSSnapIn.Version != null,
string.Format(
CultureInfo.InvariantCulture,
"Version is null for loaded PSSnapin {0}.", loadedPSSnapIn));
if (requiresPSSnapIn.Version != null)
{
if (!AreInstalledRequiresVersionsCompatible(
requiresPSSnapIn.Version, loadedPSSnapIn.Version))
{
requiresMissingPSSnapIns ??= new Collection<string>();
requiresMissingPSSnapIns.Add(BuildPSSnapInDisplayName(requiresPSSnapIn));
}
}
}
}
}

// This method verifies the following 3 elements of #Requires statement
// #Requires -RunAsAdministrator
// #Requires -PSVersion
Expand Down Expand Up @@ -481,32 +414,6 @@ internal static void VerifyElevatedPrivileges(ExternalScriptInfo scriptInfo)
}
}

/// <summary>
/// Used to determine compatibility between the versions in the requires statement and
/// the installed version. The version can be PSSnapin or msh.
/// </summary>
/// <param name="requires">Versions in the requires statement.</param>
/// <param name="installed">Version installed.</param>
/// <returns>
/// true if requires and installed's major version match and requires' minor version
/// is smaller than or equal to installed's
/// </returns>
/// <remarks>
/// In PowerShell V2, script requiring PowerShell 1.0 will fail.
/// </remarks>
private static bool AreInstalledRequiresVersionsCompatible(Version requires, Version installed)
{
return requires.Major == installed.Major && requires.Minor <= installed.Minor;
}

private static string BuildPSSnapInDisplayName(PSSnapInSpecification PSSnapin)
{
return PSSnapin.Version == null ?
PSSnapin.Name :
StringUtil.Format(DiscoveryExceptions.PSSnapInNameVersion,
PSSnapin.Name, PSSnapin.Version);
}

/// <summary>
/// Look up a command using a CommandInfo object and return its CommandProcessorBase.
/// </summary>
Expand Down
9 changes: 0 additions & 9 deletions src/System.Management.Automation/engine/ExternalScriptInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -458,15 +458,6 @@ internal uint PSVersionLineNumber
get { return 0; }
}

internal IEnumerable<PSSnapInSpecification> RequiresPSSnapIns
{
get
{
var data = GetRequiresData();
return data?.RequiresPSSnapIns;
}
}

/// <summary>
/// Gets the original contents of the script.
/// </summary>
Expand Down
8 changes: 0 additions & 8 deletions src/System.Management.Automation/engine/parser/ast.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -753,14 +753,6 @@ public class ScriptRequirements
/// </summary>
public ReadOnlyCollection<ModuleSpecification> RequiredModules { get; internal set; }

/// <summary>
/// The snapins this script requires, specified like:
/// <code>#requires -PSSnapin Snapin</code>
/// <code>#requires -PSSnapin Snapin -Version 2</code>
/// If no snapins are required, this property is an empty collection.
/// </summary>
public ReadOnlyCollection<PSSnapInSpecification> RequiresPSSnapIns { get; internal set; }
Copy link
Member

@daxian-dbw daxian-dbw Apr 3, 2023

Choose a reason for hiding this comment

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

This is a public API, so removing it is a breaking change. Would it be acceptable?

Copy link
Member

@daxian-dbw daxian-dbw Apr 3, 2023

Choose a reason for hiding this comment

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

I don't find anything on GitHub using RequiresPSSnapIns except for PowerShell repo itself:RequiresPSSnapIns - grep.app

So I think it's OK to make this breaking change


/// <summary>
/// The assemblies this script requires, specified like:
/// <code>#requires -Assembly path\to\foo.dll</code>
Expand Down
3 changes: 0 additions & 3 deletions src/System.Management.Automation/engine/parser/tokenizer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1976,9 +1976,6 @@ internal ScriptRequirements GetScriptRequirements()
RequiredPSEditions = requiredEditions != null
? new ReadOnlyCollection<string>(requiredEditions)
: ScriptRequirements.EmptyEditionCollection,
RequiresPSSnapIns = requiredSnapins != null
? new ReadOnlyCollection<PSSnapInSpecification>(requiredSnapins)
: ScriptRequirements.EmptySnapinCollection,
RequiredAssemblies = requiredAssemblies != null
? new ReadOnlyCollection<string>(requiredAssemblies)
: ScriptRequirements.EmptyAssemblyCollection,
Expand Down