Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How to redirect stderr but continue to pipe stdout? #10271

Closed
bobhy opened this issue Sep 8, 2023 · 3 comments · Fixed by #10851
Closed

How to redirect stderr but continue to pipe stdout? #10271

bobhy opened this issue Sep 8, 2023 · 3 comments · Fixed by #10851
Labels
question the issue author asks something redirection-pipe All related to redirection to files or more complex pipelines with STDERR
Milestone

Comments

@bobhy
Copy link
Contributor

bobhy commented Sep 8, 2023

Question

I'm having trouble invoking an external command and redirecting stderr (to /dev/null) while piping stdout to the next step.
Maybe there's some other kind of redirection I should be using? Maybe it's a bug?

Sample:

> sudo apt update  | filter {|line| $line | str contains --ignore-case "can be upgraded" } 

WARNING: apt does not have a stable CLI interface. Use with caution in scripts.

╭───┬──────────────────────────────────────────────────────────────────────╮
│ 0 │ 6 packages can be upgraded. Run 'apt list --upgradable' to see them. │
│   │                                                                      │
╰───┴──────────────────────────────────────────────────────────────────────╯

Almost what I want, except for the ugly WARNING that didn't flow into the pipe.

(try to) Fix it by redirecting stderr:

> sudo apt update  err> /dev/null | filter {|line| $line | str contains --ignore-case "can be upgraded" } 
Hit:1 http://us.archive.ubuntu.com/ubuntu lunar InRelease
Hit:2 http://us.archive.ubuntu.com/ubuntu lunar-updates InRelease                                                      
Hit:3 http://packages.microsoft.com/repos/code stable InRelease                                                        
Hit:4 http://us.archive.ubuntu.com/ubuntu lunar-backports InRelease                                                    
Hit:5 https://packages.microsoft.com/repos/edge stable InRelease                                                       
Hit:6 http://security.ubuntu.com/ubuntu lunar-security InRelease           
Reading package lists... Done
Building dependency tree... Done
Reading state information... Done
6 packages can be upgraded. Run 'apt list --upgradable' to see them.

Does suppress the WARNING, but now filter isn't getting any input...

At first I though this had something to do with sudo, but you can repro with nonprivileged external dir command

> dir -l .  argle  | filter {|x| $x | str contains Music}
dir: cannot access 'argle': No such file or directory
╭───┬────────────────────────────────────────────────────────╮
│ 0 │ .:                                                     │
│   │ total 89                                               │
│   │ drwxr-xr-x 2 bobhy bobhy   2 Aug 18 21:12 Desktop      │
│   │ drwxr-xr-x 2 bobhy bobhy   3 Sep  6 14:20 Documents    │
│   │ drwxr-xr-x 2 bobhy bobhy  25 Sep  6 23:47 Downloads    │
│   │ drwxr-xr-x 2 bobhy bobhy   2 Aug 18 21:12 Music        │
│   │ drwxr-xr-x 3 bobhy bobhy   3 Aug 25 13:18 Pictures     │
│   │ drwxr-xr-x 2 bobhy bobhy   2 Aug 18 21:12 Public       │
│   │                                                        │
╰───┴────────────────────────────────────────────────────────╯

The missing directory error is reported to stderr as in the previous case.

I don't quite get why each newline-delimited line of output wasn't broken into a separate line item, like it seems to have done with sudo apt, but never mind: the key thing is filter returned a list with one element, as desired.

> dir -l .  argle err> /dev/null  | filter {|x| $x | str contains Music}
.:
total 89
drwxr-xr-x 2 bobhy bobhy   2 Aug 18 21:12 Desktop
drwxr-xr-x 2 bobhy bobhy   3 Sep  6 14:20 Documents
drwxr-xr-x 2 bobhy bobhy  25 Sep  6 23:47 Downloads
drwxr-xr-x 2 bobhy bobhy   2 Aug 18 21:12 Music
drwxr-xr-x 3 bobhy bobhy   3 Aug 25 13:18 Pictures
drwxr-xr-x 2 bobhy bobhy   2 Aug 18 21:12 Public

Note that the dir output isn't being piped into filter,at all, apparently.

Additional context and details

No response

@bobhy bobhy added the question the issue author asks something label Sep 8, 2023
@fdncred
Copy link
Collaborator

fdncred commented Sep 8, 2023

I can only think of 3 things that I'd try.

  1. put the redirection at the end just to see what happens like this sudo apt update | filter {|line| $line | str contains --ignore-case "can be upgraded" } err> /dev/null. Seems illogical, but I'm curious.
  2. use complete like do -i {sudo apt update | filter ...} | complete | get stdout, which isn't the same but may work.
  3. This is what is used in atuin run-external --redirect-stderr atuin search "--interactive" | complete | $in.stderr

I agree with you that sudo apt update err> /dev/null | filter {|line| $line | str contains --ignore-case "can be upgraded" } seems like it should work. I'm not sure what's going on.

@WindSoilder, any ideas about this?

@horasal
Copy link
Contributor

horasal commented Sep 8, 2023

I'm not sure but a quick look through eval.rs, it may be:

  • if any redirection exists, all the output will just be ignored and not piped.
  • apt is tiggered through run-external but it does not always cache stdout to pipeline

workround can be use run-external with --redirect-stderr and --redirect-stdout:

❯ cat test.sh
#!/bin/bash

echo this is stderr message >> /dev/stderr
echo stdout message
echo stdout message line 2

❯ run-external ./test.sh --redirect-stdout --redirect-stderr  | lines
╭───┬───────────────────────╮
│ 0 │ stdout message        │
│ 1 │ stdout message line 2 │
╰───┴───────────────────────╯

the following quick patch makes err> pipe stdout (but it breaks out> :(

diff --git a/crates/nu-engine/src/eval.rs b/crates/nu-engine/src/eval.rs
index 462dbb347..cea130517 100644
--- a/crates/nu-engine/src/eval.rs
+++ b/crates/nu-engine/src/eval.rs
@@ -203,7 +203,6 @@ fn eval_external(

     match redirect_target {
         RedirectTarget::Piped(redirect_stdout, redirect_stderr) => {
-            if redirect_stdout {
                 call.add_named((
                     Spanned {
                         item: "redirect-stdout".into(),
@@ -211,10 +210,7 @@ fn eval_external(
                     },
                     None,
                     None,
-                ))
-            }
-
-            if redirect_stderr {
+                ));
                 call.add_named((
                     Spanned {
                         item: "redirect-stderr".into(),
@@ -223,7 +219,6 @@ fn eval_external(
                     None,
                     None,
                 ))
-            }
         }
         RedirectTarget::CombinedPipe => call.add_named((
             Spanned {
@@ -746,10 +741,11 @@ pub fn eval_element_with_input(
                     PipelineData::ExternalStream { exit_code, .. } => exit_code.take(),
                     _ => None,
                 };
-                let input = match (redirection, input) {
+                let (input_to_save, output_stdout, output_stderr) = match (redirection, input) {
                     (
                         Redirection::Stderr,
                         PipelineData::ExternalStream {
+                            stdout,
                             stderr,
                             exit_code,
                             span,
@@ -757,15 +753,34 @@ pub fn eval_element_with_input(
                             trim_end_newline,
                             ..
                         },
-                    ) => PipelineData::ExternalStream {
+                    ) => (PipelineData::ExternalStream {
                         stdout: stderr,
                         stderr: None,
                         exit_code,
                         span,
                         metadata,
                         trim_end_newline,
-                    },
-                    (_, input) => input,
+                    }, stdout, None),
+                    (
+                        Redirection::Stdout,
+                        PipelineData::ExternalStream {
+                            stdout,
+                            stderr,
+                            exit_code,
+                            span,
+                            metadata,
+                            trim_end_newline,
+                            ..
+                        },
+                    ) => (PipelineData::ExternalStream {
+                        stdout: stdout,
+                        stderr: None,
+                        exit_code,
+                        span,
+                        metadata,
+                        trim_end_newline,
+                    }, None, stderr),
+                    (_, input) => (input, None, None),
                 };

                 if let Some(save_command) = engine_state.find_decl(b"save", &[]) {
@@ -798,7 +813,7 @@ pub fn eval_element_with_input(
                             redirect_stderr: false,
                             parser_info: HashMap::new(),
                         },
-                        input,
+                        input_to_save,
                     )
                     .map(|_| {
                         // save is internal command, normally it exists with non-ExternalStream
@@ -806,8 +821,8 @@ pub fn eval_element_with_input(
                         // So nu handles exit_code correctly
                         (
                             PipelineData::ExternalStream {
-                                stdout: None,
-                                stderr: None,
+                                stdout: output_stdout,
+                                stderr: output_stderr,
                                 exit_code,
                                 span: *span,
                                 metadata: None,

@sholderbach sholderbach added the redirection-pipe All related to redirection to files or more complex pipelines with STDERR label Sep 8, 2023
@WindSoilder
Copy link
Collaborator

Sorry, I don't have much idea, I can try to look into this when I get time

WindSoilder added a commit that referenced this issue Nov 23, 2023
#10851)

# Description
Fixes: #10271

Given the following script:
```shell
# test.sh
echo aaaaa
echo bbbbb 1>&2
echo cc
```

This pr makes the following command possible:
```nushell
bash test.sh err> /dev/null | lines | each {|line| $line | str length}
```


## General idea behind the change:
When nushell redirect stderr message to external file
1. it take stdout of external stream, and pass this stream to next
command, so it won't block next pipeline command from running.
2. relative stderr stream are handled by `save` command

These two streams are handled separately, so we need to delegate a
thread to `save` command, or else we'll have a chance to hang nushell,
we have meet a similar before: #5625.

### One case to consider
What if we're failed to save to an external stream? (Like we don't have
a permission to save to a file)?
In this case nushell will just print a waning message, and don't stop
the following scripts from running.

# User-Facing Changes
## Before
```nushell
❯ bash test2.sh err> /dev/null | lines | each {|line| $line | str length}
aaaaa
cc
```

## After
```nushell
❯ bash test2.sh err> /dev/null | lines | each {|line| $line | str length}
╭───┬───╮
│ 0 │ 5 │
│ 1 │ 2 │
╰───┴───╯
```

BTY, after this pr, the following commands are impossible either, it's
important to make sure that the implementation doesn't introduce too
much costs:
```nushell
❯ echo a e> a.txt e> a.txt
Error:   × Can't make stderr redirection twice
   ╭─[entry #1:1:1]
 1 │ echo a e> a.txt e> a.txt
   ·                 ─┬
   ·                  ╰── try to remove one
   ╰────

❯ echo a o> a.txt o> a.txt
Error:   × Can't make stdout redirection twice
   ╭─[entry #2:1:1]
 1 │ echo a o> a.txt o> a.txt
   ·                 ─┬
   ·                  ╰── try to remove one
   ╰────
```
@hustcer hustcer added this to the v0.88.0 milestone Nov 27, 2023
hardfau1t pushed a commit to hardfau1t/nushell that referenced this issue Dec 14, 2023
nushell#10851)

# Description
Fixes: nushell#10271

Given the following script:
```shell
# test.sh
echo aaaaa
echo bbbbb 1>&2
echo cc
```

This pr makes the following command possible:
```nushell
bash test.sh err> /dev/null | lines | each {|line| $line | str length}
```


## General idea behind the change:
When nushell redirect stderr message to external file
1. it take stdout of external stream, and pass this stream to next
command, so it won't block next pipeline command from running.
2. relative stderr stream are handled by `save` command

These two streams are handled separately, so we need to delegate a
thread to `save` command, or else we'll have a chance to hang nushell,
we have meet a similar before: nushell#5625.

### One case to consider
What if we're failed to save to an external stream? (Like we don't have
a permission to save to a file)?
In this case nushell will just print a waning message, and don't stop
the following scripts from running.

# User-Facing Changes
## Before
```nushell
❯ bash test2.sh err> /dev/null | lines | each {|line| $line | str length}
aaaaa
cc
```

## After
```nushell
❯ bash test2.sh err> /dev/null | lines | each {|line| $line | str length}
╭───┬───╮
│ 0 │ 5 │
│ 1 │ 2 │
╰───┴───╯
```

BTY, after this pr, the following commands are impossible either, it's
important to make sure that the implementation doesn't introduce too
much costs:
```nushell
❯ echo a e> a.txt e> a.txt
Error:   × Can't make stderr redirection twice
   ╭─[entry #1:1:1]
 1 │ echo a e> a.txt e> a.txt
   ·                 ─┬
   ·                  ╰── try to remove one
   ╰────

❯ echo a o> a.txt o> a.txt
Error:   × Can't make stdout redirection twice
   ╭─[entry nushell#2:1:1]
 1 │ echo a o> a.txt o> a.txt
   ·                 ─┬
   ·                  ╰── try to remove one
   ╰────
```
dmatos2012 pushed a commit to dmatos2012/nushell that referenced this issue Feb 20, 2024
nushell#10851)

# Description
Fixes: nushell#10271

Given the following script:
```shell
# test.sh
echo aaaaa
echo bbbbb 1>&2
echo cc
```

This pr makes the following command possible:
```nushell
bash test.sh err> /dev/null | lines | each {|line| $line | str length}
```


## General idea behind the change:
When nushell redirect stderr message to external file
1. it take stdout of external stream, and pass this stream to next
command, so it won't block next pipeline command from running.
2. relative stderr stream are handled by `save` command

These two streams are handled separately, so we need to delegate a
thread to `save` command, or else we'll have a chance to hang nushell,
we have meet a similar before: nushell#5625.

### One case to consider
What if we're failed to save to an external stream? (Like we don't have
a permission to save to a file)?
In this case nushell will just print a waning message, and don't stop
the following scripts from running.

# User-Facing Changes
## Before
```nushell
❯ bash test2.sh err> /dev/null | lines | each {|line| $line | str length}
aaaaa
cc
```

## After
```nushell
❯ bash test2.sh err> /dev/null | lines | each {|line| $line | str length}
╭───┬───╮
│ 0 │ 5 │
│ 1 │ 2 │
╰───┴───╯
```

BTY, after this pr, the following commands are impossible either, it's
important to make sure that the implementation doesn't introduce too
much costs:
```nushell
❯ echo a e> a.txt e> a.txt
Error:   × Can't make stderr redirection twice
   ╭─[entry nushell#1:1:1]
 1 │ echo a e> a.txt e> a.txt
   ·                 ─┬
   ·                  ╰── try to remove one
   ╰────

❯ echo a o> a.txt o> a.txt
Error:   × Can't make stdout redirection twice
   ╭─[entry nushell#2:1:1]
 1 │ echo a o> a.txt o> a.txt
   ·                 ─┬
   ·                  ╰── try to remove one
   ╰────
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question the issue author asks something redirection-pipe All related to redirection to files or more complex pipelines with STDERR
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants