-
-
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
fix: ipfs file ls: list node size for directories #4861
Conversation
I was looking at the test file |
License: MIT Signed-off-by: Lucas Molas <schomatis@gmail.com>
It would be good to take directory sharding into account here too -- a directory is a |
Really, directories should list their sizes. Recursively reading the size of the hamt is going to be really annoying. |
Not the size of the contents, just the raw size of the (sharded) directory object. Or do you mean that's something to precompute and stick into the object? |
That still involves traversing the sharded directory. Not that bad, just annoying.
Yeah. Ideally, everything would have a filesize. |
Add that to the ipld unixfs requirements listing |
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.
SGWM, we can't simply resolve the issue of sharded directories without defeating most of the benefit of sharding.
|
I got lost after the sharded directory comments, should I modify something of this PR? |
@schomatis so, we've been discussing this:
And I'm now less convinced we should actually include the size. Normally, filesize tells you how big a file would be when downloaded. However, the directory size (in bytes) won't really tell you anything useful. Thoughts? I don't really have any strong opinions. My initial opinion was "follow unix" but I'm now less convinced. |
Is not a good guide on this. Unix uses the physical directory size (that is the list of inodes), not the size of the contents, or even the number of entries. In some ways this makes sense and it is also easy to implement, but it isn't very useful and it doesn't really make sense for us. |
As suggested in the previous comments, this PR doesn't seems to add much value, closing. |
List the size of the underlying node for directories to honor the unix API.
If a test is needed: only the JSON encoding outputs the file size and I'm not sure how to process that with the
sharness
framework.Fixes #4580.