Fix progress bar for "get" command #1279

rht · 2015-05-22T09:43:21Z

Should look like this

go-ipfs (cleanup-progressbar) x ipfs get QmQv4YQNmRPuTTHs4AgBhKEFDdN7eQYeTbSmr8JVWVfury/sintel.mp4    ~/code/gocode/src/github.com/ipfs/go-ipfs
Saving file(s) to sintel.mp4
28.00 MB / 123.28 MB [=========================>----------------------------------------------------------------------------------------] 22.71 % 7m18s

@whyrusleeping I use NodeState instead of pbdata. Is this more efficient?

jbenet · 2015-05-22T10:10:24Z

core/commands/get.go

+		return nil, 0, err
+	}
+
+	length := uint64(ns.CumulativeSize)


i think the cumulative size will be the total ipfs size, no the total unixfs size. (meaning, the ipfs size has some overhead). Maybe this is fine. (and maybe this is why the add / cat progressbar sometimes goes past 100%...)

cc @whyrusleeping ?

afaik add progressbar doesn't use ipfs size, so it's fine. I don't know which one dagreader uses.

Oh it is more correct to use ipfs size? because of the >100%?

yeah, the ipfs size is the real amount of data moved. the unixfs size may be smaller, but could even be larger (say if i have 1000 links to the same, huge object). whatever we pick we should use it correctly (i.e. the >100% i think is because they're perhaps mixed)

jbenet · 2015-05-22T10:11:25Z

I'm very excited to have this merged-- thank you for picking this up @rht

whyrusleeping · 2015-05-22T15:48:17Z

I think using the CumulativeSize from the node stat should work well. its a better representation of the data involved.

jbenet · 2015-05-30T00:03:45Z

rebase on master
One of the tests failed with get stuff: https://travis-ci.org/ipfs/go-ipfs/jobs/63625545 not sure if related or not-- could someone take a look?

expecting success: 
      mkdir -p dir &&
      touch dir/a &&
      mkdir -p dir/b &&
      echo "Hello, Worlds!" >dir/b/c &&
      HASH2=`ipfs add -r -q dir | tail -n 1` &&
      ipfs get "$HASH2" >actual

Error: unexpected EOF
not ok 36 - ipfs get succeeds (directory)
#   
#         mkdir -p dir &&
#         touch dir/a &&
#         mkdir -p dir/b &&
#         echo "Hello, Worlds!" >dir/b/c &&
#         HASH2=`ipfs add -r -q dir | tail -n 1` &&
#         ipfs get "$HASH2" >actual
#       
expecting success: 
      printf "%s\n\n" "Saving file(s) to $HASH2" >expected &&
      test_cmp expected actual

> diff -u expected actual
--- expected    2015-05-22 09:59:00.930957429 +0000
+++ actual  2015-05-22 09:59:00.920958007 +0000
@@ -1,2 +1 @@
 Saving file(s) to QmbiZReo9jpGMrVtNKNuNpLA5cLX3vj5F24wp3xemdzCAn
-
not ok 37 - ipfs get output looks good (directory)
#   
#         printf "%s
#   
#   " "Saving file(s) to $HASH2" >expected &&
#         test_cmp expected actual
#       
expecting success: 
      test_cmp dir/a "$HASH2"/a &&
      test_cmp dir/b/c "$HASH2"/b/c &&
      rm -r "$HASH2"

diff: QmbiZReo9jpGMrVtNKNuNpLA5cLX3vj5F24wp3xemdzCAn/a: No such file or directory
> diff -u dir/a QmbiZReo9jpGMrVtNKNuNpLA5cLX3vj5F24wp3xemdzCAn/a
diff: QmbiZReo9jpGMrVtNKNuNpLA5cLX3vj5F24wp3xemdzCAn/a: No such file or directory
not ok 38 - ipfs get output is valid (directory)
#   
#         test_cmp dir/a "$HASH2"/a &&
#         test_cmp dir/b/c "$HASH2"/b/c &&
#         rm -r "$HASH2"
#       
expecting success: 
      ipfs get "$HASH2" -a -C >actual

0 B / 166 B [----------------------------------------------------------] 0.00 % Error: unexpected EOF
not ok 39 - ipfs get -a -C succeeds (directory)
#   
#         ipfs get "$HASH2" -a -C >actual
#

jbenet · 2015-05-30T00:03:56Z

@rht sorry for delay here o/

rht · 2015-05-30T07:02:53Z

'echo "!"' doesn't escape the '!'
edit: nope this is orthogonal to the issue.

rht · 2015-05-30T22:54:19Z

Sorry will fix this as soon as I can.

jbenet · 2015-06-03T22:19:44Z

@rht any luck with this? would love to have it in-- get is painful without it

whyrusleeping · 2015-06-17T20:38:16Z

Is any help from me needed here?

rht · 2015-06-17T20:53:43Z

Can't figure out the source of unexpected EOF here: https://travis-ci.org/ipfs/go-ipfs/jobs/66950969#L2886, https://travis-ci.org/ipfs/go-ipfs/jobs/66950969#L2918, https://travis-ci.org/ipfs/go-ipfs/jobs/66950969#L2918. Can't reproduce locally (sharness test passes just fine)

jbenet · 2015-06-30T18:22:46Z

@rht an update here? what's needed?

License: MIT Signed-off-by: rht <rhtbot@gmail.com>

up until now there has been a very annoying bug with get, we would get halting behavior. I'm not 100% sure this commit fixes it, but it should. It certainly fixes others found in the process of digging into the get / tar extractor code. (wish we could repro the bug reliably enough to make a test case). This is a much cleaner tar writer. the ad-hoc, error-prone synch for the tar reader is gone (with i believe was incorrect). it is replaced with a simple pipe and bufio. The tar logic is now in tar.Writer, which writes unixfs dag nodes into a tar archive (no need for synch here). And get's reader is constructed with DagArchive which sets up the pipe + bufio. NOTE: this commit also changes this behavior of `get`: When retrieving a single file, if the file exists, get would fail. this emulated the behavior of wget by default, which (without opts) does not overwrite if the file is there. This change makes get fail if the file is available locally. This seems more intuitive to me as expected from a unix tool-- though perhaps it should be discussed more before adopting. Everything seems to work fine, and i have not been able to reproduce the get halt bug. License: MIT Signed-off-by: Juan Batiz-Benet <juan@benet.ai>

jbenet · 2015-08-09T01:00:34Z

@rht so this is fixed, then?

rht · 2015-08-09T01:03:18Z

Yes it is.

My only concern here is when calculating the tar size, this is needed:

if err := proto.Unmarshal(nd.Data, pb); err != nil {
        return 0, err
}

Does this download the blocks underneath?

jbenet · 2015-08-09T01:06:18Z

@rht the proto.Unmarshal does not download blocks underneath, it merely decodes the encoded data. (the ipfs ls command downloads them to check the types, so it can draw a / on dirs/)

rht · 2015-08-09T01:16:49Z

get .tar progressbar
get .tar.gz progressbar
get .gz progressbar

ok it shouldn't be confusing to use uncompressed size in .tar.gz and .gz (but the progressbar reader has to be slipped in between tarwriter and gzwriter)?

License: MIT Signed-off-by: rht <rhtbot@gmail.com>

rht · 2015-08-09T18:04:58Z

@jbenet @whyrusleeping it appears that thirdparty/tar/extractor.go can be removed completely.
Because t is used only when the output is not compressed. i.e.
ipfs getdag | dag2tar | 3rdparty_untar -o unixfs

Why not just
ipfs getdag -o unixfs.

whyrusleeping · 2015-08-09T18:24:15Z

@rht kill it!

jbenet · 2015-08-09T19:07:54Z

I believe it is also used to download tarballs via the API. That way we can curl > .tar

—
Sent from Mailbox

On Sun, Aug 9, 2015 at 8:24 PM, Jeromy Johnson notifications@github.com
wrote:

@rht kill it!

Reply to this email directly or view it on GitHub:
#1279 (comment)

rht · 2015-08-09T19:28:49Z

I believe it is also used to download tarballs via the API. That way we can curl > .tar

Nope, if you grep 'github.com/ipfs/go-ipfs/thirdparty/tar', it is only used by ipfs get to extract intermediary tar-ed dag (not tar-ed file that was put into dag).
For curl pattern, use ipfs cat <hash> | tar xf (assuming the knowledge that the hash contains a tar).

whyrusleeping · 2015-08-09T23:21:44Z

@rht i was using it in ipfs shell until today when i refactored and pulled the package out here: https://github.com/whyrusleeping/tar-utils

rht · 2015-08-10T02:09:24Z

@whyrusleeping it is used because the uncompressed output is still tar-formatted? Though if #1558 happens, func (s *Shell) Get(hash, outdir string) error will become more complex for recursive get.

rht · 2015-08-10T05:35:24Z

Ok this is not more complex. tar-utils simply need to be replaced with a generic dag walker. This can be both imported here and used in ipfs get.

jbenet added the backlog label May 22, 2015

rht force-pushed the cleanup-progressbar branch from 363cd5d to de23db5 Compare May 22, 2015 09:54

jbenet reviewed May 22, 2015
View reviewed changes

rht force-pushed the cleanup-progressbar branch 2 times, most recently from 2d034f8 to 1170fd2 Compare May 30, 2015 06:24

rht force-pushed the cleanup-progressbar branch 3 times, most recently from 5e269f9 to bf7d735 Compare May 30, 2015 17:55

whyrusleeping mentioned this pull request Jun 2, 2015

Sprint J ipfs/team-mgmt#11

Closed

58 tasks

rht force-pushed the cleanup-progressbar branch 14 times, most recently from 18e8a83 to 53f80c3 Compare June 4, 2015 05:13

rht force-pushed the cleanup-progressbar branch 4 times, most recently from 703d6de to 97ed6e0 Compare June 15, 2015 23:28

rht force-pushed the cleanup-progressbar branch 3 times, most recently from f50b7d4 to 6086258 Compare July 4, 2015 14:29

rht force-pushed the cleanup-progressbar branch 2 times, most recently from 30b963a to 65963e0 Compare August 8, 2015 08:31

Fix progress bar for "get" command

5ced5d3

License: MIT Signed-off-by: rht <rhtbot@gmail.com>

rht added 2 commits August 9, 2015 23:24

Reorg unixfs/tar

0020f0b

License: MIT Signed-off-by: rht <rhtbot@gmail.com>

test

a1cebe6

License: MIT Signed-off-by: rht <rhtbot@gmail.com>

rht force-pushed the cleanup-progressbar branch from 65963e0 to a1cebe6 Compare August 9, 2015 17:20

rht closed this Oct 3, 2015

jbenet removed the backlog label Oct 3, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix progress bar for "get" command #1279

Fix progress bar for "get" command #1279

rht commented May 22, 2015

jbenet May 22, 2015

jbenet May 22, 2015

rht May 22, 2015

rht May 22, 2015

jbenet May 22, 2015

jbenet commented May 22, 2015

whyrusleeping commented May 22, 2015

jbenet commented May 30, 2015

jbenet commented May 30, 2015

rht commented May 30, 2015

rht commented May 30, 2015

jbenet commented Jun 3, 2015

whyrusleeping commented Jun 17, 2015

rht commented Jun 17, 2015

jbenet commented Jun 30, 2015

jbenet commented Aug 9, 2015

rht commented Aug 9, 2015

jbenet commented Aug 9, 2015

rht commented Aug 9, 2015

rht commented Aug 9, 2015

whyrusleeping commented Aug 9, 2015

jbenet commented Aug 9, 2015

@rht kill it!

rht commented Aug 9, 2015

whyrusleeping commented Aug 9, 2015

rht commented Aug 10, 2015

rht commented Aug 10, 2015

Fix progress bar for "get" command #1279

Fix progress bar for "get" command #1279

Conversation

rht commented May 22, 2015

jbenet May 22, 2015

Choose a reason for hiding this comment

jbenet May 22, 2015

Choose a reason for hiding this comment

rht May 22, 2015

Choose a reason for hiding this comment

rht May 22, 2015

Choose a reason for hiding this comment

jbenet May 22, 2015

Choose a reason for hiding this comment

jbenet commented May 22, 2015

whyrusleeping commented May 22, 2015

jbenet commented May 30, 2015

jbenet commented May 30, 2015

rht commented May 30, 2015

rht commented May 30, 2015

jbenet commented Jun 3, 2015

whyrusleeping commented Jun 17, 2015

rht commented Jun 17, 2015

jbenet commented Jun 30, 2015

jbenet commented Aug 9, 2015

rht commented Aug 9, 2015

jbenet commented Aug 9, 2015

rht commented Aug 9, 2015

rht commented Aug 9, 2015

whyrusleeping commented Aug 9, 2015

jbenet commented Aug 9, 2015

@rht kill it!

rht commented Aug 9, 2015

whyrusleeping commented Aug 9, 2015

rht commented Aug 10, 2015

rht commented Aug 10, 2015