feat: add optional bundling of coreutils builtins behind experimental feature flag

[cataggar]

This is a first pass at adding the coreutils as builtins per the suggestion #990 (comment) .

cargo install --locked brush-shell --features coreutils-builtins

brush.exe-0.4$ help
brush version 0.3.0 (git:4db4716) - https://github.com/reubeno/brush

The following commands are implemented as shell built-ins:
   .                      factor                 set
   :                      false                  sha1sum
   [                      fc                     sha224sum
   alias                  fg                     sha256sum
   arch                   fmt                    sha384sum
   b2sum                  fold                   sha512sum
   base32                 getopts                shift
   base64                 hash                   shopt
   basename               head                   shred
   basenc                 help                   shuf
   bg                     history                sleep
   bind                   hostname               sort
   break                  jobs                   source
   brushctl               join                   split
   brushinfo              let                    sum
   builtin                link                   sync
   caller                 ln                     tac
   cat                    local                  tail
   cd                     logout                 tee
   cksum                  ls                     test
   comm                   mapfile                times
   command                md5sum                 touch
   compgen                mkdir                  tr
   complete               mktemp                 trap
   compopt                more                   true
   continue               mv                     truncate
   cp                     nl                     tsort
   csplit                 nproc                  type
   cut                    numfmt                 typeset
   date                   od                     uecho
   dd                     paste                  ufalse
   declare                popd                   unalias
   df                     pr                     uname
   dir                    printenv               unexpand
   dircolors              printf                 uniq
   dirname                ptx                    unlink
   dirs                   pushd                  unset
   disown                 pwd                    uprintf
   du                     read                   upwd
   echo                   readarray              utest
   enable                 readlink               utrue
   env                    readonly               vdir
   eval                   realpath               wait
   exit                   return                 wc
   expand                 rm                     whoami
   export                 rmdir                  yes
   expr                   seq
brush.exe-0.4$ uname
Windows_NT

[github-actions]

Public API changes for crate: brush-core

Added items

+pub use brush_core::builtins::BoxFuture
+pub brush_core::commands::SimpleCommand::argv0: core::option::Option<alloc::string::String>
+pub fn brush_core::Shell<SE>::register_builtin_if_unset<S: core::convert::Into<alloc::string::String>>(&mut self, name: S, registration: brush_core::builtins::Registration<SE>)

Public API changes for crate: brush-shell

Added items

+pub mod brush_shell::bundled
+pub const brush_shell::bundled::DISPATCH_FLAG: &str
+pub fn brush_shell::bundled::install(commands: std::collections::hash::map::HashMap<alloc::string::String, brush_shell::bundled::BundledFn>)
+pub fn brush_shell::bundled::install_default_providers()
+pub fn brush_shell::bundled::maybe_dispatch() -> core::option::Option<i32>
+pub fn brush_shell::bundled::register_shims<SE: brush_core::extensions::ShellExtensions>(shell: &mut brush_core::shell::Shell<SE>)
+pub fn brush_shell::bundled::registry() -> core::option::Option<&'static std::collections::hash::map::HashMap<alloc::string::String, brush_shell::bundled::BundledFn>>
+pub type brush_shell::bundled::BundledFn = fn(args: alloc::vec::Vec<std::ffi::os_str::OsString>) -> i32

Performance Benchmark Report

Benchmark name	Baseline (μs)	Test/PR (μs)	Delta (μs)	Delta %
clone_shell_object	17.48 μs	17.64 μs	0.16 μs	⚪ Unchanged
eval_arithmetic	0.15 μs	0.15 μs	0.00 μs	⚪ Unchanged
expand_one_string	1.49 μs	1.52 μs	0.03 μs	⚪ Unchanged
for_loop	29.30 μs	28.85 μs	-0.45 μs	⚪ Unchanged
full_peg_complex	57.60 μs	57.80 μs	0.20 μs	⚪ Unchanged
full_peg_for_loop	6.14 μs	6.20 μs	0.06 μs	⚪ Unchanged
full_peg_nested_expansions	16.17 μs	16.25 μs	0.08 μs	⚪ Unchanged
full_peg_pipeline	4.16 μs	4.22 μs	0.06 μs	🟠 +1.42%
full_peg_simple	1.79 μs	1.80 μs	0.01 μs	⚪ Unchanged
function_call	3.17 μs	3.12 μs	-0.05 μs	⚪ Unchanged
instantiate_shell	53.91 μs	53.51 μs	-0.40 μs	⚪ Unchanged
instantiate_shell_with_init_scripts	25157.38 μs	25185.76 μs	28.38 μs	⚪ Unchanged
parse_peg_bash_completion	2061.61 μs	2059.01 μs	-2.61 μs	⚪ Unchanged
parse_peg_complex	20.08 μs	19.86 μs	-0.22 μs	🟢 -1.10%
parse_peg_for_loop	1.97 μs	2.08 μs	0.11 μs	🟠 +5.37%
parse_peg_pipeline	2.13 μs	2.10 μs	-0.02 μs	⚪ Unchanged
parse_peg_simple	1.09 μs	1.12 μs	0.03 μs	🟠 +2.85%
run_echo_builtin_command	16.22 μs	16.62 μs	0.40 μs	🟠 +2.46%
tokenize_sample_script	3.33 μs	3.45 μs	0.12 μs	⚪ Unchanged

Code Coverage Report: Only Changed Files listed

Package	Base Coverage	New Coverage	Difference
brush-core/src/commands.rs	🟢 91.57%	🟢 91.65%	🟢 0.08%
brush-core/src/jobs.rs	🔴 49.55%	🟠 50.91%	🟢 1.36%
brush-core/src/results.rs	🟢 80.16%	🟢 83.33%	🟢 3.17%
brush-core/src/shell/builtin_registry.rs	🔴 46.15%	🔴 30%	🔴 -16.15%
brush-shell/src/bundled.rs	🔴 0%	🔴 18.75%	🟢 18.75%
brush-shell/src/entry.rs	🟢 90.43%	🟢 90.31%	🔴 -0.12%
Overall Coverage	🟢 75.5%	🟢 75.23%	🔴 -0.27%

Minimum allowed coverage is 70%, this run produced 75.23%

Test Summary: bash-completion test suite

Outcome	Count	Percentage
✅ Pass	1582	75.01
❗️ Error	18	0.85
❌ Fail	155	7.35
⏩ Skip	339	16.07
❎ Expected Fail	13	0.62
✔️ Unexpected Pass	2	0.09
📊 Total	2109	100.00

[reubeno]

Very cool! Thanks for putting this together 😄 With all these commands present, how well does the shell seem to work on Windows? (I'm assuming from your output above that's where you've tested this?)

I skimmed through the changes and they look quite reasonable. (I wish there were a way to dynamically get the command short-description from uutils, but I don't think it's a deal-breaker.) I'll look at the macros a bit more closely later this weekend.

I've thought about it a bit more since and one thing we'll need to keep in mind is that much of brush's shell state isn't reflected back to the hosting process's OS state. As specific examples: when a variable is set in brush it's not observable via OS env APIs; also, file redirections aren't actually reflected back to the real host file descriptor table or OS-level notions of stdout/stderr/stdin. That's a benefit for OS portability and for being able to spawn subshells in-process, but I could foresee some issues with coreutils commands not being able to "see" those changes.

With that said, this seems interesting to enough to keep pursuing. I'm inclined to bring these in under an "experimental" qualifier, as we've done with some of the other speculative features/extensions. (That just might entail prefixing the feature in brush-shell with the experimental- prefix. We should also explore the uutils code base to see if there might be some way of overriding env and/or stdio access. At worst, we could explore other creative options that entail running the coreutils commands in a child process despite being available in-binary.

[cataggar]

I created a minimal Azure Linux Dockerfile to try it out and it works well.
cataggar#1

[reubeno]

@cataggar I'd like to move toward getting this PR merged but would like to qualify the coreutils integration as "experimental" -- particularly because of known challenges with stdio redirection.

Are you up for making some adjustments? Alternatively, I'm open to making some of the adjustments myself if you're okay with me pushing changes to your PR branch. In particular here are the items that we'll need to sort out:

• Rename feature coreutils-builtins to experimental-coreutils-builtins to indicate its experimental status.
• Get PR checks passing (I see the format checks are failing); cargo xtask ci pre-commit can be used locally to help catch some of these.
• Rebase on latest main.
• Ensure that the license file is symlink'd under the new crate dir like it is for others

[oech3]

You should exclude unix specificsome progs from windows and wasm* target. (cygwin target is OK).

[reubeno]

You should exclude unix specificsome progs from windows and wasm* target. (cygwin target is OK).

@oech3 can you share more about what you're thinking here? Or a concrete example?

[oech3]

https://github.com/uutils/coreutils/blob/de5e4f54f224a25875bdc207cd57f035a58fbe0a/Cargo.toml#L174-L225
You cannot build other utils for wasip1.

[oech3]

uutils has policy to avoid process::exit for this usecase. But still used. Does it exit brush itself?

[reubeno]

uutils has policy to avoid process::exit for this usecase. But still used. Does it exit brush itself?

Yes, I'd expect it would. It's for reasons like this that I'd consider this integration experimental. The input/output redirection issues I mention above are another class of challenges.

[casey]

I'm the author of just, and I just (ha ha, never gets old) stumbled across this PR and the original conversation in #990.

There's been a longstanding desire to embed a shell and common utilities in just in order to make it truly cross platform. I was researching what options are available and found this issue.

The combination of brush and uutils/coreutils as a Rust-native busybox is really appealing!

There are definitely potential issues. The binary would be absolutely gargantuan, and just has very strict backwards compatibility requirements. But this could be sidestepped by having users install the brush + coreutils binary separately and use it via set shell := ['brush'] in their justfile.

Anyways, just wanted to say that I think this is a super cool direction to explore, and I'm very interested in how things develop.

[reubeno]

I'm the author of just, and I just (ha ha, never gets old) stumbled across this PR and the original conversation in #990.

There's been a longstanding desire to embed a shell and common utilities in just in order to make it truly cross platform. I was researching what options are available and found this issue.

The combination of brush and uutils/coreutils as a Rust-native busybox is really appealing!

There are definitely potential issues. The binary would be absolutely gargantuan, and just has very strict backwards compatibility requirements. But this could be sidestepped by having users install the brush + coreutils binary separately and use it via set shell := ['brush'] in their justfile.

Anyways, just wanted to say that I think this is a super cool direction to explore, and I'm very interested in how things develop.

Thanks for reaching out, @casey! (Big fan of just by the way 😄. We don't use it for brush currently but I've used it for other projects.)

I'm also interested in the idea of a(n emeddable?) variant build of brush that's more intentionally busybox-ish and self-contained. (For unix-y platforms I'm hesitant for the default brush binary to embed coreutils; I expect that users typically want to use their system installation of coreutils instead. We may choose otherwise on other platforms like Windows, though, on which those standard utilities generally aren't present.)

Do you have a point of view on which shell functionality / common utilities you and your users would want to see just have access to? For example, there's a fair bit of logic in shells like bash and brush to support interactive use, and which typically isn't required for scripts/automation. We've tried to keep interactive logic separatable in brush's core crates; I've long wanted to build a version of brush that's limited to script execution, and which excludes all the code + dependencies only required for interactive usage.

[casey]

I'm also interested in the idea of a(n emeddable?) variant build of brush that's more intentionally busybox-ish and self-contained. (For unix-y platforms I'm hesitant for the default brush binary to embed coreutils; I expect that users typically want to use their system installation of coreutils instead. We may choose otherwise on other platforms like Windows, though, on which those standard utilities generally aren't present.)

I think it makes sense for those to be different binaries. brush with no userland, and brushbox which is brush plus coreutils.

Do you have a point of view on which shell functionality / common utilities you and your users would want to see just have access to? For example, there's a fair bit of logic in shells like bash and brush to support interactive use, and which typically isn't required for scripts/automation. We've tried to keep interactive logic separatable in brush's core crates; I've long wanted to build a version of brush that's limited to script execution, and which excludes all the code + dependencies only required for interactive usage.

If it was embedded in just, than it would be nice to get the binary size down, so interactive logic could be stripped out. However, that's only to get the binary size down, and I think that would only be really important if brush were embedded. In the end it's probably more flexible to have a separate brushbox binary, in which case it's less important to get the binary size down, and users would probably want the interactive features in the standalone binary.

Then you would just do:

set shell := ["brushbox", "brush"]

In your justfile, or whatever the command line would be.

[casey]

Oh, and sorry, I didn't fully answer this question:

Do you have a point of view on which shell functionality / common utilities you and your users would want to see just have access to?

I'm not actually sure about which utilities users would want. I think there are probably some obscure ones that don't get used frequently, but I'm not sure how to think of the tradeoff between a larger binary with all utilities, and a smaller binary without some obscure utilities.

[oech3]

Can we bind uutils/date to #677 ?

[Elsie19]

Suggested change

	let mut m = HashMap::<String, builtins::Registration<SE>>::new();
	let mut m = HashMap::<&'static str, builtins::Registration<SE>>::new();

Is there no reason this could be used?