Hacktoberfest 2026: những issue maintainer đã đánh dấu cho tháng Mười, đang mở và phù hợp người mới. Xem issue Hacktoberfest

CliWrap trialled and rejected — close the gaps in ProcessTasks instead

Đang mở
  • Có gì không ổn?
#583 1 bình luận 0 reaction 1 người được giao Xem trên GitHub

Maintainer thường phản hồi trong vòng 5 ngày

@ChrisonSimtian đang làm issue này rồi.

Từ ngày 27/7/2026.

Đánh giá

Issue này chưa được đánh giá.

Mô tả

enhancement
Decision

CliWrap was trialled in Fallout.Migrate and rejected. Process execution stays on the in-house ProcessTasks (src/Fallout.Tooling/ProcessTasks.cs). No new package dependency.

Trialled and reverted in #576, which needed to shell out to dotnet tool uninstall / dotnet tool install from SwitchGlobalToolStep.

What the comparison showed

Both versions were built and run side by side against dotnet tool list --global, a deliberately failing dotnet tool uninstall, and a 1 ms timeout. Identical exit codes, identical stdout, identical stderr, and both return null on timeout. The difference is only in what the calling code costs to write.

Lines (non-blank)
CliWrap 18
Raw System.Diagnostics.Process, written correctly 47
ProcessTasks (what we shipped) 31

With CliWrap:

using var cancellation = new CancellationTokenSource(commandTimeout);

return await Cli.Wrap("dotnet")
    .WithArguments(arguments)
    .WithValidation(CommandResultValidation.None)
    .ExecuteBufferedAsync(cancellation.Token);

With ProcessTasks:

var argumentLine = arguments.Select(x => x.DoubleQuoteIfNeeded()).JoinSpace();

using var process = ProcessTasks.StartProcess(
    "dotnet", argumentLine, timeout: (int)commandTimeout.TotalMilliseconds,
    logOutput: false, logInvocation: false);

if (process == null || !process.WaitForExit())
    return null;

return new CommandResult(
    process.ExitCode,
    Join(process.Output, OutputType.Std),
    Join(process.Output, OutputType.Err));
What CliWrap actually solves

Worth recording, because these are the traps a hand-rolled runner falls into:

  1. The pipe-buffer deadlock. The obvious raw implementation is WaitForExit() then StandardOutput.ReadToEnd(). That hangs once the child fills the ~64KB pipe buffer while the parent is blocked in WaitForExit. Avoiding it needs event-based reads plus counting two EOF sentinels. ProcessTasks already handles this, which is the main reason it was a viable replacement.
  2. Cancellation that actually kills. WaitForExitAsync(token) returns on timeout but leaves the child running. An explicit Kill(entireProcessTree: true) is required or a timeout orphans the process.
  3. A single result object carrying exit code plus both streams.
What CliWrap does not solve any more

Its reputation is dated on two points:

  • Argument escaping — ProcessStartInfo.ArgumentList has handled quoting since .NET Core 2.1.
  • Async — WaitForExitAsync has been in the BCL since .NET 5.
What using ProcessTasks instead costs

Honest list, since these are the gaps a future improvement would close:

  • No CancellationToken. Only an int millisecond timeout.
  • Synchronous only. No WaitForExitAsync, so a step that is otherwise async has to wrap it.
  • Arguments are one pre-quoted string. No ArgumentList equivalent, so every caller does its own DoubleQuoteIfNeeded(). This is the one place the raw BCL is now better than our own layer.
  • Output is one interleaved collection filtered by OutputType, rather than separate stdout and stderr.
  • Logs through Serilog by default. A tool that renders through Spectre must pass logOutput: false, logInvocation: false.
  • Asserts instead of returning null when the executable is not on PATH, so callers need a try/catch.
  • Pulls Serilog and NuGet.Packaging into anything referencing Fallout.Tooling. For Fallout.Migrate this swapped one direct dependency for two transitive ones.
Acceptance criteria
  • ProcessTasks (or its successor) accepts a CancellationToken alongside the millisecond timeout.
  • An async overload exists, so callers do not have to block a thread.
  • An argument-list overload exists that quotes each argument, instead of every caller calling DoubleQuoteIfNeeded().
  • Not finding the executable on PATH is reportable without a try/catch around the call.
  • docs/dependencies.md keeps pointing here as the reason there is no third-party process runner.
Notes
  • Related: #310 (FT-5, context-scope tool-path resolvers and process defaults) is the natural home for the resolver half of this.
  • Do not re-litigate adding CliWrap without new information. The measured difference is 18 lines against 31, for one call site.
Ngôn ngữ chính
C#
Star
169
Fork
21
Merge trung bình
3 ngày 7 giờ
Pull request đã merge (30 ngày)
9

Chuẩn bị môi trường

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Issue khác của Fallout-build/Fallout

Tất cả issue của Fallout-build/Fallout

Issue tương tự

Thêm issue về C#

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.