Hacktoberfest 2026: le issue che i maintainer hanno segnato per ottobre, aperte e adatte ai principianti. Sfoglia le issue Hacktoberfest

CliWrap trialled and rejected — close the gaps in ProcessTasks instead

Aperta
  • Qualcosa non va?
#583 1 commento 0 reazioni 1 assegnatario Vedi su GitHub

I maintainer di solito rispondono entro 5 giorni

@ChrisonSimtian ci sta già lavorando.

Dal 27/7/2026.

Valutazione

Questa issue non è ancora stata valutata.

Descrizione

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.
Lingua principale
C#
Stelle
169
Fork
21
Merge medio
3g 7h
PR unite (30g)
9

Preparare l'ambiente

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di Fallout-build/Fallout

Tutte le issue di Fallout-build/Fallout

Issue simili

Altre issue su C#

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.