Skip to content

Update ComputerUnix.cs to fix Restart- and Stop-Computer on Linux/Mac #19780

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

Closed
mark-wilson-spc wants to merge 9 commits into from
Closed

Update ComputerUnix.cs to fix Restart- and Stop-Computer on Linux/Mac #19780

Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
1fe8294
Update ComputerUnix.cs to fix Restart- and Stop-Computer on Linux and…
mark-wilson-spc Jun 10, 2023
1ec7d7f
Change RunCommand args default value to constant
mark-wilson-spc Jun 11, 2023
36f334c
Update RunCommand params per PR input from iSazinov
mark-wilson-spc Jun 13, 2023
439f210
Update ComputerUnix.cs change MacOs Stop- args to -h now
mark-wilson-spc Jun 14, 2023
a721390
Update ComputerUnix.cs with MacOs and Unix force and noforce args
mark-wilson-spc Jun 16, 2023
fe7f66a
Update ComputerUnix.cs - add const to string declarations and remove …
mark-wilson-spc Jun 16, 2023
d2c064f
Update ComputerUnix.cs Restore default value FALSE to FORCE params
mark-wilson-spc Jun 16, 2023
1190c86
Update ComputerUnix.cs fix const string syntax error
mark-wilson-spc Jun 16, 2023
fa1cc99
Update ComputerUnix.cs fix syntax error
mark-wilson-spc Jun 16, 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
91 changes: 84 additions & 7 deletions 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 @@ -21,6 +21,16 @@ namespace Microsoft.PowerShell.Commands
public sealed class RestartComputerCommand : CommandLineCmdletBase
{
// TODO: Support remote computers?

#region "Parameters"

/// <summary>
/// Force the operation to take place if possible.
/// </summary>
[Parameter]
public SwitchParameter Force { get; set; }

#endregion "Parameters"

#region "Overrides"

Expand All @@ -29,6 +39,36 @@ public sealed class RestartComputerCommand : CommandLineCmdletBase
/// </summary>
protected override void BeginProcessing()
{
const string unixRestartCommand = "/sbin/shutdown";
const string unixRestartArgs = "-r now";

const string macOSRestartCommand = "osascript";
const string macOSRestartArgs = @"-e 'tell application ""System Events"" to restart'";
Copy link
Copy Markdown
Collaborator

@JamesWTruher JamesWTruher Jul 17, 2023

Choose a reason for hiding this comment

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

if this is the more gentle approach, would it be a good idea to pop UI for confirmation?

Copy link
Copy Markdown
Collaborator

@iSazonov iSazonov Jan 9, 2024

Choose a reason for hiding this comment

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

ShouldProcess might be appropriate.


const string macOSForceRestartCommand = "/sbin/shutdown";
const string macOSForceRestartArgs = "-r now";

string command;
string args;

if (Platform.IsMacOS)
{
if (Force.IsPresent)
{
command = macOSForceRestartCommand;
args = macOSForceRestartArgs;
}
else
{
command = macOSRestartCommand;
args = macOSRestartArgs;
}
}
else {
command = unixRestartCommand;
args = unixRestartArgs;
}

Copy link
Copy Markdown
Collaborator

@JamesWTruher JamesWTruher Jul 17, 2023

Choose a reason for hiding this comment

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

now that you've changed the way shutdown is being called (and adding the more gentle osascript approach, I would be happier if we boosted the tests here, at least to have validation between force and not-force on mac

Copy link
Copy Markdown
Member

@SteveL-MSFT SteveL-MSFT Jul 17, 2023

Choose a reason for hiding this comment

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

In CI, a test hook will have to be needed to verify intended behavior

if (InternalTestHooks.TestStopComputer)
{
var retVal = InternalTestHooks.TestStopComputerResults;
Expand All @@ -42,7 +82,7 @@ protected override void BeginProcessing()
return;
}

RunCommand("/sbin/shutdown", "-r now");
RunCommand(command, args);
}
#endregion "Overrides"
}
Expand All @@ -58,19 +98,56 @@ protected override void BeginProcessing()
public sealed class StopComputerCommand : CommandLineCmdletBase
{
// TODO: Support remote computers?

#region "Parameters"

/// <summary>
/// Force the operation to take place if possible.
/// </summary>
[Parameter]
public SwitchParameter Force { get; set; }

#endregion "Parameters"

#region "Overrides"

/// <summary>
/// BeginProcessing.
/// </summary>
protected override void BeginProcessing()
{
var args = "-P now";

const string unixStopCommand = "/sbin/shutdown";
const string unixStopArgs = "-P now";

const string macOSStopCommand = "osascript";
const string macOSStopArgs = @"-e 'tell application ""System Events"" to shut down'";

const string macOSForceStopCommand = "/sbin/shutdown";
const string macOSForceStopArgs = "-h now";

string command;
string args;

if (Platform.IsMacOS)
{
args = "now";
if (Force.IsPresent)
{
command = macOSForceStopCommand;
args = macOSForceStopArgs;
}
else
{
command = macOSStopCommand;
args = macOSStopArgs;
}
}
else
{
command = unixStopCommand;
args = unixStopArgs;
}

if (InternalTestHooks.TestStopComputer)
{
var retVal = InternalTestHooks.TestStopComputerResults;
Expand All @@ -84,7 +161,7 @@ protected override void BeginProcessing()
return;
}

RunCommand("/sbin/shutdown", args);
RunCommand(command, args);
}
#endregion "Overrides"
}
Expand Down Expand Up @@ -153,13 +230,13 @@ protected override void StopProcessing()
/// <summary>
/// Run a command.
/// </summary>
protected void RunCommand(String command, String args) {
protected void RunCommand(string command, string args) {
Copy link
Copy Markdown
Contributor

@mklement0 mklement0 Jun 14, 2023

Choose a reason for hiding this comment

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

The way /sbin/shutdown is invoked makes it impossible to detect failure on the PowerShell side, though that's probably for a separate PR; see

_process = new Process()
{
StartInfo = new ProcessStartInfo
{
FileName = "/sbin/shutdown",
Arguments = string.Empty,
FileName = command,
Arguments = args,
RedirectStandardOutput = false,
UseShellExecute = false,
CreateNoWindow = true,
Expand Down