From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 57D951FF145 for ; Thu, 05 Feb 2026 15:44:37 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 577581411F; Thu, 5 Feb 2026 15:45:09 +0100 (CET) Message-ID: Date: Thu, 5 Feb 2026 15:45:04 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v5 proxmox-backup 10/16] client: treat minus sign as stdin To: Robert Obkircher , pbs-devel@lists.proxmox.com References: <20260130164552.281581-1-r.obkircher@proxmox.com> <20260130164552.281581-11-r.obkircher@proxmox.com> Content-Language: en-US, de-DE From: Christian Ebner In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1770302627248 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.047 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: YGGZBDZL3U44F7A6DXRAPMN2DHZ5NVBK X-Message-ID-Hash: YGGZBDZL3U44F7A6DXRAPMN2DHZ5NVBK X-MailFrom: c.ebner@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Backup Server development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On 2/5/26 2:39 PM, Robert Obkircher wrote: > > On 2/4/26 15:42, Christian Ebner wrote: >> On 2/4/26 3:00 PM, Robert Obkircher wrote: >>> >>> On 2/2/26 13:14, Christian Ebner wrote: >>>> On 1/30/26 5:45 PM, Robert Obkircher wrote: >>>>> Treat "-" as an alias for "/dev/stdin". If there is an actual file >>>>> with that name it can still be read via "./-". >>>>> >>>>> Signed-off-by: Robert Obkircher >>>>> --- >>>>>    proxmox-backup-client/src/main.rs | 7 +++++++ >>>>>    1 file changed, 7 insertions(+) >>>>> >>>>> diff --git a/proxmox-backup-client/src/main.rs >>>>> b/proxmox-backup-client/src/main.rs >>>>> index 7fc711fd..37878b01 100644 >>>>> --- a/proxmox-backup-client/src/main.rs >>>>> +++ b/proxmox-backup-client/src/main.rs >>>>> @@ -845,6 +845,13 @@ async fn create_backup( >>>>>            } >>>>>            target_set.insert(target.clone()); >>>>>    +        // one can still use ./- to refer to an actual file with >>>>> that name >>>>> +        let filename = if filename == "-" { >>>>> +            String::from("/dev/stdin") >>>> >>>> should we add a warning here? >>>> >>>> This is actually a breaking change, but guarding it behind a flag is >>>> maybe a bit overkill. We could check if the file exists before >>>> continuing and if so fail with an appropriate warning/error? >>> >>> Another option would be to just require /dev/stdin and print a hint >>> about that if the user tries to specify "-". What do you think >>> about that? >> >> Might be the better option for the time being, yes. And must be >> documented correctly of course. The '-' shortcut could still be >> added as breaking change in a future version. >> >> But now I'm wondering, how is the multi archive case handled if >> reading from stdin? >> >> To extend your example in the cover letter: `ssh host cmd | >> proxmox-backup-client backup a.img:/dev/stdin b.img:/dev/stdin` >> >> Without having tested it I guess this will fail on the second >> archive creation since that will be empty? > Yes, this specific example will fail for that reason. > > But in general, running something like `cat - test.txt -` or `cat > /dev/stdin test.txt /dev/stdin` actually works if you run it > interactively in a terminal and hit `ctrl+d` to end the first input. This is however very error prone and requiring user input to proceed to the next archive does not help with user experience for using the client. So I'd rather not support such combined workflows at all. >> >> This should be checked and fail on command invocation instead. > This is not trivial because of symlinks: > > $ file /dev/stdin > /dev/stdin: symbolic link to /proc/self/fd/0 > > $ file /proc/self/fd/0 > /proc/self/fd/0: symbolic link to /dev/pts/5 > > $ echo | file /proc/self/fd/0 > /proc/self/fd/0: symbolic link to pipe:[317534] > > > I can think of two solutions: > > 1) Since we already read the metadata of all inputs at the start, we > could check that the inode number is unique for fifo files. If that is enough to identify the duplicate cases, then this seems to be the better option, yes. > 2) Simply check if /dev/stdin and/or /proc/self/fd/0 appear multiple > times as strings. In this case a warning might be better than an error. > > > I also just realized that my patch didn't actually work interactively, > because pts devices are not fifo pipes. Somehow I've only ever tested > this with my script or non-interactive pipes. > > I was able to fix this by allowing character devices, but initially I > got some really weird behavior. For example, a single ctrl+D was not > enough to end the stream and I also got errors about multiple smaller > chunks being sent. The problem was that FixedChunkStream continued to > call (try_)poll_next even after it got a None, which is not allowed. > >> >>> The only real advantage of "-" is that it could be implemented in >> a> platform independent way by reading from io::stdin(). That would >>> require a lot of changes though. >>> >>>> >>>>> +        } else { >>>>> +            filename >>>>> +        }; >>>>> + >>>>>            use std::os::unix::fs::FileTypeExt; >>>>>              let metadata = std::fs::metadata(&filename) >>>> >>