I rebased the code to the latest master, and in a few days I will proceed to fix it based on changes requested in the old PR 👍

Is there other software that has this option name (--stdin-from-command)? It's quite long, feels like --stdin-command is more concise without losing any semantics.

Is there other software that has this option name (--stdin-from-command)? It's quite long, feels like --stdin-command is more concise without losing any semantics.

Actually, content-from-command may be better, that is also the name it has in Borg backup. See #4254 (comment)

Nah, there's already two --stdin-* options, no point in deviating from that prefix.

@MichaelEischer I've implemented your suggestions from the old pull request. I implemented a new io.ReadCloser instead of a new fs.FS, calling cmd.Wait() on io.ReadCloser.Close(). The archiver is back to the original code.

I'm testing the code now :-)

Thank you for continuing with this feature. I tested your branch and it still creates snapshots when subcommand exists with non zero:

$ restic-stdin-command/restic backup --stdin-from-command -- pg_dump -h foobar  
repository 12ba8696 opened (version 2, compression level auto)
using parent snapshot 75e9f4c7
subprocess pg_dump: pg_dump: error: could not translate host name "foobar" to address: Name or service not known
error: read /stdin: no data read

Files:           0 new,     0 changed,     0 unmodified
Dirs:            0 new,     0 changed,     0 unmodified
Added to the repository: 0 B   (0 B   stored)

processed 0 files, 0 B in 0:00
snapshot d044b2eb saved
Warning: at least one source file could not be read

$ echo $?
3

$ pg_dump -h foobar; echo $?
pg_dump: error: could not translate host name "foobar" to address: Name or service not known
1

Thank you for continuing with this feature. I tested your branch and it still creates snapshots when subcommand exists with non zero:

$ restic-stdin-command/restic backup --stdin-from-command -- pg_dump -h foobar  
repository 12ba8696 opened (version 2, compression level auto)
using parent snapshot 75e9f4c7
subprocess pg_dump: pg_dump: error: could not translate host name "foobar" to address: Name or service not known
error: read /stdin: no data read

Files:           0 new,     0 changed,     0 unmodified
Dirs:            0 new,     0 changed,     0 unmodified
Added to the repository: 0 B   (0 B   stored)

processed 0 files, 0 B in 0:00
snapshot d044b2eb saved
Warning: at least one source file could not be read

$ echo $?
3

$ pg_dump -h foobar; echo $?
pg_dump: error: could not translate host name "foobar" to address: Name or service not known
1

Uh, I was about to do the same test :-D

Thanks for your report, I'll investigate and fix 👍

The problem was that the error returned in file reading or closing was logged, but the snapshot itself was continuing unaffected (because the logic assumed that an error while reading one file should not abort the snapshot).

The ReadCloserCommand now returns a "fatal" error, and the error is checked in the cmd_backup.go file: if it is "fatal", the snapshot is aborted. Otherwise, the behavior is the same as before: log the error, and exit with code 3 after completing the snapshot.

What do you think?

What do you think?

I'm not sure about the command terminated with no output fatal error. The current behaviour in stdin mode is to log no data read and exit with status code 3 when reading no data.

I like your current implementation more, because having a snapshot with no data makes no sense for me. But it might be behaving differently than we have in existing implementations.

What do you think?

I'm not sure about the command terminated with no output fatal error. The current behaviour in stdin mode is to log no data read and exit with status code 3 when reading no data.

I like your current implementation more, because having a snapshot with no data makes no sense for me. But it might be behaving differently than we have in existing implementations.

You are right, it is a different behavior. I assumed that this is expected, given that we are explicitly asking for monitoring the command.

On second thought, the best way is to check the command exit code also in ReadCloserCommand.Read(), and return a fatal error only when there is an EOF and the exit code is non-zero. By doing so, if the command exits with 0 and there is no output, the snapshot is created with a warning (which is the current behavior for files and stdin), while the snapshot is not created if the exit code is non-zero (which is expected).

I'll modify the code asap :-)

Ok, now the code should stop the snapshot creation by looking only at the exit code. If there is no output, the snapshot is still created with a warning (and exit code 3), which is the same behavior that other snapshot options have (--stdin, but also normal file backup). Let me know if there is something we can improve :-)

Works for me. Very good!

I think there should now be some feedback on the code itself.

Works for me. Very good!

I think there should now be some feedback on the code itself.

Agreed, thanks for feedbacks and tests :-)

I rebased the branch to the latest master :-)

@Enrico204 Thanks for finishing the PR, and sorry for the late feedback. I've somehow missed that the review comments have been addressed in the meantime.

I've reworked the code a bit: the command startup is now handled by fs.CommandReader (that's the new name for the helper in fs). Instead of using canceling the backup indirectly using a context, arch.Error now just returns an error if it is Fatal. This also allows for a nicer error message.

No problem at all :-)

I will compile this branch and I will test it as soon as possible!

I am testing the code since a few weeks: so far, no issues :-)