mirror of
https://github.com/nushell/nushell.git
synced 2025-04-30 08:04:25 +02:00
208ebeefab
5 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Ian Manske
|
3d008e2c4e
|
Error on non-zero exit statuses (#13515)
# Description This PR makes it so that non-zero exit codes and termination by signal are treated as a normal `ShellError`. Currently, these are silent errors. That is, if an external command fails, then it's code block is aborted, but the parent block can sometimes continue execution. E.g., see #8569 and this example: ```nushell [1 2] | each { ^false } ``` Before this would give: ``` ╭───┬──╮ │ 0 │ │ │ 1 │ │ ╰───┴──╯ ``` Now, this shows an error: ``` Error: nu:🐚:eval_block_with_input × Eval block failed with pipeline input ╭─[entry #1:1:2] 1 │ [1 2] | each { ^false } · ┬ · ╰── source value ╰──── Error: nu:🐚:non_zero_exit_code × External command had a non-zero exit code ╭─[entry #1:1:17] 1 │ [1 2] | each { ^false } · ──┬── · ╰── exited with code 1 ╰──── ``` This PR fixes #12874, fixes #5960, fixes #10856, and fixes #5347. This PR also partially addresses #10633 and #10624 (only the last command of a pipeline is currently checked). It looks like #8569 is already fixed, but this PR will make sure it is definitely fixed (fixes #8569). # User-Facing Changes - Non-zero exit codes and termination by signal now cause an error to be thrown. - The error record value passed to a `catch` block may now have an `exit_code` column containing the integer exit code if the error was due to an external command. - Adds new config values, `display_errors.exit_code` and `display_errors.termination_signal`, which determine whether an error message should be printed in the respective error cases. For non-interactive sessions, these are set to `true`, and for interactive sessions `display_errors.exit_code` is false (via the default config). # Tests Added a few tests. # After Submitting - Update docs and book. - Future work: - Error if other external commands besides the last in a pipeline exit with a non-zero exit code. Then, deprecate `do -c` since this will be the default behavior everywhere. - Add a better mechanism for exit codes and deprecate `$env.LAST_EXIT_CODE` (it's buggy). |
||
Devyn Cairns
|
91d44f15c1
|
Allow plugins to report their own version and store it in the registry (#12883)
# Description This allows plugins to report their version (and potentially other metadata in the future). The version is shown in `plugin list` and in `version`. The metadata is stored in the registry file, and reflects whatever was retrieved on `plugin add`, not necessarily the running binary. This can help you to diagnose if there's some kind of mismatch with what you expect. We could potentially use this functionality to show a warning or error if a plugin being run does not have the same version as what was in the cache file, suggesting `plugin add` be run again, but I haven't done that at this point. It is optional, and it requires the plugin author to make some code changes if they want to provide it, since I can't automatically determine the version of the calling crate or anything tricky like that to do it. Example: ``` > plugin list | select name version is_running pid ╭───┬────────────────┬─────────┬────────────┬─────╮ │ # │ name │ version │ is_running │ pid │ ├───┼────────────────┼─────────┼────────────┼─────┤ │ 0 │ example │ 0.93.1 │ false │ │ │ 1 │ gstat │ 0.93.1 │ false │ │ │ 2 │ inc │ 0.93.1 │ false │ │ │ 3 │ python_example │ 0.1.0 │ false │ │ ╰───┴────────────────┴─────────┴────────────┴─────╯ ``` cc @maxim-uvarov (he asked for it) # User-Facing Changes - `plugin list` gets a `version` column - `version` shows plugin versions when available - plugin authors *should* add `fn metadata()` to their `impl Plugin`, but don't have to # Tests + Formatting Tested the low level stuff and also the `plugin list` column. # After Submitting - [ ] update plugin guide docs - [ ] update plugin protocol docs (`Metadata` call & response) - [ ] update plugin template (`fn metadata()` should be easy) - [ ] release notes |
||
|
Ian Manske
|
6fd854ed9f
|
Replace ExternalStream with new ByteStream type (#12774)
# Description
This PR introduces a `ByteStream` type which is a `Read`-able stream of
bytes. Internally, it has an enum over three different byte stream
sources:
```rust
pub enum ByteStreamSource {
Read(Box<dyn Read + Send + 'static>),
File(File),
Child(ChildProcess),
}
```
This is in comparison to the current `RawStream` type, which is an
`Iterator<Item = Vec<u8>>` and has to allocate for each read chunk.
Currently, `PipelineData::ExternalStream` serves a weird dual role where
it is either external command output or a wrapper around `RawStream`.
`ByteStream` makes this distinction more clear (via `ByteStreamSource`)
and replaces `PipelineData::ExternalStream` in this PR:
```rust
pub enum PipelineData {
Empty,
Value(Value, Option<PipelineMetadata>),
ListStream(ListStream, Option<PipelineMetadata>),
ByteStream(ByteStream, Option<PipelineMetadata>),
}
```
The PR is relatively large, but a decent amount of it is just repetitive
changes.
This PR fixes #7017, fixes #10763, and fixes #12369.
This PR also improves performance when piping external commands. Nushell
should, in most cases, have competitive pipeline throughput compared to,
e.g., bash.
| Command | Before (MB/s) | After (MB/s) | Bash (MB/s) |
| -------------------------------------------------- | -------------:|
------------:| -----------:|
| `throughput \| rg 'x'` | 3059 | 3744 | 3739 |
| `throughput \| nu --testbin relay o> /dev/null` | 3508 | 8087 | 8136 |
# User-Facing Changes
- This is a breaking change for the plugin communication protocol,
because the `ExternalStreamInfo` was replaced with `ByteStreamInfo`.
Plugins now only have to deal with a single input stream, as opposed to
the previous three streams: stdout, stderr, and exit code.
- The output of `describe` has been changed for external/byte streams.
- Temporary breaking change: `bytes starts-with` no longer works with
byte streams. This is to keep the PR smaller, and `bytes ends-with`
already does not work on byte streams.
- If a process core dumped, then instead of having a `Value::Error` in
the `exit_code` column of the output returned from `complete`, it now is
a `Value::Int` with the negation of the signal number.
# After Submitting
- Update docs and book as necessary
- Release notes (e.g., plugin protocol changes)
- Adapt/convert commands to work with byte streams (high priority is
`str length`, `bytes starts-with`, and maybe `bytes ends-with`).
- Refactor the `tee` code, Devyn has already done some work on this.
---------
Co-authored-by: Devyn Cairns <devyn.cairns@gmail.com>
|
||
|
Devyn Cairns
|
ad6deadf24
|
Flush on every plugin Data message (#12728)
# Description This helps to ensure data produced on a stream is immediately available to the consumer of the stream. The BufWriter introduced for performance reasons in 0.93 exposed the behavior that data messages wouldn't make it to the other side until they filled the buffer in @cablehead's [`nu_plugin_from_sse`](https://github.com/cablehead/nu_plugin_from_sse). I had originally not flushed on every `Data` message because I figured that it isn't really critical that the other side sees those messages immediately, since they're not used for control and they are flushed when waiting for acknowledgement or when the buffer is too full anyway. Increasing the amount of data that can be sent with a single underlying write increases performance, but this interferes with some plugins that want to use streams in a more real-time way. In the future I would like to make this configurable, maybe even per-command, so that a command can decide what the priority is. But for now I think this is reasonable. In the worst case, this decreases performance by about 40%, when sending very small values (just numbers). But for larger values, this PR actually increases performance by about 20%, because I've increased the buffer size about 2x to 16,384 bytes. The previous value of 8,192 bytes was too small to fit a full buffer coming from an external command, so doubling it makes sense, and now a write of a buffer from an external command can be done in exactly one write call, which I think makes sense. I'm doing this at the same time because flushing each data message would make it very likely that each individual data message from an external stream would require exactly two writes rather than approximately one (amortized). Again, hopefully the tradeoff isn't too bad, and if it is I'll just make it configurable. # User-Facing Changes - Performance of plugin streams will be a bit different - Plugins that expect to send streams in real-time will work again # Tests + Formatting - 🟢 `toolkit fmt` - 🟢 `toolkit clippy` - 🟢 `toolkit test` - 🟢 `toolkit test stdlib` |
||
|
Devyn Cairns
|
0c4d5330ee
|
Split the plugin crate (#12563)
# Description This breaks `nu-plugin` up into four crates: - `nu-plugin-protocol`: just the type definitions for the protocol, no I/O. If someone wanted to wire up something more bare metal, maybe for async I/O, they could use this. - `nu-plugin-core`: the shared stuff between engine/plugin. Less stable interface. - `nu-plugin-engine`: everything required for the engine to talk to plugins. Less stable interface. - `nu-plugin`: everything required for the plugin to talk to the engine, what plugin developers use. Should be the most stable interface. No changes are made to the interface exposed by `nu-plugin` - it should all still be there. Re-exports from `nu-plugin-protocol` or `nu-plugin-core` are used as required. Plugins shouldn't ever have to use those crates directly. This should be somewhat faster to compile as `nu-plugin-engine` and `nu-plugin` can compile in parallel, and the engine doesn't need `nu-plugin` and plugins don't need `nu-plugin-engine` (except for test support), so that should reduce what needs to be compiled too. The only significant change here other than splitting stuff up was to break the `source` out of `PluginCustomValue` and create a new `PluginCustomValueWithSource` type that contains that instead. One bonus of that is we get rid of the option and it's now more type-safe, but it also means that the logic for that stuff (actually running the plugin for custom value ops) can live entirely within the `nu-plugin-engine` crate. # User-Facing Changes - New crates. - Added `local-socket` feature for `nu` to try to make it possible to compile without that support if needed. # Tests + Formatting - 🟢 `toolkit fmt` - 🟢 `toolkit clippy` - 🟢 `toolkit test` - 🟢 `toolkit test stdlib` |