Adding directory with ./ leads to unexpected results

[rob-deutsch]

Version information:

go-ipfs version: 0.4.17-dfd19c470
Repo version: 7
System version: amd64/darwin
Golang version: go1.10.3

Type:

Bug

Description:

Unexpected behaviour when adding a directory using ./.

Expected behaviour when adding a directory is as follows:

$ ipfs add -r whoops/
added QmSkLz9P64rK8gwPw9JWN8FDBkJEP2x5W48DFJ4JjVB2nr whoops/bigfile
added QmNnfArfVAabiCgQKeE2cNi7CfYScX5BgHyvNRdJUSWehB whoops/smallfile
added QmYhHUDK28dU5QJnMnPwGU8gCUxegdcA5LkHjNU1ikbwWY whoops
$ ipfs ls QmYhHUDK28dU5QJnMnPwGU8gCUxegdcA5LkHjNU1ikbwWY
QmSkLz9P64rK8gwPw9JWN8FDBkJEP2x5W48DFJ4JjVB2nr 102424431 bigfile
QmNnfArfVAabiCgQKeE2cNi7CfYScX5BgHyvNRdJUSWehB 61        smallfile
$

Abnormal directory when adding the same directory with ./:

$ cd whoops/
$ ipfs add -r ./
added QmSkLz9P64rK8gwPw9JWN8FDBkJEP2x5W48DFJ4JjVB2nr bigfile
added QmNnfArfVAabiCgQKeE2cNi7CfYScX5BgHyvNRdJUSWehB smallfile
added QmUNLLsPACCz1vLxQVkXqqLX5R1X345qqfHbsf67hvA3Nn .
$ ipfs ls QmUNLLsPACCz1vLxQVkXqqLX5R1X345qqfHbsf67hvA3Nn
$ 

The primary observable problem is that ls doesn't list any files, but something deeper has probably gone awry here.

[overbool]

@rob-deutsch Maybe ipfs add regard the ./ as a special file, because I got the same result added QmUNLLsPACCz1vLxQVkXqqLX5R1X345qqfHbsf67hvA3Nn ./ in my several tests. The QmUNLLsPACCz1vLxQVkXqqLX5R1X345qqfHbsf67hvA3Nn is a fixed value.

@Stebalien Can I try to solve him?

[overbool]

@Stebalien
https://github.com/ipfs/go-ipfs-cmds/blob/master/cli/parse.go#L458-L460

Maybe this line should add fpath == "./"

[schomatis]

Hey you two! Thanks @rob-deutsch for reporting this and @overbool for diagnosing it, the slash is definetly tripping the command up, I'm not sure if ./ should be added as a special case or if the path should be stripped off of the trailing slashes, @keks is the command specialist so he can take the lead on this one and help you fix it.

[rob-deutsch]

I just did some research, and I believe that QmUNLLsPACCz1vLxQVkXqqLX5R1X345qqfHbsf67hvA3Nn is the address of an empty directory.

I could be biased, because I just spent a few hours looking at it (for unrelated reasons), but I think the issue could be around...

go-ipfs/core/coreunix/add.go#L380

I don't know if that code is wrong though. The issue might be further up the pipeline. go-ipfs thinks that the highest-level directory name when i type "./" is ".".

I'll await @keks expertise!

[rob-deutsch]

On second thought.... maybe my shell knowledge is incomplete! I just thought I'd try ipfs add -r . and I got the desired result:

$ cd whoops/
$ ipfs add -r .
added QmSkLz9P64rK8gwPw9JWN8FDBkJEP2x5W48DFJ4JjVB2nr bigfile
added QmNnfArfVAabiCgQKeE2cNi7CfYScX5BgHyvNRdJUSWehB smallfile
added QmYhHUDK28dU5QJnMnPwGU8gCUxegdcA5LkHjNU1ikbwWY whoops
$ ipfs ls QmYhHUDK28dU5QJnMnPwGU8gCUxegdcA5LkHjNU1ikbwWY
QmSkLz9P64rK8gwPw9JWN8FDBkJEP2x5W48DFJ4JjVB2nr 102424431 bigfile
QmNnfArfVAabiCgQKeE2cNi7CfYScX5BgHyvNRdJUSWehB 61        smallfile
$

So what's making a difference is whether I have / on the end of my path or not. I know that tar/cp/mv and friends can be picky when it comes to trailing /s. Maybe go-ipfs is mirroring the same conventions?

[edit]: That being said, if ./ means "only the contents of the directory" then go-ipfs probably shouldn't be creating that root node, right?

[keks]

My approach would be to pull fpath = filepath.Clean(fpath) to the very beginning of the function, i.e. before the if branch. That strips any slashes and we have "." again.

[overbool]

@keks Is this what you mean?

func appendFile(fpath string, argDef *cmdkit.Argument, recursive, hidden bool) (files.File, error) {
	fpath = filepath.Clean(fpath) // clean the path
	if fpath == "."{
		cwd, err := os.Getwd()
		if err != nil {
			return nil, err

https://github.com/ipfs/go-ipfs-cmds/pull/121/files#diff-3be2b56996dc8a3b09575e16f3cc9ef0L459

[Stebalien]

Really, we can just move https://github.com/ipfs/go-ipfs-cmds/blob/6aaaa76dfebaa3aea7030b9c505319235336da96/cli/parse.go#L471.

[overbool]

@Stebalien I had moved fpath = filepath.ToSlash(filepath.Clean(fpath)) to the beginning, is it right?

https://github.com/ipfs/go-ipfs-cmds/pull/121/files#diff-3be2b56996dc8a3b09575e16f3cc9ef0L459

[Stebalien]

I think so, but I'll let @keks take a look.

[Stebalien]

Needs tests.