Skip to content

Commit 3a0e1bb

Browse files
tonybaloneyatifaziz
tonybaloney
and
atifaziz
authored
Change all spawned processes to use argument lists. Add InstallPackage to IPythonPackageInstaller (#470)
* Make all process arguments lists, ensure changes are propagated. * Propagate arguments through correctly * Fix uv arguments * Apply suggestions from code review Co-authored-by: Atif Aziz <[email protected]> * Simplify the pip and uv classes --------- Co-authored-by: Atif Aziz <[email protected]>
1 parent d3fee0e commit 3a0e1bb

File tree

6 files changed

+55
-28
lines changed

6 files changed

+55
-28
lines changed

‎src/CSnakes.Runtime/EnvironmentManagement/VenvEnvironmentManagement.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ public void EnsureEnvironment(PythonLocationMetadata pythonLocation)
2020
if (!Directory.Exists(path))
2121
{
2222
logger?.LogDebug("Creating virtual environment at {VirtualEnvPath} using {PythonBinaryPath}", fullPath, pythonLocation.PythonBinaryPath);
23-
var (process1, _, _) = ProcessUtils.ExecutePythonCommand(logger, pythonLocation, $"-VV");
24-
var (process2, _, error) = ProcessUtils.ExecutePythonCommand(logger, pythonLocation, $"-m venv {fullPath}");
23+
var (process1, _, _) = ProcessUtils.ExecutePythonCommand(logger, pythonLocation, "-VV");
24+
var (process2, _, error) = ProcessUtils.ExecutePythonCommand(logger, pythonLocation, "-m", "venv", fullPath);
2525

2626
if (process1.ExitCode != 0 || process2.ExitCode != 0)
2727
{

‎src/CSnakes.Runtime/Locators/CondaLocator.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ internal CondaLocator(ILogger? logger, string condaBinaryPath)
1717
{
1818
this.logger = logger;
1919
this.condaBinaryPath = condaBinaryPath;
20-
var (process, result, errors) = ExecuteCondaCommand($"info --json");
20+
var (process, result, errors) = ExecuteCondaCommand("info", "--json");
2121
if (process.ExitCode != 0)
2222
{
2323
logger?.LogError("Failed to determine Python version from Conda {Error}.", errors);
@@ -43,9 +43,9 @@ internal CondaLocator(ILogger? logger, string condaBinaryPath)
4343
folder = basePrefix;
4444
}
4545

46-
internal (Process process, string? output, string? errors) ExecuteCondaCommand(string arguments) => ProcessUtils.ExecuteCommand(logger, condaBinaryPath, arguments);
46+
internal (Process process, string? output, string? errors) ExecuteCondaCommand(params string[] arguments) => ProcessUtils.ExecuteCommand(logger, condaBinaryPath, arguments);
4747

48-
internal bool ExecuteCondaShellCommand(string arguments) => ProcessUtils.ExecuteShellCommand(logger, condaBinaryPath, arguments);
48+
internal bool ExecuteCondaShellCommand(params string[] arguments) => ProcessUtils.ExecuteShellCommand(logger, condaBinaryPath, arguments);
4949

5050
public override PythonLocationMetadata LocatePython() =>
5151
LocatePythonInternal(folder);

‎src/CSnakes.Runtime/PackageManagement/IPythonPackageInstaller.cs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,4 +14,13 @@ public interface IPythonPackageInstaller
1414
/// <param name="virtualEnvironmentLocation">The location of the virtual environment (optional).</param>
1515
/// <returns>A task representing the asynchronous package installation operation.</returns>
1616
Task InstallPackages(string home, IEnvironmentManagement? environmentManager);
17+
18+
/// <summary>
19+
/// Install a single package.
20+
/// </summary>
21+
/// <param name="home">The home directory.</param>
22+
/// <param name="environmentManager">The location of the virtual environment (optional).</param>
23+
/// <param name="package">Name of the package to install.</param>
24+
/// <returns>A task representing the asynchronous package installation operation.</returns>
25+
Task InstallPackage(string home, IEnvironmentManagement? environmentManager, string package);
1726
}

‎src/CSnakes.Runtime/PackageManagement/PipInstaller.cs

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ public Task InstallPackages(string home, IEnvironmentManagement? environmentMana
1414
if (File.Exists(requirementsPath))
1515
{
1616
logger?.LogDebug("File {Requirements} was found.", requirementsPath);
17-
InstallPackagesWithPip(home, environmentManager, $"-r {requirementsFileName}", logger);
17+
RunPipInstall(home, environmentManager, ["-r", requirementsFileName], logger);
1818
}
1919
else
2020
{
@@ -24,12 +24,22 @@ public Task InstallPackages(string home, IEnvironmentManagement? environmentMana
2424
return Task.CompletedTask;
2525
}
2626

27-
internal static void InstallPackagesWithPip(string home, IEnvironmentManagement? environmentManager, string requirements, ILogger? logger)
27+
public Task InstallPackage(string home, IEnvironmentManagement? environmentManager, string package)
28+
{
29+
RunPipInstall(home, environmentManager, [package], logger);
30+
31+
return Task.CompletedTask;
32+
}
33+
34+
internal static void InstallPackageWithPip(string home, IEnvironmentManagement? environmentManager, string requirement, ILogger? logger)
35+
=> RunPipInstall(home, environmentManager, [requirement], logger);
36+
37+
private static void RunPipInstall(string home, IEnvironmentManagement? environmentManager, string[] requirements, ILogger? logger)
2838
{
2939
string fileName = pipBinaryName;
3040
string workingDirectory = home;
3141
string path = "";
32-
string arguments = $"install {requirements} --disable-pip-version-check";
42+
string[] arguments = [ "install", .. requirements, "--disable-pip-version-check" ];
3343

3444
if (environmentManager is not null)
3545
{

‎src/CSnakes.Runtime/PackageManagement/UVInstaller.cs

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,19 @@ internal class UVInstaller(ILogger<UVInstaller>? logger, string requirementsFile
88
{
99
static readonly string binaryName = $"uv{(RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? ".exe" : "")}";
1010

11+
/// <summary>
12+
/// Install packages from a requirements path
13+
/// </summary>
14+
/// <param name="home">HOME directory</param>
15+
/// <param name="environmentManager">Environment manager</param>
16+
/// <returns>A task.</returns>
1117
public Task InstallPackages(string home, IEnvironmentManagement? environmentManager)
1218
{
1319
string requirementsPath = Path.GetFullPath(Path.Combine(home, requirementsFileName));
1420
if (File.Exists(requirementsPath))
1521
{
1622
logger?.LogDebug("File {Requirements} was found.", requirementsPath);
17-
InstallPackagesWithUv(home, environmentManager, $"-r {requirementsFileName}", logger);
23+
RunUvPipInstall(home, environmentManager, ["-r", requirementsFileName], logger);
1824
}
1925
else
2026
{
@@ -24,12 +30,19 @@ public Task InstallPackages(string home, IEnvironmentManagement? environmentMana
2430
return Task.CompletedTask;
2531
}
2632

27-
static internal void InstallPackagesWithUv(string home, IEnvironmentManagement? environmentManager, string requirements, ILogger? logger)
33+
public Task InstallPackage(string home, IEnvironmentManagement? environmentManager, string package)
34+
{
35+
RunUvPipInstall(home, environmentManager, [package], logger);
36+
37+
return Task.CompletedTask;
38+
}
39+
40+
static private void RunUvPipInstall(string home, IEnvironmentManagement? environmentManager, string[] requirements, ILogger? logger)
2841
{
2942
string fileName = binaryName;
3043
string workingDirectory = home;
3144
string path = "";
32-
string arguments = $"pip install {requirements} --color never";
45+
string[] arguments = ["pip", "install", .. requirements, "--color", "never"];
3346

3447
if (environmentManager is not null)
3548
{
@@ -42,7 +55,7 @@ static internal void InstallPackagesWithUv(string home, IEnvironmentManagement?
4255
if (!File.Exists(uvPath))
4356
{
4457
// Install it with pip
45-
PipInstaller.InstallPackagesWithPip(home, environmentManager, "uv", logger);
58+
PipInstaller.InstallPackageWithPip(home, environmentManager, "uv", logger);
4659
}
4760

4861
fileName = uvPath;

‎src/CSnakes.Runtime/ProcessUtils.cs

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,45 +1,41 @@
11
using CSnakes.Runtime.Locators;
22
using Microsoft.Extensions.Logging;
3+
using System.Collections.ObjectModel;
34
using System.Diagnostics;
45

56
namespace CSnakes.Runtime;
67

78
internal static class ProcessUtils
89
{
9-
internal static (Process proc, string? result, string? errors) ExecutePythonCommand(ILogger? logger, PythonLocationMetadata pythonLocation, string arguments)
10+
internal static (Process proc, string? result, string? errors) ExecutePythonCommand(ILogger? logger, PythonLocationMetadata pythonLocation, params string[] arguments)
1011
{
11-
ProcessStartInfo startInfo = new()
12+
13+
ProcessStartInfo startInfo = new(pythonLocation.PythonBinaryPath, arguments)
1214
{
1315
WorkingDirectory = pythonLocation.Folder,
14-
FileName = pythonLocation.PythonBinaryPath,
15-
Arguments = arguments,
1616
RedirectStandardError = true,
1717
RedirectStandardOutput = true,
1818
CreateNoWindow = true,
1919
};
2020
return ExecuteCommand(logger, startInfo);
2121
}
2222

23-
internal static (Process proc, string? result, string? errors) ExecuteCommand(ILogger? logger, string fileName, string arguments)
23+
internal static (Process proc, string? result, string? errors) ExecuteCommand(ILogger? logger, string fileName, params string[] arguments)
2424
{
25-
ProcessStartInfo startInfo = new()
25+
ProcessStartInfo startInfo = new(fileName, arguments)
2626
{
27-
FileName = fileName,
28-
Arguments = arguments,
2927
RedirectStandardError = true,
3028
RedirectStandardOutput = true,
3129
CreateNoWindow = true,
3230
};
3331
return ExecuteCommand(logger, startInfo);
3432
}
3533

36-
internal static bool ExecuteShellCommand(ILogger? logger, string fileName, string arguments)
34+
internal static bool ExecuteShellCommand(ILogger? logger, string fileName, params string[] arguments)
3735
{
3836
logger?.LogDebug("Executing shell command {FileName} {Arguments}", fileName, arguments);
39-
ProcessStartInfo startInfo = new()
37+
ProcessStartInfo startInfo = new(fileName, arguments)
4038
{
41-
FileName = fileName,
42-
Arguments = arguments,
4339
UseShellExecute = true,
4440
CreateNoWindow = true,
4541
WindowStyle = ProcessWindowStyle.Hidden,
@@ -80,13 +76,12 @@ private static (Process proc, string? result, string? errors) ExecuteCommand(ILo
8076
process.WaitForExit();
8177
return (process, result, errors);
8278
}
83-
internal static void ExecuteProcess(string fileName, string arguments, string workingDirectory, string path, ILogger? logger, IReadOnlyDictionary<string, string?>? extraEnv = null)
79+
80+
internal static void ExecuteProcess(string fileName, IEnumerable<string> arguments, string workingDirectory, string path, ILogger? logger, IReadOnlyDictionary<string, string?>? extraEnv = null)
8481
{
85-
ProcessStartInfo startInfo = new()
82+
ProcessStartInfo startInfo = new(fileName, arguments)
8683
{
8784
WorkingDirectory = workingDirectory,
88-
FileName = fileName,
89-
Arguments = arguments,
9085
CreateNoWindow = true,
9186
};
9287

0 commit comments

Comments
 (0)