`cp`: add progress bar #3903

tertsdiepraam · 2022-09-05T09:05:34Z

Based on the recent discord discussion around #1455, I figured I would give it a go to see what the feature would look like. This is mostly meant to spark some discussion about what this should look like and how it should behave.

Details of this implementation:

indicatif is used to draw the progress bar.
Before copying all the files, we have to recursively read all the files to get the total size (like du)
With very large directories, this means a bit of lag before we can actually show the bar. The entire application is also slowed down noticeably, though I haven't done any benchmarks.
I ignored the size of the directories themselves as it seems insignificant in most cases.
We update the progress bar with each copied file.
This behaviour might need to be tweaked when using cp with the symlink options as that wouldn't actually copy that many bytes.

If we actually want to land this, the progress bar (and accompanying performance hit) would of course need to be opt-in. I'm leaning towards adding both a compile-time feature gate and a run-time flag to enable this.

This is what it looks like:

sylvestre · 2022-09-05T09:14:56Z

it is magic how small it is :)

tertsdiepraam · 2022-09-05T09:53:45Z

Indeed! indicatif is amazing :)

sylvestre · 2022-09-05T10:04:48Z

what about adding an option --progress ?

tertsdiepraam · 2022-09-05T11:02:23Z

Yeah that makes sense. I added -g/--progress. I'm not sure what the g stands for, but it's used in advcpmv, which seems to be the most notable "prior art" for this. xcp also has a progress bar, but only has a flag to disable the bar (--no-progress).

I think a compile-time flag might be unnecessary, because the bloat by indicatif seems to be fairly minimal (about 70Kib, which is 1.5% of cp), so it might not be necessary.

sylvestre · 2022-09-05T12:17:04Z

Maybe we should have a page in which we list all the stuff that we added compared to GNU's?

for example, I think we have more hashing functions

tertsdiepraam · 2022-09-05T18:22:44Z

I'll add something like that to the docs in this PR. I'll also add the bar to mv and then I'll mark it ready for review. In using it for testing, I already found myself agreeing with the issue author that this is very useful.

We can also bikeshed the layout and appearance of the bar itself. I think it makes sense to put cp: as a prefix like we do with warnings and errors so the user can see what command in a script is showing the bar. I think I'll also add the ETA.

tertsdiepraam · 2022-09-05T23:58:10Z

I changed my mind about mv, because it just renames files, it's usually very fast anyway, so doesn't really need a progress bar.

With the documentation on the extensions over GNU, I think this is ready.

sylvestre · 2022-09-06T07:21:10Z

src/uu/cp/src/cp.rs

@@ -938,7 +952,22 @@ fn copy(sources: &[Source], target: &TargetSlice, options: &Options) -> CopyResu
    let mut non_fatal_errors = false;
    let mut seen_sources = HashSet::with_capacity(sources.len());
    let mut symlinked_files = HashSet::new();
-    for source in sources {
+


needs some rustfmt ;)

sylvestre · 2022-10-03T18:25:07Z

sorry, I think I failed as fixing the conflicts :(

sylvestre · 2022-10-15T18:27:17Z

small error:


error[B004]: found 2 duplicate entries for crate 'terminal_size'
    ┌─ /github/workspace/Cargo.lock:201:1
    │  
201 │ ╭ terminal_size 0.1.17 registry+https://github.com/rust-lang/crates.io-index
202 │ │ terminal_size 0.2.1 registry+https://github.com/rust-lang/crates.io-index
    │ ╰─────────────────────────────────────────────────────────────────────────^ lock entries

tertsdiepraam · 2022-10-15T19:08:18Z

Welp, looks like it's blocked. So I think we'll have to make an exception in deny.

github-actions · 2022-10-17T09:58:19Z

GNU testsuite comparison:

Congrats! The gnu test tests/rm/rm2 is no longer failing!
GNU test failed: tests/misc/tee. tests/misc/tee is passing on 'main'. Maybe you have to rebase?

sylvestre · 2022-10-17T14:23:53Z

docs/src/extensions.md

@@ -0,0 +1,31 @@
+# Extensions over GNU


Love that file :)

src/uu/cp/src/cp.rs

sylvestre · 2022-10-22T08:26:27Z

needs rebasing again, sorry :(

Once done, if the CI is green, please land it :)

Adds the `-g` and `--progress` flags to enable a progress bar via indicatif.

docs: fix typos in extensions page

github-actions · 2022-11-11T08:20:55Z

GNU testsuite comparison:

GNU test failed: tests/misc/timeout. tests/misc/timeout is passing on 'main'. Maybe you have to rebase?

sylvestre · 2022-11-11T08:25:46Z

sorry it took that long :)

doronbehar · 2022-11-11T09:10:27Z

Would it be possible to add progress bar for mv as well 🙏?

tertsdiepraam · 2022-11-11T11:32:21Z

@doronbehar I found that mv is generally quite fast because it essentially just renames the files instead of having to read them. If you have a test case where it's slow enough to need a progress bar I could take a look.

sylvestre · 2022-11-11T12:33:13Z

Maybe mv can be slow when moving between disks. (ex sdd - > USB)

doronbehar · 2022-11-11T15:28:33Z

Maybe mv can be slow when moving between disks. (ex sdd - > USB)

That's exactly the use case I was thinking about.

tertsdiepraam · 2022-11-11T15:35:09Z

That makes sense! I'll open a new issue for it.

tertsdiepraam changed the title ~~cp: add progress bar~~ cp: add progress bar Sep 5, 2022

tertsdiepraam force-pushed the cp-progress branch 2 times, most recently from 4ae15d0 to 2ccf3c8 Compare September 5, 2022 11:20

tertsdiepraam marked this pull request as ready for review September 5, 2022 23:56

sylvestre reviewed Sep 6, 2022

View reviewed changes

tertsdiepraam force-pushed the cp-progress branch 4 times, most recently from 5ffcec2 to b75dc0a Compare September 10, 2022 10:04

tertsdiepraam mentioned this pull request Sep 10, 2022

uutils: Non-standard output and options? #2837

Closed

tertsdiepraam force-pushed the cp-progress branch from 8bc5a97 to 6ff7684 Compare October 15, 2022 17:51

tertsdiepraam requested a review from sylvestre October 17, 2022 10:42

sylvestre reviewed Oct 17, 2022

View reviewed changes

docs/src/extensions.md

@@ -0,0 +1,31 @@

# Extensions over GNU

Copy link

Contributor

sylvestre Oct 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love that file :)

sylvestre reviewed Oct 17, 2022

View reviewed changes

src/uu/cp/src/cp.rs Show resolved Hide resolved

tertsdiepraam force-pushed the cp-progress branch from 3acf5f3 to 756223d Compare October 22, 2022 09:02

tertsdiepraam added 3 commits November 11, 2022 00:44

cp: add progress bar

21e691c

Adds the `-g` and `--progress` flags to enable a progress bar via indicatif.

docs: document extensions over GNU

4237762

docs: fix typos in extensions page

add terminal_size to cargo deny exceptions

b821d21

tertsdiepraam force-pushed the cp-progress branch from 756223d to b821d21 Compare November 10, 2022 23:44

Fix a clippy warning

abfccb3

sylvestre merged commit 81a2abc into uutils:main Nov 11, 2022

tertsdiepraam mentioned this pull request Nov 11, 2022

mv progress bar #4128

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`cp`: add progress bar #3903

`cp`: add progress bar #3903

tertsdiepraam commented Sep 5, 2022 •

edited

Loading

sylvestre commented Sep 5, 2022

tertsdiepraam commented Sep 5, 2022

sylvestre commented Sep 5, 2022

tertsdiepraam commented Sep 5, 2022

sylvestre commented Sep 5, 2022

tertsdiepraam commented Sep 5, 2022

tertsdiepraam commented Sep 5, 2022

sylvestre Sep 6, 2022

sylvestre commented Oct 3, 2022

sylvestre commented Oct 15, 2022

tertsdiepraam commented Oct 15, 2022

github-actions bot commented Oct 17, 2022

sylvestre Oct 17, 2022

sylvestre commented Oct 22, 2022

github-actions bot commented Nov 11, 2022

sylvestre commented Nov 11, 2022

doronbehar commented Nov 11, 2022

tertsdiepraam commented Nov 11, 2022

sylvestre commented Nov 11, 2022

doronbehar commented Nov 11, 2022

tertsdiepraam commented Nov 11, 2022

cp: add progress bar #3903

cp: add progress bar #3903

Conversation

tertsdiepraam commented Sep 5, 2022 • edited Loading

sylvestre commented Sep 5, 2022

tertsdiepraam commented Sep 5, 2022

sylvestre commented Sep 5, 2022

tertsdiepraam commented Sep 5, 2022

sylvestre commented Sep 5, 2022

tertsdiepraam commented Sep 5, 2022

tertsdiepraam commented Sep 5, 2022

sylvestre Sep 6, 2022

Choose a reason for hiding this comment

sylvestre commented Oct 3, 2022

sylvestre commented Oct 15, 2022

tertsdiepraam commented Oct 15, 2022

github-actions bot commented Oct 17, 2022

sylvestre Oct 17, 2022

Choose a reason for hiding this comment

sylvestre commented Oct 22, 2022

github-actions bot commented Nov 11, 2022

sylvestre commented Nov 11, 2022

doronbehar commented Nov 11, 2022

tertsdiepraam commented Nov 11, 2022

sylvestre commented Nov 11, 2022

doronbehar commented Nov 11, 2022

tertsdiepraam commented Nov 11, 2022

`cp`: add progress bar #3903

`cp`: add progress bar #3903

tertsdiepraam commented Sep 5, 2022 •

edited

Loading