Skip to content
This repository has been archived by the owner on Mar 10, 2020. It is now read-only.

Make the size field of ls correspond to to the size of the file #427

Closed
Stebalien opened this issue Jan 8, 2019 · 10 comments
Closed

Make the size field of ls correspond to to the size of the file #427

Stebalien opened this issue Jan 8, 2019 · 10 comments
Labels

Comments

@Stebalien
Copy link
Contributor

Currently, the size returned by ls is the size of the DAG under the link (using link.Size). However, most users will expect ipfs ls /some/directory to list the sizes of the files (as returned by ipfs cat /some/directory/file | wc -b).

Proposal:

  1. Return a 0 (-1?) size for directories. Directory sizes really don't make sense.
  2. Return the actual file size for files. That is, the size stored in the file's filesize field.

I highly doubt this will break any existing projects. Really, it's more likely to fix hidden bugs than it is to cause them.

See: ipfs/kubo#5906 (comment)

@magik6k
Copy link
Contributor

magik6k commented Jan 8, 2019

Return a 0 (-1?)

I'd make it 0 as some things may expect uint

@achingbrain
Copy link
Collaborator

I’m massively in favour of this. Returning the DAG size is only useful to protocol spelunkers and is not verified by the codec so is not trustworthy.

For directories I’d prefer to omit the size field in the response instead of using 0 or -1.

@Stebalien
Copy link
Contributor Author

@magik6k will omitting them be fine? We could also use the DAG size for directories (given that we can do whatever we want, really).

Historical note from @whyrusleeping: ipfs ls was originally supposed to be used for both files and merkledag objects (pre IPLD). ipfs file ls was supposed to be the command for listing files. However, that no longer really applies.

@magik6k
Copy link
Contributor

magik6k commented Jan 9, 2019

I'd keep directory sizes in the response json, just not display them on the client

@kevina
Copy link

kevina commented Jan 9, 2019

I'd keep directory sizes in the response json, just not display them on the client

Is there a reason foe this?

@Stebalien
Copy link
Contributor Author

I'd keep directory sizes in the response json, just not display them on the client

Ideally, the user would be able to disable both --size and --resolve-type to avoid fetching the root block of every file in the directory.

@djdv
Copy link

djdv commented Jan 10, 2019

Relevant: ipfs/kubo#5325

@Stebalien
Copy link
Contributor Author

We've merged this change in go-ipfs.

@alanshaw
Copy link
Contributor

@achingbrain do you have any bandwidth to pick this up and restore interop?

alanshaw added a commit that referenced this issue Mar 13, 2019
Adjusts file sizes accordingly.

refs #427

License: MIT
Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
alanshaw added a commit that referenced this issue Mar 13, 2019
* fix: ls files sizes for compat with go-ipfs 0.4.19

Adjusts file sizes accordingly.

refs #427

License: MIT
Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>

* fix: end test after stream is complete

License: MIT
Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
@alanshaw
Copy link
Contributor

This was fixed by ipfs-inactive/js-ipfs-unixfs-exporter#20 and will be released in js-ipfs 0.35

@ghost ghost removed the ready label Mar 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

6 participants