Tyrrrz / CliWrap

Library for running command-line processes
MIT License
4.33k stars 266 forks source link

Write to StandardInput as the process is running #129

Closed Socolin closed 2 years ago

Socolin commented 2 years ago

Details

Hello,

I'm trying to replace old Process.Start... with CliWrap and I have a scenario that does not seems to be easily supported yet.

For performance I have to run git cat-file --batch and then when I'm have a hash for which I need the content I'm writing into STDIN the file, then I wait for the STDOUT to read the response and continue processing.

Once all the files have been resolve it close STDIN and then the Process stop.

So I would be looking for some kind of ExecuteInBackgroundAsync() that would returns and object with like STDIN and a like a method WaitToCloseAsync() to retrieve the ExitCode

Maybe with some kind of "pipe stream" and not awaiting ExecuteAsync() I could write a work arround but then it would become more complex than my current code, since in my knowledge, .NET does not provide any stream like this.

Tyrrrz commented 2 years ago

You can use a stream with explicit termination as input: e.g. SimplexStream from here or just implement your own. Then, pipe the output of the command to its handler.

// Pseudocode
var stdin = new SimplexStream();
var cmd = stdin | Cli.Wrap("foo") | (line => Console.WriteLine(line));
var cmdTask = cmd.ExecuteAsync();

// Write stuff to stdin
await stdin.WriteAsync(...);

// Eventually signal completion
stdin.CompleteWriting();

// Await the task
var result = await cmdTask;
silkfire commented 2 years ago

Doesn't a regular MemoryStream act the same way, i.e. CliWrap will be waiting for the stream to close before completing execution of the pipeline? Or does it always close the stream right away and read whatever's already been written to it?

Socolin commented 2 years ago

@Tyrrrz Ho nice I did not knew someone wrote a Stream like that (I've been missing this for quite some times now), look great thanks. And from what I see it's just a wrapper around System.Io.Pipelines.Pipe, I'll need to check this too.

I tried and it was fine for STDIN, I need to call .FlushAsync() manually but this is ok. But then I got another problem with STDOUT this time, since I need to read this as binary I also need a stream here. But I was not able to find a way to call .FlushAsync() when using PipeTarget.ToStream so I ended up overriding WriteAsync to call FlushAsync too and now it's all good.

private class AutoFlushSimplexStream : SimplexStream
{
    public override async Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken)
    {
        await base.WriteAsync(buffer, offset, count, cancellationToken);
        await base.FlushAsync(cancellationToken);
    }
}

Thanks, I'm closing this now.


Doesn't a regular MemoryStream act the same way, i.e. CliWrap will be waiting for the stream to close before completing execution of the pipeline? Or does it always close the stream right away and read whatever's already been written to it?

@silkfire MemoryStream allow you to write into it, then rewind it (memoryStream.Position = 0), then read into it. It does not support scenario where I could write multiple time and it's get read.

Here a small example of what's wrong

[Test]
public async Task MemoryStream()
{
    var memoryStream = new MemoryStream();

    var readTask = Task.Run(async () =>
    {
        var buffer = new byte[1];
        await TestContext.Out.WriteLineAsync("Reading");
        while (await memoryStream.ReadAsync(buffer) != 0)
        {
            await TestContext.Out.WriteLineAsync(buffer[0].ToString());
        }

        await TestContext.Out.WriteLineAsync("Read Completed"); // We end up here before calling .Close()
    });

    var writeTask = Task.Run(async () =>
    {
        await TestContext.Out.WriteLineAsync("Writing");
        await memoryStream.WriteAsync(new byte[] {0x42});
        await memoryStream.WriteAsync(new byte[] {0x52});
        await Task.Delay(TimeSpan.FromSeconds(1));
        await memoryStream.WriteAsync(new byte[] {0x64});
        await Task.Delay(TimeSpan.FromSeconds(1));
        await memoryStream.WriteAsync(new byte[] {0x42});
        await TestContext.Out.WriteLineAsync("Write complete");
        memoryStream.Close();
    });

    await Task.WhenAll(readTask, writeTask);
}
silkfire commented 2 years ago

@Socolin Thanks for your explanation! Regarding the auto flush, iirc, the PipeTarget.ToStream should have an autoFlush option.

Tyrrrz commented 2 years ago

Doesn't a regular MemoryStream act the same way, i.e. CliWrap will be waiting for the stream to close before completing execution of the pipeline? Or does it always close the stream right away and read whatever's already been written to it?

No, a regular MemoryStream is always considered "closed". Once you read all data, it returns 0 on ReadAsync(), which signals end of stream. Streams with explicit termination (e.g. SimplexStream) will not return from ReadAsync() until there is either data to read, or until ReportCompletion() or similar was called.

Tyrrrz commented 2 years ago

I tried and it was fine for STDIN, I need to call .FlushAsync() manually but this is ok. But then I got another problem with STDOUT this time, since I need to read this as binary I also need a stream here. But I was not able to find a way to call .FlushAsync() when using PipeTarget.ToStream so I ended up overriding WriteAsync to call FlushAsync too and now it's all good.

This should already happen by default:

https://github.com/Tyrrrz/CliWrap/blob/954a9cbd322036d8071d513a71402dc361d99973/CliWrap/PipeTarget.cs#L32-L41

https://github.com/Tyrrrz/CliWrap/blob/954a9cbd322036d8071d513a71402dc361d99973/CliWrap/Utils/Extensions/StreamExtensions.cs#L12-L25

Socolin commented 2 years ago

Ho I missed that argument :p Thanks that's awesome :)

Socolin commented 2 years ago

Ho it's true by default.

I dont understand why it was not working when I tested it, it's working perfectly now without specifying it.

silkfire commented 2 years ago

@Tyrrrz Does CliWrap rewind a MemoryStream to the beginning before calling ReadAsync? Otherwise there wouldn't be anything to read?

Tyrrrz commented 2 years ago

@Tyrrrz Does CliWrap rewind a MemoryStream to the beginning before calling ReadAsync? Otherwise there wouldn't be anything to read?

It does not. The user controls that.

However, if you create the memory stream like this:

var ms = new MemoryStream(new byte[] {1,2,3,4,5});

Then it's already at position 0 from beginning.