-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
add correct test for 1515 #5364
Conversation
@@ -394,6 +394,27 @@ file_size() { | |||
$_STAT "$1" | |||
} | |||
|
|||
directory_size() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just something like du -sb --apparent-size
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Didn't know about it.
- MacOS 😢.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 nit, then lgtm
test/sharness/lib/test-lib.sh
Outdated
@@ -409,3 +430,9 @@ convert_tcp_maddr() { | |||
port_from_maddr() { | |||
echo $1 | awk -F'/' '{ print $NF }' | |||
} | |||
|
|||
|
|||
fail() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is test_fsh
- https://github.com/ipfs/go-ipfs/blob/1054826ac49f66d329d3088143f1a7f2b9930dcc/test/ipfs-test-lib.sh#L22-L27 and it's used all over sharness tests - like test_cmp expected unique_hash || test_fsh cat add_output
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah nice!
When checking to see if GC fully reverses an `ipfs add`, we should check the size of the actual files, not the directory sizes. A bunch of empty directories won't use *that* much space and really shouldn't count against GC. closes #1515 License: MIT Signed-off-by: Steven Allen <[email protected]>
Then the process is still irreversible. Accumulating empty dirs doesn't sound sustainable either. While the effect is negligible for embedded machines, this is not necessarily the case for clusters. |
It's bounded to 1024 empty directories. Whenever we store an object, we store it at |
When checking to see if GC fully reverses an
ipfs add
, we should check the size of the actual files, not the directory sizes. A bunch of empty directories won't use that much space and really shouldn't count against GC.closes #1515
I'm not sure why this has been sitting around for so long but I never seem to have created this PR.