Skip to content

Cleanups in command discovery #10815

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
iSazonov merged 6 commits into from
Dec 3, 2019
Merged

Cleanups in command discovery #10815

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
79d153f
Enable nullable, make minor cleanups, add a test
iSazonov Oct 17, 2019
5a021c3
Reduce allocations in GetAliasTable()
iSazonov Oct 17, 2019
bf6580f
Remove searching by extensions on Unix
iSazonov Oct 17, 2019
313a2f4
Fix typo.
iSazonov Nov 19, 2019
31ef215
Update src/System.Management.Automation/engine/CommandSearcher.cs
iSazonov Dec 3, 2019
0e95719
Fix HelpUri for Stop-Computer on Unix
iSazonov Dec 3, 2019
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
2 changes: 1 addition & 1 deletion src/Microsoft.PowerShell.Commands.Management/commands/management/ComputerUnix.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ namespace Microsoft.PowerShell.Commands
/// Cmdlet to stop computer.
/// </summary>
[Cmdlet(VerbsLifecycle.Stop, "Computer", SupportsShouldProcess = true,
HelpUri = "https://go.microsoft.com/fwlink/?LinkID=135263", RemotingCapability = RemotingCapability.SupportedByCommand)]
HelpUri = "https://go.microsoft.com/fwlink/?LinkID=2097151", RemotingCapability = RemotingCapability.SupportedByCommand)]
public sealed class StopComputerCommand : PSCmdlet, IDisposable
{
#region Private Members
Expand Down
23 changes: 1 addition & 22 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 @@ -1286,27 +1286,6 @@ internal void UnregisterLookupCommandInfoAction(string currentAction, string com
private HashSet<string> _activeCommandNotFound = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
private HashSet<string> _activePostCommand = new HashSet<string>(StringComparer.OrdinalIgnoreCase);

/// <summary>
/// Gets a CommandPathSearch constructed with the specified patterns and
/// using the PATH as the lookup directories.
/// </summary>
/// <param name="patterns">
/// The patterns to search for. These patterns must be in the form taken
/// by DirectoryInfo.GetFiles().
/// </param>
/// <returns>
/// An instance of CommandPathSearch that is initialized with the specified
/// patterns and using the PATH as the lookup directories.
/// </returns>
internal IEnumerable<string> GetCommandPathSearcher(IEnumerable<string> patterns)
{
// Get the PATH environment variable
IEnumerable<string> lookupPathArray = GetLookupDirectoryPaths();

// Construct the CommandPathSearch object and return it.
return new CommandPathSearch(patterns, lookupPathArray, Context);
}

/// <summary>
/// Gets the resolved paths contained in the PATH environment
/// variable.
Expand All @@ -1317,7 +1296,7 @@ internal IEnumerable<string> GetCommandPathSearcher(IEnumerable<string> patterns
/// <remarks>
/// The result is an ordered list of paths with paths starting with "." unresolved until lookup time.
/// </remarks>
internal IEnumerable<string> GetLookupDirectoryPaths()
internal LookupPathCollection GetLookupDirectoryPaths()
{
LookupPathCollection result = new LookupPathCollection();

Expand Down
64 changes: 31 additions & 33 deletions src/System.Management.Automation/engine/CommandPathSearch.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
using System.IO;
using System.Linq;

using Dbg = System.Management.Automation.Diagnostics;
#nullable enable

namespace System.Management.Automation
{
Expand All @@ -24,29 +24,28 @@ internal class CommandPathSearch : IEnumerable<string>, IEnumerator<string>
/// Constructs a command searching enumerator that resolves the location
/// of a command using the PATH environment variable.
/// </summary>
/// <param name="patterns">
/// The patterns to search for in the path.
/// <param name="commandName">
/// The command name to search for in the path.
/// </param>
/// <param name="lookupPaths">
/// The paths to directories in which to lookup the command.
/// Ex.null: paths from PATH environment variable.
/// </param>
/// <param name="context">
/// The execution context for the current engine instance.
/// </param>
internal CommandPathSearch(
IEnumerable<string> patterns,
IEnumerable<string> lookupPaths,
ExecutionContext context)
{
Init(patterns, lookupPaths, context);
}

/// <param name="acceptableCommandNames">
/// The patterns to search for in the paths.
/// </param>
/// <param name="useFuzzyMatch">
/// Use likely relevant search.
/// </param>
internal CommandPathSearch(
string commandName,
IEnumerable<string> lookupPaths,
LookupPathCollection lookupPaths,
ExecutionContext context,
Collection<string> acceptableCommandNames,
bool useFuzzyMatch = false)
bool useFuzzyMatch)
{
_useFuzzyMatch = useFuzzyMatch;
string[] commandPatterns;
Expand Down Expand Up @@ -76,21 +75,20 @@ internal CommandPathSearch(
_postProcessEnumeratedFiles = JustCheckExtensions;
}

Init(commandPatterns, lookupPaths, context);
_orderedPathExt = CommandDiscovery.PathExtensionsWithPs1Prepended;
}

private void Init(IEnumerable<string> commandPatterns, IEnumerable<string> searchPath, ExecutionContext context)
{
// Note, discovery must be set before resolving the current directory

_context = context;
_patterns = commandPatterns;

_lookupPaths = new LookupPathCollection(searchPath);
_lookupPaths = lookupPaths;
ResolveCurrentDirectoryInLookupPaths();

this.Reset();
_orderedPathExt = CommandDiscovery.PathExtensionsWithPs1Prepended;

// The same as in this.Reset()
_lookupPathsEnumerator = _lookupPaths.GetEnumerator();
_patternEnumerator = _patterns.GetEnumerator();
_currentDirectoryResults = Array.Empty<string>();
_currentDirectoryResultsEnumerator = _currentDirectoryResults.GetEnumerator();
_justReset = true;
}

/// <summary>
Expand Down Expand Up @@ -120,8 +118,8 @@ private void ResolveCurrentDirectoryInLookupPaths()

foreach (int index in _lookupPaths.IndexOfRelativePath())
{
string resolvedDirectory = null;
string resolvedPath = null;
string? resolvedDirectory = null;
string? resolvedPath = null;

CommandDiscovery.discoveryTracer.WriteLine(
"Lookup directory \"{0}\" appears to be a relative path. Attempting resolution...",
Expand Down Expand Up @@ -407,7 +405,7 @@ public void Dispose()
/// </param>
private void GetNewDirectoryResults(string pattern, string directory)
{
IEnumerable<string> result = null;
IEnumerable<string>? result = null;
try
{
CommandDiscovery.discoveryTracer.WriteLine("Looking for {0} in {1}", pattern, directory);
Expand Down Expand Up @@ -473,7 +471,7 @@ private void GetNewDirectoryResults(string pattern, string directory)
_currentDirectoryResultsEnumerator = _currentDirectoryResults.GetEnumerator();
}

private IEnumerable<string> CheckAgainstAcceptableCommandNames(string[] fileNames)
private IEnumerable<string>? CheckAgainstAcceptableCommandNames(string[] fileNames)
{
var baseNames = fileNames.Select(Path.GetFileName).ToArray();

Expand All @@ -482,8 +480,8 @@ private IEnumerable<string> CheckAgainstAcceptableCommandNames(string[] fileName

// Porting note: allow files with executable bit on non-Windows platforms

Collection<string> result = null;
if (baseNames.Length > 0)
Collection<string>? result = null;
if (baseNames.Length > 0 && _acceptableCommandNames != null)
{
foreach (var name in _acceptableCommandNames)
{
Expand All @@ -504,14 +502,14 @@ private IEnumerable<string> CheckAgainstAcceptableCommandNames(string[] fileName
return result;
}

private IEnumerable<string> JustCheckExtensions(string[] fileNames)
private IEnumerable<string>? JustCheckExtensions(string[] fileNames)
{
// Warning: pretty duplicated code
// Result must be ordered by PATHEXT order of precedence.

// Porting note: allow files with executable bit on non-Windows platforms

Collection<string> result = null;
Collection<string>? result = null;
foreach (var allowedExt in _orderedPathExt)
{
foreach (var fileName in fileNames)
Expand Down Expand Up @@ -575,10 +573,10 @@ private IEnumerable<string> JustCheckExtensions(string[] fileNames)
/// <summary>
/// If not null, called with the enumerated files for further processing.
/// </summary>
private Func<string[], IEnumerable<string>> _postProcessEnumeratedFiles;
private Func<string[], IEnumerable<string>?> _postProcessEnumeratedFiles;

private string[] _orderedPathExt;
private Collection<string> _acceptableCommandNames;
private Collection<string>? _acceptableCommandNames;

private bool _useFuzzyMatch = false;

Expand Down
36 changes: 17 additions & 19 deletions src/System.Management.Automation/engine/CommandSearcher.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1332,27 +1332,25 @@ private string GetNextLiteralPathThatExists(string command, out ProviderInfo pro
/// If <paramref name="name"/> contains one or more of the
/// invalid characters defined in InvalidPathChars.
/// </exception>
internal Collection<string> ConstructSearchPatternsFromName(string name, bool commandDiscovery = false)
internal LookupPathCollection ConstructSearchPatternsFromName(string name, bool commandDiscovery = false)
{
Dbg.Assert(
!string.IsNullOrEmpty(name),
"Caller should verify name");

Collection<string> result = new Collection<string>();
var result = new LookupPathCollection();

// First check to see if the commandName has an extension, if so
// look for that first
bool commandNameAddedFirst = Path.HasExtension(name);

bool commandNameAddedFirst = false;

if (!string.IsNullOrEmpty(Path.GetExtension(name)))
if (commandNameAddedFirst)
{
result.Add(name);
commandNameAddedFirst = true;
}

// Add the extensions for script, module and data files in that order...
if ((_commandTypes & CommandTypes.ExternalScript) != 0)
if (_commandTypes.HasFlag(CommandTypes.ExternalScript))
{
result.Add(name + StringLiterals.PowerShellScriptFileExtension);
if (!commandDiscovery)
Expand All @@ -1362,20 +1360,17 @@ internal Collection<string> ConstructSearchPatternsFromName(string name, bool co
result.Add(name + StringLiterals.PowerShellDataFileExtension);
}
}

if ((_commandTypes & CommandTypes.Application) != 0)
#if !UNIX
if (_commandTypes.HasFlag(CommandTypes.Application))
{
// Now add each extension from the PATHEXT environment variable

foreach (string extension in CommandDiscovery.PathExtensions)
{
result.Add(name + extension);
}
}

// Now add the commandName by itself if it wasn't added as the first
// pattern

#endif
// Now add the commandName by itself if it wasn't added as the first pattern
if (!commandNameAddedFirst)
{
result.Add(name);
Expand Down Expand Up @@ -1553,14 +1548,15 @@ private void setupPathSearcher()
_commandName,
_context.CommandDiscovery.GetLookupDirectoryPaths(),
_context,
ConstructSearchPatternsFromName(_commandName, commandDiscovery: true));
ConstructSearchPatternsFromName(_commandName, commandDiscovery: true),
useFuzzyMatch : false);
}
else if (_canDoPathLookupResult == CanDoPathLookupResult.PathIsRooted)
{
_canDoPathLookup = true;

string directory = Path.GetDirectoryName(_commandName);
var directoryCollection = new[] { directory };
var directoryCollection = new LookupPathCollection { directory };

CommandDiscovery.discoveryTracer.WriteLine(
"The path is rooted, so only doing the lookup in the specified directory: {0}",
Expand All @@ -1576,7 +1572,8 @@ private void setupPathSearcher()
fileName,
directoryCollection,
_context,
ConstructSearchPatternsFromName(fileName, commandDiscovery: true));
ConstructSearchPatternsFromName(fileName, commandDiscovery: true),
useFuzzyMatch : false);
}
else
{
Expand All @@ -1603,7 +1600,7 @@ private void setupPathSearcher()
}
else
{
var directoryCollection = new[] { directory };
var directoryCollection = new LookupPathCollection { directory };

string fileName = Path.GetFileName(_commandName);

Expand All @@ -1615,7 +1612,8 @@ private void setupPathSearcher()
fileName,
directoryCollection,
_context,
ConstructSearchPatternsFromName(fileName, commandDiscovery: true));
ConstructSearchPatternsFromName(fileName, commandDiscovery: true),
useFuzzyMatch : false);
}
else
{
Expand Down
4 changes: 3 additions & 1 deletion src/System.Management.Automation/engine/SessionStateAliasAPIs.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,10 @@ internal void AddSessionStateEntry(SessionStateAliasEntry entry, string scopeID)
/// </summary>
internal IDictionary<string, AliasInfo> GetAliasTable()
{
// On 7.0 version we have 132 aliases so we set a larger number to reduce re-allocations.
const int InitialAliasCount = 150;
Dictionary<string, AliasInfo> result =
new Dictionary<string, AliasInfo>(StringComparer.OrdinalIgnoreCase);
new Dictionary<string, AliasInfo>(InitialAliasCount, StringComparer.OrdinalIgnoreCase);

SessionStateScopeEnumerator scopeEnumerator =
new SessionStateScopeEnumerator(_currentScope);
Expand Down
12 changes: 12 additions & 0 deletions test/powershell/engine/Basic/CommandDiscovery.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Copyright (c) Microsoft Corporation. All rights reserved.
# Licensed under the MIT License.

Describe "Command Discovery tests" -Tags "CI" {

BeforeAll {
Expand Down Expand Up @@ -215,4 +216,15 @@ Describe "Command Discovery tests" -Tags "CI" {
{Get-Command -Name 'global:help' -ErrorAction Stop} | Should -Throw -ErrorId 'CommandNotFoundException'
}
}

Context "Native command discovery" {
It 'Can discover a native command without extension' {
$expectedName = if ($IsWindows) { "ping.exe" } else { "ping" }
(Get-Command -Name "ping" -CommandType Application).Name | Should -Match $expectedName
}

It 'Can discover a native command with extension on Windows' -skip:(-not $IsWindows) {
(Get-Command -Name "ping.exe" -CommandType Application).Name | Should -Match "ping.exe"
}
}
}