Skip to content

Commit

Permalink
Remove unnecessary PowerShellProcessArchitecture (#1964)
Browse files Browse the repository at this point in the history 
This simply wasn't being used and was overly complicated. The only time
we want the architecture is when queried via LSP so that the VS Code
client can determine which installer to download for its auto-update
feature. If we can, we should deduplicate version logic in the loader
and the `VersionUtils` class.
  • Loading branch information
@andyleejordan
andyleejordan authored Dec 2, 2022
1 parent 533dc64 commit ee53878
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 116 deletions.
1 change: 1 addition & 0 deletions src/PowerShellEditorServices.Hosting/EditorServicesLoader.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,7 @@ private void LogOperatingSystemDetails()
");
}

// TODO: Deduplicate this with VersionUtils.
private static string GetOSArchitecture()
{
#if CoreCLR
Expand Down
73 changes: 5 additions & 68 deletions src/PowerShellEditorServices/Services/PowerShell/Context/PowerShellVersionDetails.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,34 +11,11 @@ namespace Microsoft.PowerShell.EditorServices.Services.PowerShell.Context
{
using System.Management.Automation;

/// <summary>
/// Defines the possible enumeration values for the PowerShell process architecture.
/// </summary>
internal enum PowerShellProcessArchitecture
{
/// <summary>
/// The processor architecture is unknown or wasn't accessible.
/// </summary>
Unknown,

/// <summary>
/// The processor architecture is 32-bit.
/// </summary>
X86,

/// <summary>
/// The processor architecture is 64-bit.
/// </summary>
X64
}

/// <summary>
/// Provides details about the version of the PowerShell runtime.
/// </summary>
internal class PowerShellVersionDetails
{
#region Properties

/// <summary>
/// Gets the version of the PowerShell runtime.
/// </summary>
Expand All @@ -55,40 +32,26 @@ internal class PowerShellVersionDetails
/// </summary>
public string Edition { get; }

/// <summary>
/// Gets the architecture of the PowerShell process.
/// </summary>
public PowerShellProcessArchitecture Architecture { get; }

#endregion

#region Constructors

/// <summary>
/// Creates an instance of the PowerShellVersionDetails class.
/// </summary>
/// <param name="version">The version of the PowerShell runtime.</param>
/// <param name="versionString">A string representation of the PowerShell version.</param>
/// <param name="editionString">The string representation of the PowerShell edition.</param>
/// <param name="architecture">The processor architecture.</param>
public PowerShellVersionDetails(
Version version,
string versionString,
string editionString,
PowerShellProcessArchitecture architecture)
string editionString)
{
Version = version;
VersionString = versionString;
Edition = editionString;
Architecture = architecture;
}

#endregion

#region Public Methods

/// <summary>
/// Gets the PowerShell version details for the given runspace.
/// Gets the PowerShell version details for the given runspace. This doesn't use
/// VersionUtils because we may be remoting, and therefore want the remote runspace's
/// version, not the local process.
/// </summary>
/// <param name="logger">An ILogger implementation used for writing log messages.</param>
/// <param name="pwsh">The PowerShell instance for which to get the version.</param>
Expand All @@ -98,7 +61,6 @@ public static PowerShellVersionDetails GetVersionDetails(ILogger logger, PowerSh
Version powerShellVersion = new(5, 0);
string versionString = null;
string powerShellEdition = "Desktop";
PowerShellProcessArchitecture architecture = PowerShellProcessArchitecture.Unknown;

try
{
Expand Down Expand Up @@ -129,25 +91,6 @@ public static PowerShellVersionDetails GetVersionDetails(ILogger logger, PowerSh
}

versionString = psVersionTable["GitCommitId"] is string gitCommitId ? gitCommitId : powerShellVersion.ToString();

PSCommand procArchCommand = new PSCommand().AddScript("$env:PROCESSOR_ARCHITECTURE", useLocalScope: true);

string arch = pwsh
.AddScript("$env:PROCESSOR_ARCHITECTURE", useLocalScope: true)
.InvokeAndClear<string>()
.FirstOrDefault();

if (arch != null)
{
if (string.Equals(arch, "AMD64", StringComparison.CurrentCultureIgnoreCase))
{
architecture = PowerShellProcessArchitecture.X64;
}
else if (string.Equals(arch, "x86", StringComparison.CurrentCultureIgnoreCase))
{
architecture = PowerShellProcessArchitecture.X86;
}
}
}
}
catch (Exception ex)
Expand All @@ -156,13 +99,7 @@ public static PowerShellVersionDetails GetVersionDetails(ILogger logger, PowerSh
"Failed to look up PowerShell version, defaulting to version 5.\r\n\r\n" + ex.ToString());
}

return new PowerShellVersionDetails(
powerShellVersion,
versionString,
powerShellEdition,
architecture);
return new PowerShellVersionDetails(powerShellVersion, versionString, powerShellEdition);
}

#endregion
}
}
25 changes: 1 addition & 24 deletions src/PowerShellEditorServices/Services/PowerShell/Handlers/GetVersionHandler.cs
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

using System;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.PowerShell.EditorServices.Services.PowerShell;
Expand All @@ -13,35 +12,13 @@ internal class GetVersionHandler : IGetVersionHandler
{
public async Task<PowerShellVersion> Handle(GetVersionParams request, CancellationToken cancellationToken)
{
PowerShellProcessArchitecture architecture = PowerShellProcessArchitecture.Unknown;
// This should be changed to using a .NET call sometime in the future... but it's just for logging purposes.
string arch = Environment.GetEnvironmentVariable("PROCESSOR_ARCHITECTURE");
if (arch != null)
{
if (string.Equals(arch, "AMD64", StringComparison.CurrentCultureIgnoreCase))
{
architecture = PowerShellProcessArchitecture.X64;
}
else if (string.Equals(arch, "x86", StringComparison.CurrentCultureIgnoreCase))
{
architecture = PowerShellProcessArchitecture.X86;
}
}

return new PowerShellVersion
{
Version = VersionUtils.PSVersionString,
Edition = VersionUtils.PSEdition,
DisplayVersion = VersionUtils.PSVersion.ToString(2),
Architecture = architecture.ToString()
Architecture = VersionUtils.Architecture
};
}

private enum PowerShellProcessArchitecture
{
Unknown,
X86,
X64
}
}
}
29 changes: 5 additions & 24 deletions src/PowerShellEditorServices/Services/PowerShell/Handlers/IGetVersionHandler.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// Licensed under the MIT License.

using MediatR;
using Microsoft.PowerShell.EditorServices.Services.PowerShell.Context;
using OmniSharp.Extensions.JsonRpc;

namespace Microsoft.PowerShell.EditorServices.Services.PowerShell
Expand All @@ -12,29 +11,11 @@ internal interface IGetVersionHandler : IJsonRpcRequestHandler<GetVersionParams,

internal class GetVersionParams : IRequest<PowerShellVersion> { }

internal class PowerShellVersion
internal record PowerShellVersion
{
public string Version { get; set; }
public string DisplayVersion { get; set; }
public string Edition { get; set; }
public string Architecture { get; set; }

public PowerShellVersion()
{
}

public PowerShellVersion(PowerShellVersionDetails versionDetails)
{
Version = versionDetails.VersionString;
DisplayVersion = $"{versionDetails.Version.Major}.{versionDetails.Version.Minor}";
Edition = versionDetails.Edition;

Architecture = versionDetails.Architecture switch
{
PowerShellProcessArchitecture.X64 => "x64",
PowerShellProcessArchitecture.X86 => "x86",
_ => "Architecture Unknown",
};
}
public string Version { get; init; }
public string DisplayVersion { get; init; }
public string Edition { get; init; }
public string Architecture { get; init; }
}
}
24 changes: 24 additions & 0 deletions src/PowerShellEditorServices/Utility/VersionUtils.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,11 @@ internal static class VersionUtils
/// True if we are running on Linux, false otherwise.
/// </summary>
public static bool IsLinux { get; } = RuntimeInformation.IsOSPlatform(OSPlatform.Linux);

/// <summary>
/// The .NET Architecture as a string.
/// </summary>
public static string Architecture { get; } = PowerShellReflectionUtils.GetOSArchitecture();
}

internal static class PowerShellReflectionUtils
Expand Down Expand Up @@ -96,5 +101,24 @@ internal static class PowerShellReflectionUtils
public static string PSVersionString { get; } = s_psCurrentVersionProperty != null
? s_psCurrentVersionProperty.GetValue(null).ToString()
: s_psVersionProperty.GetValue(null).ToString();

public static string GetOSArchitecture()
{
#if CoreCLR
if (Environment.OSVersion.Platform != PlatformID.Win32NT)
{
return RuntimeInformation.OSArchitecture.ToString();
}
#endif
// If on win7 (version 6.1.x), avoid System.Runtime.InteropServices.RuntimeInformation
if (Environment.OSVersion.Version < new Version(6, 2))
{
return Environment.Is64BitProcess
? "X64"
: "X86";
}

return RuntimeInformation.OSArchitecture.ToString();
}
}
}

0 comments on commit ee53878

Please sign in to comment.