ipfs / kubo

An IPFS implementation in Go
https://docs.ipfs.tech/how-to/command-line-quick-start/
Other
16.11k stars 3.01k forks source link

Multi file add does not work as I would expect #2811

Open kevina opened 8 years ago

kevina commented 8 years ago

It seams that even if -w is not specified, files added via ipfs add <file1> <file2> ... are added to a mfs directory. As I see it this leads to two unexpected behaviors:

1) The command will fail if a file with the same name is added twice, even if they are separate files:

$ mkdir a b
$ echo "Hello World!" > a/hello
$ echo 'HELLO WORLD!' > b/hello
$ ipfs add a/hello b/hello
added QmfM2r8seH2GiRaC4esTjeraXEachRt8ZsSeGaWTPLyMoG hello
Error: directory already has entry by that name

2) The files are pinned indirectly via the created directory

$ mv b/hello b/hello2
$ ipfs add  a/hello b/hello2 
added QmfM2r8seH2GiRaC4esTjeraXEachRt8ZsSeGaWTPLyMoG hello
added QmemWc3tw1HEM5oTTE1bxwvjKkn1yGFKJF1zVCZXD5wXgi hello2
$ ipfs pin rm QmfM2r8seH2GiRaC4esTjeraXEachRt8ZsSeGaWTPLyMoG
Error: QmfM2r8seH2GiRaC4esTjeraXEachRt8ZsSeGaWTPLyMoG is pinned indirectly under QmXpzvr3QHQ2Yxwb264HsNQsASJkD7jLhFBArCy6DX5iKV

Without -w the behavior I would expect is that each file is added and pinned individually.

whyrusleeping commented 8 years ago

hrm... interesting. That does seem like a bug

whyrusleeping commented 8 years ago

I guess the 'proper' solution here would be to make each parameter to add be its own separate 'add' call

kevina commented 8 years ago

So if the node is online, the client should make one request per file? Or are you thinking of something else.

Note that when "-r" is specified and the node is online, ipfs add -r dir/ translates to ipfs add -r dir dir/file1 dir/file2 ....

kevina commented 8 years ago

@whyrusleeping an easier solution would be to disallow multiple files when -r is specified and then if neither -w and -r is specified add and pin each file individually without adding the files to any directory.

I am hacking on this code now and think I can implement the above solution in a few hours.

kevina commented 8 years ago

@whyrusleeping I implemented a solution on my filestore branch (see 293e848). If neither '-w' or '-r' is specified it does not use an mfs-root and adds and pins each file individually. Pins are not flushed until the end to avoid a major slowdown.

whyrusleeping commented 8 years ago

@kevina i think the solution is simpler than that. I believe all we need to do is change the addAllAndPin method to use a separate adder for each input argument. (although it might not be as simple as i'm imagining). i'm hoping we can avoid further complicating the adder logic that way

kevina commented 8 years ago

@whyrusleeping I can try that, but for performance reasons it is important that pins are not flushed until the very end. Otherwise we lose most of the benefit of combining multiple files in a single command. In my informal tests the performance difference between flushing the pins each time and waiting to the end is an order of magnitude. There may also may be some performance lost due to the adding of an completely unnecessary directory object for each file.

kevina commented 8 years ago

@whyrusleeping I implemented your solution in commit e4b48dd. I might be able to refactor the addAllAndPin to make this a little nicer. I modified PinRoot so that flushing can be delayed to the end. Otherwise, performance will suffer.

I like my original solution better as it avoids the unnecessary mfs code altogether when directories are not being added. With your solution a unnecessary directory entry is being created and then left in the datastore.

My very informal test thought, indicate that the performance is about the same in either solution.

kevina commented 8 years ago

@whyrusleeping #3258 implemented the solution I am currently using. I would prefer we just use that for now. The adder is likely to undergo a rewrite soon for #3172.