Skip to content
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

MFS directory listings with l: true changes the file list order #5181

Closed
achingbrain opened this issue Jul 3, 2018 · 8 comments
Closed

MFS directory listings with l: true changes the file list order #5181

achingbrain opened this issue Jul 3, 2018 · 8 comments
Assignees
Labels
topic/files Topic files topic/technical debt Topic technical debt

Comments

@achingbrain
Copy link
Member

It seems that the order of directory contents returned from ipfs.files.ls api invocations changes if you pass l: true or l: false.

Compare (these tests have not previously only run against go-ipfs as there's been no mfs implementation in js-ipfs):

https://github.com/ipfs/interface-ipfs-core/blob/master/js/src/files/ls.js#L54-L61

vs

https://github.com/ipfs/interface-ipfs-core/blob/master/js/src/files/ls.js#L74-L92

Is this desired behaviour? Only we'll have to implement it in js-land as well if so.

@schomatis
Copy link
Contributor

Is this desired behaviour?

It wouldn't seem so, a sort() was added to the function that list names (b4a47e3) when -l: false,

https://github.com/ipfs/go-ipfs/blob/7e8f6c96047e1a94c1c892aa50e1c75e01b7fda2/mfs/dir.go#L235-L251

the function that lists entries (-l: true) with all of the information (including the name) doesn't sort its output,

https://github.com/ipfs/go-ipfs/blob/7e8f6c96047e1a94c1c892aa50e1c75e01b7fda2/mfs/dir.go#L253-L260

@Stebalien
Copy link
Member

Yeah, that sort should probably be removed and turned into an option (applied at a higher layer).

@schomatis schomatis self-assigned this Jul 8, 2018
@schomatis schomatis added topic/files Topic files topic/technical debt Topic technical debt labels Jul 8, 2018
@schomatis
Copy link
Contributor

Ok, I'll check if that sort can be removed without upsetting the current tests and add is as an option (independent of the -l one).

@schomatis
Copy link
Contributor

So, while working on #5219 I remembered that the entries are already sorted at the DAG link level,

https://github.com/ipfs/go-ipfs/blob/a9efa7e201707bd1a0d48fe1146a76ec0097db92/merkledag/node.go#L79-L84

I actually couldn't get an output from ipfs files ls that wasn't sorted (with or without -l),

It seems that the order of directory contents returned from ipfs.files.ls api invocations changes if you pass l: true or l: false.

@achingbrain Could you provide me an example of that?

@schomatis
Copy link
Contributor

schomatis commented Jul 13, 2018 via email

@schomatis
Copy link
Contributor

I'm suspecting that I'm confusing the JS ipfs.files.ls call with the actual ipfs files ls (go-ipfs) command.

@Stebalien
Copy link
Member

Note: They're only sorted for non-sharded directories.

@schomatis
Copy link
Contributor

ipfs config --json Experimental.ShardingEnabled true
HASH=$(echo "" | ipfs add -Q)
ipfs files cp /ipfs/$HASH /b
ipfs files cp /ipfs/$HASH /a
ipfs files ls /
# a
# b
ipfs files ls / -l
# b	Qmc5m94Gu7z62RC8waSKkZUrCCBJPyHbkpmGzEePxy2oXJ	1
# a	Qmc5m94Gu7z62RC8waSKkZUrCCBJPyHbkpmGzEePxy2oXJ	1

@ghost ghost removed the status/in-progress In progress label Jul 19, 2018
achingbrain added a commit to ipfs-inactive/interface-js-ipfs-core that referenced this issue Aug 2, 2018
achingbrain added a commit to ipfs-inactive/interface-js-ipfs-core that referenced this issue Aug 2, 2018
Previously we'd get files in different orders between `files.ls()` and
`files.ls({long: true})` due to a bug in `go-ipfs` (see ipfs/kubo#5181)

Since this has been resolved we can now expect consistent ordering.
achingbrain added a commit to ipfs-inactive/js-ipfs-mfs that referenced this issue Aug 2, 2018
Previously we had to do the extra sort due to ipfs/kubo#5181

Now that's been resolved & released we can remove it, yay!

License: MIT
Signed-off-by: achingbrain <alex@achingbrain.net>
alanshaw pushed a commit to ipfs-inactive/interface-js-ipfs-core that referenced this issue Aug 2, 2018
Previously we'd get files in different orders between `files.ls()` and
`files.ls({long: true})` due to a bug in `go-ipfs` (see ipfs/kubo#5181)

Since this has been resolved we can now expect consistent ordering.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic/files Topic files topic/technical debt Topic technical debt
Projects
None yet
Development

No branches or pull requests

3 participants