Skip to content
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

Support unc paths for command execution #784

Merged
merged 9 commits into from
May 25, 2016
Merged

Support unc paths for command execution #784

Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
88a186f
Extract common argument logic
mjbvz Mar 28, 2016
361ccb1
Fix ProcessOutput not supporting unc paths
mjbvz Mar 29, 2016
a259cdc
small cleanup
mjbvz Mar 29, 2016
03411a2
Use QuoteSingleArgument
mjbvz Mar 29, 2016
c6e0259
Use QuoteSingleArgument for working direcotry
mjbvz Mar 29, 2016
7d1af57
Fix indent
mjbvz Mar 29, 2016
a811a3c
Merge branch 'master' into unc-path-fix
mjbvz May 19, 2016
36dcb99
SEt UseShellExecute to true for consistency
mjbvz May 24, 2016
2d62755
Revert setting useshell execute to false
mjbvz May 24, 2016
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
72 changes: 34 additions & 38 deletions Common/Product/SharedProject/ProcessOutput.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,23 +184,20 @@ public static ProcessOutput Run(
redirector,
quoteArgs,
outputEncoding,
errorEncoding
);
}

var psi = new ProcessStartInfo(filename);
if (quoteArgs) {
psi.Arguments = string.Join(" ",
arguments.Where(a => a != null).Select(QuoteSingleArgument));
} else {
psi.Arguments = string.Join(" ", arguments.Where(a => a != null));
}
psi.WorkingDirectory = workingDirectory;
psi.CreateNoWindow = !visible;
psi.UseShellExecute = false;
psi.RedirectStandardError = !visible || (redirector != null);
psi.RedirectStandardOutput = !visible || (redirector != null);
psi.RedirectStandardInput = !visible;
errorEncoding);
}

var psi = new ProcessStartInfo("cmd.exe") {
Arguments = string.Format(@"/S /C pushd {0} & {1} {2}",
Copy link
Contributor

Choose a reason for hiding this comment

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

does pushd have a timeout? What happens if there is no network access or the drive is inaccessible?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, between 45 and 120 seconds from what I can gather (it seem to differ by Windows version.) I don't see any easy way to adjust the timeout for a single command.

Not ideal, but also not a huge problem. It's more an external dependency that we should be aware of when trying to diagnose reported problems.

QuoteSingleArgument(workingDirectory),
QuoteSingleArgument(filename),
GetArguments(arguments, quoteArgs)),
CreateNoWindow = !visible,
UseShellExecute = false,
Copy link
Contributor

Choose a reason for hiding this comment

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

make this true

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nvrmind, reverted the suggested fix. This has to be false because we set the Redirect* properties. The elevated example manually redirects to a file, which is why it can get away with setting this to true.

RedirectStandardError = !visible || (redirector != null),
RedirectStandardOutput = !visible || (redirector != null),
RedirectStandardInput = !visible
};
psi.StandardOutputEncoding = outputEncoding ?? psi.StandardOutputEncoding;
psi.StandardErrorEncoding = errorEncoding ?? outputEncoding ?? psi.StandardErrorEncoding;
if (env != null) {
Expand Down Expand Up @@ -237,27 +234,18 @@ public static ProcessOutput RunElevated(
) {
var outFile = Path.GetTempFileName();
var errFile = Path.GetTempFileName();
var psi = new ProcessStartInfo("cmd.exe");
psi.CreateNoWindow = true;
psi.WindowStyle = ProcessWindowStyle.Hidden;
psi.UseShellExecute = true;
psi.Verb = "runas";

string args;
if (quoteArgs) {
args = string.Join(" ", arguments.Where(a => a != null).Select(QuoteSingleArgument));
} else {
args = string.Join(" ", arguments.Where(a => a != null));
}
psi.Arguments = string.Format("/S /C \"{0} {1} >>{2} 2>>{3}\"",
QuoteSingleArgument(filename),
args,
QuoteSingleArgument(outFile),
QuoteSingleArgument(errFile)
);
psi.WorkingDirectory = workingDirectory;
psi.CreateNoWindow = true;
psi.UseShellExecute = true;
var psi = new ProcessStartInfo("cmd.exe") {
WindowStyle = ProcessWindowStyle.Hidden,
Verb = "runas",
CreateNoWindow = true,
UseShellExecute = true,
Arguments = string.Format(@"/S /C pushd {0} & ""{1} {2} >>{3} 2>>{4}""",
QuoteSingleArgument(workingDirectory),
QuoteSingleArgument(filename),
GetArguments(arguments, quoteArgs),
QuoteSingleArgument(outFile),
QuoteSingleArgument(errFile))
};

var process = new Process();
process.StartInfo = psi;
Expand Down Expand Up @@ -316,6 +304,14 @@ public static ProcessOutput RunElevated(
return result;
}

private static string GetArguments(IEnumerable<string> arguments, bool quoteArgs) {
if (quoteArgs) {
return string.Join(" ", arguments.Where(a => a != null).Select(QuoteSingleArgument));
} else {
return string.Join(" ", arguments.Where(a => a != null));
}
}

internal static IEnumerable<string> SplitLines(string source) {
int start = 0;
int end = source.IndexOfAny(EolChars);
Expand Down